mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-02 00:06:58 +00:00
db: add manifest_key and let the existing backfill populate it
First half of replacing manifests.id with a node-independent identity. Nothing depends on the column yet: id is still the primary key, and layers and manifest_references still reference it. Getting the column in place and filled first means the eventual swap operates on data that is already complete and already proven unique, instead of doing the fill and three table rebuilds in one step. The value cannot be computed by the migration. It is a truncated sha256, SQLite has no hash builtin, and go-libsql exposes no way to register one. So the fill uses machinery that already exists: the Jetstream backfill re-upserts every manifest across the protocol on startup, and both upsert paths now write manifest_key. That only works because of one extra clause. Both upserts guard their DO UPDATE with a WHERE that skips rows where nothing changed, which on a backfill re-run is nearly every row, so they would have skipped the very manifests that need filling. Adding "OR manifests.manifest_key IS NULL" is what makes an otherwise no-op pass populate the column. Verified by removing it: the backfill then fills zero of three manifests instead of three of three. The index is UNIQUE even though the column is nullable. SQLite treats NULLs as distinct, so unfilled rows coexist while every filled row is checked. That makes production data verify the 16-byte truncation rather than us assuming it: if two manifests ever derived the same key, it fails loudly at insert instead of silently attaching one manifest's layers to another after the swap. AppView logs the unfilled count at startup, since there is no single moment at which this becomes complete and the follow-up migration is only safe at zero. ManifestKey replaces the old fat "did|repo|digest" map key rather than sitting beside it; they were always the same question, answered without asking the database. The jetstream tests hand-maintained their own CREATE TABLE statements, which is the drift problem moved into a test: the copy had already fallen behind (it still had tags.id) and only failed once a query touched the difference. They use db.InitDB now, with foreign keys switched off to preserve the behavior the hand-rolled schema had. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f186760847
commit
802cc4ba96
+37
-12
@@ -1,6 +1,8 @@
|
||||
package db
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"strings"
|
||||
@@ -59,7 +61,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
start, end := chunk(len(manifests), i)
|
||||
batch := manifests[start:end]
|
||||
|
||||
const cols = 11
|
||||
const cols = 12
|
||||
args := make([]any, 0, len(batch)*cols)
|
||||
for _, m := range batch {
|
||||
args = append(args,
|
||||
@@ -68,13 +70,19 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
m.ConfigSize, m.ArtifactType,
|
||||
nullString(m.SubjectDigest),
|
||||
m.CreatedAt,
|
||||
ManifestKey(m.DID, m.Repository, m.Digest),
|
||||
)
|
||||
}
|
||||
|
||||
// The trailing "manifest_key IS NULL" clause on the WHERE is what
|
||||
// populates rows that predate the column. Without it this upsert skips
|
||||
// unchanged manifests entirely, so re-running the backfill would never
|
||||
// fill their key and the column would stay half-empty forever.
|
||||
query := `
|
||||
INSERT INTO manifests
|
||||
(did, repository, digest, hold_endpoint, schema_version, media_type,
|
||||
config_digest, config_size, artifact_type, subject_digest, created_at)
|
||||
config_digest, config_size, artifact_type, subject_digest, created_at,
|
||||
manifest_key)
|
||||
VALUES ` + buildPlaceholders(len(batch), cols) + `
|
||||
ON CONFLICT(did, repository, digest) DO UPDATE SET
|
||||
hold_endpoint = excluded.hold_endpoint,
|
||||
@@ -83,7 +91,8 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
config_digest = excluded.config_digest,
|
||||
config_size = excluded.config_size,
|
||||
artifact_type = excluded.artifact_type,
|
||||
subject_digest = excluded.subject_digest
|
||||
subject_digest = excluded.subject_digest,
|
||||
manifest_key = excluded.manifest_key
|
||||
WHERE excluded.hold_endpoint != manifests.hold_endpoint
|
||||
OR excluded.schema_version != manifests.schema_version
|
||||
OR excluded.media_type != manifests.media_type
|
||||
@@ -91,6 +100,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
OR excluded.config_size IS NOT manifests.config_size
|
||||
OR excluded.artifact_type != manifests.artifact_type
|
||||
OR excluded.subject_digest IS NOT manifests.subject_digest
|
||||
OR manifests.manifest_key IS NULL
|
||||
`
|
||||
if _, err := db.Exec(query, args...); err != nil {
|
||||
return nil, fmt.Errorf("batch insert manifests: %w", err)
|
||||
@@ -135,7 +145,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
return nil, fmt.Errorf("scan manifest id: %w", err)
|
||||
}
|
||||
// Key format matches what callers use: "did|repo|digest".
|
||||
out[manifestKey(did, repo, digest)] = id
|
||||
out[ManifestKey(did, repo, digest)] = id
|
||||
}
|
||||
rows.Close()
|
||||
}
|
||||
@@ -143,15 +153,30 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// ManifestKey builds the lookup key used by BatchInsertManifests' result map.
|
||||
// Callers construct the same key from their in-memory Manifest structs to
|
||||
// find the assigned id.
|
||||
// ManifestKey derives a manifest's node-independent identity from its natural
|
||||
// key, (did, repository, digest).
|
||||
//
|
||||
// This is both the value stored in manifests.manifest_key and the lookup key for
|
||||
// BatchInsertManifests' result map, which are the same concept: "which manifest
|
||||
// is this" answered without asking the database.
|
||||
//
|
||||
// Deterministic on purpose. A rowid is allocated by whichever node performs the
|
||||
// insert, so it is only a stable identity while every write funnels through a
|
||||
// single writer. Any node can compute this one, before the write, without a
|
||||
// round trip.
|
||||
//
|
||||
// Truncated to 16 bytes: 128 bits puts the birthday bound at 2^64, which is far
|
||||
// past the number of manifests that could ever exist here. The column carries a
|
||||
// UNIQUE index anyway, so real data checks the assumption rather than trusting
|
||||
// it.
|
||||
func ManifestKey(did, repository, digest string) string {
|
||||
return manifestKey(did, repository, digest)
|
||||
}
|
||||
|
||||
func manifestKey(did, repository, digest string) string {
|
||||
return did + "|" + repository + "|" + digest
|
||||
h := sha256.New()
|
||||
h.Write([]byte(did))
|
||||
h.Write([]byte{0})
|
||||
h.Write([]byte(repository))
|
||||
h.Write([]byte{0})
|
||||
h.Write([]byte(digest))
|
||||
return hex.EncodeToString(h.Sum(nil)[:16])
|
||||
}
|
||||
|
||||
// BatchInsertLayers inserts a batch of layers, skipping any that already exist.
|
||||
|
||||
@@ -0,0 +1,253 @@
|
||||
package db
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func manifestKeyTestUser(t *testing.T, database *sql.DB) string {
|
||||
t.Helper()
|
||||
const did = "did:plc:manifestowner"
|
||||
if err := UpsertUser(database, &User{
|
||||
DID: did,
|
||||
Handle: "owner.example.com",
|
||||
PDSEndpoint: "https://pds.example.com",
|
||||
LastSeen: time.Now(),
|
||||
}); err != nil {
|
||||
t.Fatalf("UpsertUser: %v", err)
|
||||
}
|
||||
return did
|
||||
}
|
||||
|
||||
func sampleManifest(did, repository, digest string) *Manifest {
|
||||
return &Manifest{
|
||||
DID: did,
|
||||
Repository: repository,
|
||||
Digest: digest,
|
||||
HoldEndpoint: "did:web:hold.example.com",
|
||||
SchemaVersion: 2,
|
||||
MediaType: "application/vnd.oci.image.manifest.v1+json",
|
||||
ArtifactType: "container-image",
|
||||
CreatedAt: time.Now(),
|
||||
}
|
||||
}
|
||||
|
||||
func storedManifestKey(t *testing.T, database *sql.DB, did, repository, digest string) (string, bool) {
|
||||
t.Helper()
|
||||
var key sql.NullString
|
||||
err := database.QueryRow(
|
||||
`SELECT manifest_key FROM manifests WHERE did = ? AND repository = ? AND digest = ?`,
|
||||
did, repository, digest,
|
||||
).Scan(&key)
|
||||
if err != nil {
|
||||
t.Fatalf("read manifest_key: %v", err)
|
||||
}
|
||||
return key.String, key.Valid
|
||||
}
|
||||
|
||||
// TestManifestKeyIsDeterministicAndDistinct: the key stands in for a manifest's
|
||||
// identity, so the same natural key must always produce the same value and
|
||||
// different ones must not collide.
|
||||
func TestManifestKeyIsDeterministicAndDistinct(t *testing.T) {
|
||||
a := ManifestKey("did:plc:alice", "myapp", "sha256:abc")
|
||||
if a != ManifestKey("did:plc:alice", "myapp", "sha256:abc") {
|
||||
t.Error("ManifestKey is not deterministic")
|
||||
}
|
||||
if len(a) != 32 {
|
||||
t.Errorf("expected a 32-character key, got %d", len(a))
|
||||
}
|
||||
|
||||
distinct := map[string]string{
|
||||
"different did": ManifestKey("did:plc:bob", "myapp", "sha256:abc"),
|
||||
"different repo": ManifestKey("did:plc:alice", "otherapp", "sha256:abc"),
|
||||
"different digest": ManifestKey("did:plc:alice", "myapp", "sha256:def"),
|
||||
}
|
||||
for name, key := range distinct {
|
||||
if key == a {
|
||||
t.Errorf("%s produced the same key", name)
|
||||
}
|
||||
}
|
||||
|
||||
// The separator matters: without it, ("ab", "c", d) and ("a", "bc", d)
|
||||
// would hash identically and two different manifests would share an identity.
|
||||
if ManifestKey("ab", "c", "d") == ManifestKey("a", "bc", "d") {
|
||||
t.Error("field boundaries are not encoded; concatenation is ambiguous")
|
||||
}
|
||||
}
|
||||
|
||||
// TestInsertManifestPopulatesKey: new rows must never be written without a key,
|
||||
// or the backfill would have to keep chasing them.
|
||||
func TestInsertManifestPopulatesKey(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
|
||||
got, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc")
|
||||
if !ok {
|
||||
t.Fatal("manifest_key was left NULL on insert")
|
||||
}
|
||||
if want := ManifestKey(did, "myapp", "sha256:abc"); got != want {
|
||||
t.Errorf("manifest_key = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBackfillFillsKeyForUnchangedManifest is the mechanism this whole approach
|
||||
// rests on.
|
||||
//
|
||||
// The column cannot be filled by its migration, because the value is a truncated
|
||||
// sha256 and SQLite has no hash builtin. Instead the ordinary Jetstream backfill
|
||||
// fills it while re-upserting every manifest. That only works because the upsert
|
||||
// carries an extra "OR manifests.manifest_key IS NULL" clause on its WHERE
|
||||
// guard: without it the upsert skips manifests whose other fields are unchanged,
|
||||
// which is nearly all of them on a re-run, and the column would stay half-empty
|
||||
// forever.
|
||||
func TestBackfillFillsKeyForUnchangedManifest(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
m := sampleManifest(did, "myapp", "sha256:abc")
|
||||
if _, err := InsertManifest(database, m); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
|
||||
// Simulate a row that predates the column.
|
||||
if _, err := database.Exec(
|
||||
`UPDATE manifests SET manifest_key = NULL WHERE did = ? AND repository = ? AND digest = ?`,
|
||||
did, "myapp", "sha256:abc",
|
||||
); err != nil {
|
||||
t.Fatalf("clear manifest_key: %v", err)
|
||||
}
|
||||
if _, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc"); ok {
|
||||
t.Fatal("failed to simulate a pre-existing row")
|
||||
}
|
||||
|
||||
// Re-upsert exactly the same manifest, as the backfill does. Nothing else
|
||||
// about it has changed.
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil {
|
||||
t.Fatalf("re-insert: %v", err)
|
||||
}
|
||||
|
||||
got, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc")
|
||||
if !ok {
|
||||
t.Fatal("an unchanged manifest kept its NULL key; the upsert's WHERE guard skipped it")
|
||||
}
|
||||
if want := ManifestKey(did, "myapp", "sha256:abc"); got != want {
|
||||
t.Errorf("manifest_key = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBatchBackfillFillsKeyForUnchangedManifests is the same property for the
|
||||
// batch path, which is the one the backfill worker actually uses.
|
||||
func TestBatchBackfillFillsKeyForUnchangedManifests(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
manifests := []Manifest{
|
||||
*sampleManifest(did, "myapp", "sha256:aaa"),
|
||||
*sampleManifest(did, "myapp", "sha256:bbb"),
|
||||
*sampleManifest(did, "otherapp", "sha256:ccc"),
|
||||
}
|
||||
if _, err := BatchInsertManifests(database, manifests); err != nil {
|
||||
t.Fatalf("BatchInsertManifests: %v", err)
|
||||
}
|
||||
if _, err := database.Exec(`UPDATE manifests SET manifest_key = NULL`); err != nil {
|
||||
t.Fatalf("clear keys: %v", err)
|
||||
}
|
||||
|
||||
if _, err := BatchInsertManifests(database, manifests); err != nil {
|
||||
t.Fatalf("re-run BatchInsertManifests: %v", err)
|
||||
}
|
||||
|
||||
remaining, total, err := ManifestKeyBackfillProgress(database)
|
||||
if err != nil {
|
||||
t.Fatalf("ManifestKeyBackfillProgress: %v", err)
|
||||
}
|
||||
if total != int64(len(manifests)) {
|
||||
t.Errorf("total = %d, want %d", total, len(manifests))
|
||||
}
|
||||
if remaining != 0 {
|
||||
t.Errorf("%d manifests still have no key after a full backfill pass", remaining)
|
||||
}
|
||||
}
|
||||
|
||||
// TestManifestKeyBackfillProgressCountsUnfilled: this is how anyone decides the
|
||||
// follow-up migration is safe to run, so it has to actually count.
|
||||
func TestManifestKeyBackfillProgressCountsUnfilled(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
for _, digest := range []string{"sha256:aaa", "sha256:bbb", "sha256:ccc"} {
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", digest)); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
remaining, total, err := ManifestKeyBackfillProgress(database)
|
||||
if err != nil {
|
||||
t.Fatalf("ManifestKeyBackfillProgress: %v", err)
|
||||
}
|
||||
if remaining != 0 || total != 3 {
|
||||
t.Errorf("remaining=%d total=%d, want 0 and 3", remaining, total)
|
||||
}
|
||||
|
||||
if _, err := database.Exec(
|
||||
`UPDATE manifests SET manifest_key = NULL WHERE digest IN ('sha256:aaa', 'sha256:bbb')`,
|
||||
); err != nil {
|
||||
t.Fatalf("clear keys: %v", err)
|
||||
}
|
||||
|
||||
remaining, total, err = ManifestKeyBackfillProgress(database)
|
||||
if err != nil {
|
||||
t.Fatalf("ManifestKeyBackfillProgress: %v", err)
|
||||
}
|
||||
if remaining != 2 || total != 3 {
|
||||
t.Errorf("remaining=%d total=%d, want 2 and 3", remaining, total)
|
||||
}
|
||||
}
|
||||
|
||||
// TestManifestKeyUniqueIndexRejectsCollisions: the index is what turns the
|
||||
// 16-byte truncation from an assumption into something the database checks. If
|
||||
// it were ever dropped, a collision would silently attach one manifest's layers
|
||||
// to another after the eventual swap.
|
||||
func TestManifestKeyUniqueIndexRejectsCollisions(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
key, _ := storedManifestKey(t, database, did, "myapp", "sha256:abc")
|
||||
|
||||
// Force a second, genuinely different manifest to claim the same key.
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:def")); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
_, err := database.Exec(
|
||||
`UPDATE manifests SET manifest_key = ? WHERE digest = ?`, key, "sha256:def")
|
||||
if err == nil {
|
||||
t.Error("two manifests were allowed to share a manifest_key; the unique index is missing")
|
||||
}
|
||||
}
|
||||
|
||||
// TestManifestKeyNullsDoNotCollide: the unique index has to tolerate many
|
||||
// unfilled rows, or the migration could not add it before the backfill runs.
|
||||
func TestManifestKeyNullsDoNotCollide(t *testing.T) {
|
||||
database := revTestDB(t)
|
||||
did := manifestKeyTestUser(t, database)
|
||||
|
||||
for _, digest := range []string{"sha256:aaa", "sha256:bbb", "sha256:ccc"} {
|
||||
if _, err := InsertManifest(database, sampleManifest(did, "myapp", digest)); err != nil {
|
||||
t.Fatalf("InsertManifest: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// SQLite treats NULLs as distinct in a unique index; several unfilled rows
|
||||
// must coexist.
|
||||
if _, err := database.Exec(`UPDATE manifests SET manifest_key = NULL`); err != nil {
|
||||
t.Errorf("multiple NULL manifest_key rows were rejected: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,33 @@
|
||||
description: |
|
||||
Add manifests.manifest_key, a node-independent identity derived from the
|
||||
natural key (did, repository, digest).
|
||||
|
||||
Nothing depends on it yet. id remains the primary key and layers /
|
||||
manifest_references still reference it. This is the first half of replacing the
|
||||
rowid: get the column in place and populated, so the eventual swap operates on
|
||||
data that is already complete and already proven unique, rather than doing the
|
||||
fill and three table rebuilds in one step.
|
||||
|
||||
Nullable, and there is no UPDATE here to fill it, because the value cannot be
|
||||
computed in SQL: it is a truncated sha256 and SQLite has no hash builtin, nor
|
||||
can go-libsql register one. The fill happens through the machinery that already
|
||||
exists. The Jetstream backfill re-upserts every manifest across the protocol on
|
||||
startup, and both upsert paths now write manifest_key and carry an extra
|
||||
"OR manifests.manifest_key IS NULL" clause on their WHERE guard. Without that
|
||||
clause the upserts would skip unchanged manifests and the column would never
|
||||
fill; with it, the existing backfill populates the table as a side effect of a
|
||||
run it was making anyway.
|
||||
|
||||
The index is UNIQUE even though the column is nullable. SQLite treats NULLs as
|
||||
distinct, so rows not yet filled do not collide with each other, while every
|
||||
filled row is checked. That makes production data verify the 16-byte
|
||||
truncation instead of us assuming it: if two manifests ever derived the same
|
||||
key, the insert fails loudly here rather than silently attaching one manifest's
|
||||
layers to another after the swap.
|
||||
|
||||
AppView logs the count of unfilled rows at startup. When it reaches zero the
|
||||
follow-up migration can make manifest_key NOT NULL, move layers and
|
||||
manifest_references onto it, and drop id.
|
||||
query: |
|
||||
ALTER TABLE manifests ADD COLUMN manifest_key TEXT;
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_manifests_manifest_key ON manifests(manifest_key);
|
||||
@@ -800,11 +800,16 @@ func DeleteTagsNotInList(db DBTX, did string, keepTags []struct{ Repository, Tag
|
||||
// Returns the manifest ID (works correctly for both insert and update)
|
||||
// Note: Annotations are stored separately in repository_annotations table
|
||||
func InsertManifest(db DBTX, manifest *Manifest) (int64, error) {
|
||||
// The trailing "manifest_key IS NULL" clause on the WHERE is what populates
|
||||
// rows that predate the column. Without it this upsert skips unchanged
|
||||
// manifests entirely, so re-running the backfill would never fill their key
|
||||
// and the column would stay half-empty forever.
|
||||
_, err := db.Exec(`
|
||||
INSERT INTO manifests
|
||||
(did, repository, digest, hold_endpoint, schema_version, media_type,
|
||||
config_digest, config_size, artifact_type, subject_digest, created_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
config_digest, config_size, artifact_type, subject_digest, created_at,
|
||||
manifest_key)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(did, repository, digest) DO UPDATE SET
|
||||
hold_endpoint = excluded.hold_endpoint,
|
||||
schema_version = excluded.schema_version,
|
||||
@@ -812,7 +817,8 @@ func InsertManifest(db DBTX, manifest *Manifest) (int64, error) {
|
||||
config_digest = excluded.config_digest,
|
||||
config_size = excluded.config_size,
|
||||
artifact_type = excluded.artifact_type,
|
||||
subject_digest = excluded.subject_digest
|
||||
subject_digest = excluded.subject_digest,
|
||||
manifest_key = excluded.manifest_key
|
||||
WHERE excluded.hold_endpoint != manifests.hold_endpoint
|
||||
OR excluded.schema_version != manifests.schema_version
|
||||
OR excluded.media_type != manifests.media_type
|
||||
@@ -820,11 +826,13 @@ func InsertManifest(db DBTX, manifest *Manifest) (int64, error) {
|
||||
OR excluded.config_size IS NOT manifests.config_size
|
||||
OR excluded.artifact_type != manifests.artifact_type
|
||||
OR excluded.subject_digest IS NOT manifests.subject_digest
|
||||
OR manifests.manifest_key IS NULL
|
||||
`, manifest.DID, manifest.Repository, manifest.Digest, manifest.HoldEndpoint,
|
||||
manifest.SchemaVersion, manifest.MediaType, manifest.ConfigDigest,
|
||||
manifest.ConfigSize, manifest.ArtifactType,
|
||||
sql.NullString{String: manifest.SubjectDigest, Valid: manifest.SubjectDigest != ""},
|
||||
manifest.CreatedAt)
|
||||
manifest.CreatedAt,
|
||||
ManifestKey(manifest.DID, manifest.Repository, manifest.Digest))
|
||||
|
||||
if err != nil {
|
||||
return 0, err
|
||||
@@ -3188,3 +3196,24 @@ func GetDailyStats(db DBTX, did, repository, startDate, endDate string) ([]Daily
|
||||
}
|
||||
return stats, rows.Err()
|
||||
}
|
||||
|
||||
// ManifestKeyBackfillProgress reports how far the manifest_key fill has got:
|
||||
// the number of manifests still missing a key, and the total.
|
||||
//
|
||||
// manifest_key is populated by the ordinary Jetstream backfill rather than by a
|
||||
// migration, because the value is a truncated sha256 and SQLite cannot compute
|
||||
// it. That makes "is it done yet" a question worth being able to answer: the
|
||||
// follow-up migration that makes the column NOT NULL, moves layers and
|
||||
// manifest_references onto it, and drops id is only safe once remaining is zero.
|
||||
func ManifestKeyBackfillProgress(db DBTX) (remaining, total int64, err error) {
|
||||
err = db.QueryRow(`
|
||||
SELECT
|
||||
COUNT(*) FILTER (WHERE manifest_key IS NULL),
|
||||
COUNT(*)
|
||||
FROM manifests
|
||||
`).Scan(&remaining, &total)
|
||||
if err != nil {
|
||||
return 0, 0, fmt.Errorf("manifest_key backfill progress: %w", err)
|
||||
}
|
||||
return remaining, total, nil
|
||||
}
|
||||
|
||||
@@ -33,9 +33,23 @@ CREATE TABLE IF NOT EXISTS manifests (
|
||||
artifact_type TEXT NOT NULL DEFAULT 'container-image', -- container-image, helm-chart, unknown
|
||||
subject_digest TEXT, -- digest of the parent manifest (for attestations/referrers)
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
-- Node-independent identity: hex(sha256(did || 0 || repository || 0 || digest))
|
||||
-- truncated to 16 bytes. See db.ManifestKey.
|
||||
--
|
||||
-- Not yet load-bearing. id is still the primary key and layers /
|
||||
-- manifest_references still reference it; this column is being populated
|
||||
-- first so the swap can happen later against data that is already complete
|
||||
-- and already proven unique. Nullable only because rows predating it are
|
||||
-- filled in by the ordinary Jetstream backfill rather than all at once.
|
||||
manifest_key TEXT,
|
||||
UNIQUE(did, repository, digest),
|
||||
FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE
|
||||
);
|
||||
-- UNIQUE despite the column being nullable: SQLite treats NULLs as distinct, so
|
||||
-- unfilled rows do not collide, while every filled row is checked. That turns
|
||||
-- the 16-byte truncation from an assumption into something production data
|
||||
-- verifies before anything depends on it as a key.
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_manifests_manifest_key ON manifests(manifest_key);
|
||||
CREATE INDEX IF NOT EXISTS idx_manifests_did_repo ON manifests(did, repository);
|
||||
CREATE INDEX IF NOT EXISTS idx_manifests_created_at ON manifests(created_at DESC);
|
||||
CREATE INDEX IF NOT EXISTS idx_manifests_digest ON manifests(digest);
|
||||
|
||||
@@ -27,19 +27,6 @@ func TestBatchCaptains_VerifiesHoldService(t *testing.T) {
|
||||
db := setupTestDB(t)
|
||||
defer db.Close()
|
||||
|
||||
execStatements(t, db, `
|
||||
CREATE TABLE hold_captain_records (
|
||||
hold_did TEXT PRIMARY KEY,
|
||||
owner_did TEXT NOT NULL,
|
||||
public BOOLEAN NOT NULL,
|
||||
allow_all_crew BOOLEAN NOT NULL,
|
||||
deployed_at TEXT,
|
||||
region TEXT,
|
||||
successor TEXT,
|
||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
`)
|
||||
|
||||
realHold := "did:web:realhold.example.com"
|
||||
notAHold := "did:plc:notahold"
|
||||
unresolvable := "did:plc:unresolvable"
|
||||
|
||||
@@ -5,10 +5,10 @@ import (
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"atcr.io/pkg/appview/db"
|
||||
"atcr.io/pkg/atproto"
|
||||
"github.com/bluesky-social/indigo/atproto/identity"
|
||||
"github.com/bluesky-social/indigo/atproto/syntax"
|
||||
@@ -55,112 +55,28 @@ func holdIdentity(did, url string) *identity.Identity {
|
||||
}
|
||||
}
|
||||
|
||||
// execStatements splits a multi-statement SQL string and executes each statement individually.
|
||||
// go-libsql does not support multi-statement Exec like mattn/go-sqlite3.
|
||||
func execStatements(t *testing.T, db *sql.DB, schema string) {
|
||||
t.Helper()
|
||||
for stmt := range strings.SplitSeq(schema, ";") {
|
||||
stmt = strings.TrimSpace(stmt)
|
||||
if stmt == "" {
|
||||
continue
|
||||
}
|
||||
if _, err := db.Exec(stmt); err != nil {
|
||||
t.Fatalf("Failed to execute statement: %v\nSQL: %s", err, stmt)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// setupTestDB creates an in-memory SQLite database for testing
|
||||
// setupTestDB returns a database built from the real schema.
|
||||
//
|
||||
// This used to hand-maintain its own CREATE TABLE statements, which is the same
|
||||
// drift problem TestSchemaMatchesMigrations exists to prevent, just moved into a
|
||||
// test: the copy silently fell behind (it still had tags.id after that column
|
||||
// was dropped, and lacked manifests.manifest_key) and only failed once a query
|
||||
// happened to touch the difference. Using db.InitDB means schema changes cannot
|
||||
// rot this file.
|
||||
func setupTestDB(t *testing.T) *sql.DB {
|
||||
database, err := sql.Open("libsql", ":memory:")
|
||||
database, err := db.InitDB(":memory:", db.LibsqlConfig{})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open test database: %v", err)
|
||||
}
|
||||
|
||||
// Create schema
|
||||
schema := `
|
||||
CREATE TABLE users (
|
||||
did TEXT PRIMARY KEY,
|
||||
handle TEXT NOT NULL,
|
||||
pds_endpoint TEXT NOT NULL,
|
||||
avatar TEXT,
|
||||
default_hold_did TEXT,
|
||||
oci_client TEXT DEFAULT '',
|
||||
registry_domain TEXT DEFAULT '',
|
||||
last_seen TIMESTAMP NOT NULL
|
||||
);
|
||||
|
||||
CREATE TABLE manifests (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
did TEXT NOT NULL,
|
||||
repository TEXT NOT NULL,
|
||||
digest TEXT NOT NULL,
|
||||
hold_endpoint TEXT NOT NULL,
|
||||
schema_version INTEGER NOT NULL,
|
||||
media_type TEXT NOT NULL,
|
||||
config_digest TEXT,
|
||||
config_size INTEGER,
|
||||
artifact_type TEXT NOT NULL DEFAULT 'container-image',
|
||||
subject_digest TEXT,
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
UNIQUE(did, repository, digest)
|
||||
);
|
||||
|
||||
CREATE TABLE repository_annotations (
|
||||
did TEXT NOT NULL,
|
||||
repository TEXT NOT NULL,
|
||||
key TEXT NOT NULL,
|
||||
value TEXT NOT NULL,
|
||||
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY(did, repository, key),
|
||||
FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
CREATE TABLE layers (
|
||||
manifest_id INTEGER NOT NULL,
|
||||
digest TEXT NOT NULL,
|
||||
size INTEGER NOT NULL,
|
||||
media_type TEXT NOT NULL,
|
||||
layer_index INTEGER NOT NULL,
|
||||
annotations TEXT,
|
||||
PRIMARY KEY(manifest_id, layer_index)
|
||||
);
|
||||
|
||||
CREATE TABLE manifest_references (
|
||||
manifest_id INTEGER NOT NULL,
|
||||
digest TEXT NOT NULL,
|
||||
media_type TEXT NOT NULL,
|
||||
size INTEGER NOT NULL,
|
||||
platform_architecture TEXT,
|
||||
platform_os TEXT,
|
||||
platform_variant TEXT,
|
||||
platform_os_version TEXT,
|
||||
is_attestation BOOLEAN DEFAULT FALSE,
|
||||
reference_index INTEGER NOT NULL,
|
||||
PRIMARY KEY(manifest_id, reference_index)
|
||||
);
|
||||
|
||||
CREATE TABLE tags (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
did TEXT NOT NULL,
|
||||
repository TEXT NOT NULL,
|
||||
tag TEXT NOT NULL,
|
||||
digest TEXT NOT NULL,
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
UNIQUE(did, repository, tag)
|
||||
);
|
||||
|
||||
CREATE TABLE stars (
|
||||
starrer_did TEXT NOT NULL,
|
||||
owner_did TEXT NOT NULL,
|
||||
repository TEXT NOT NULL,
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
PRIMARY KEY(starrer_did, owner_did, repository)
|
||||
);
|
||||
`
|
||||
|
||||
execStatements(t, database, schema)
|
||||
|
||||
// Foreign keys off, matching the hand-rolled schema this replaced. These
|
||||
// tests insert manifests and tags for DIDs with no users row, exercising the
|
||||
// processor rather than referential integrity, and libSQL enables foreign
|
||||
// keys by default (unlike mattn). Exec, not QueryRow: unlike most libSQL
|
||||
// PRAGMAs this one returns no rows, the same way schema.go sets it.
|
||||
if _, err := database.Exec("PRAGMA foreign_keys = OFF"); err != nil {
|
||||
t.Fatalf("Failed to disable foreign keys: %v", err)
|
||||
}
|
||||
return database
|
||||
}
|
||||
|
||||
@@ -779,40 +695,8 @@ func TestProcessRecord_RoutesCorrectly(t *testing.T) {
|
||||
db := setupTestDB(t)
|
||||
defer db.Close()
|
||||
|
||||
// Add missing tables for this test
|
||||
execStatements(t, db, `
|
||||
CREATE TABLE repo_pages (
|
||||
did TEXT NOT NULL,
|
||||
repository TEXT NOT NULL,
|
||||
description TEXT,
|
||||
avatar_cid TEXT,
|
||||
created_at TIMESTAMP NOT NULL,
|
||||
updated_at TIMESTAMP NOT NULL,
|
||||
PRIMARY KEY(did, repository)
|
||||
);
|
||||
CREATE TABLE hold_captain_records (
|
||||
hold_did TEXT PRIMARY KEY,
|
||||
owner_did TEXT NOT NULL,
|
||||
public BOOLEAN NOT NULL,
|
||||
allow_all_crew BOOLEAN NOT NULL,
|
||||
deployed_at TEXT,
|
||||
region TEXT,
|
||||
successor TEXT,
|
||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
CREATE TABLE hold_crew_members (
|
||||
hold_did TEXT NOT NULL,
|
||||
member_did TEXT NOT NULL,
|
||||
rkey TEXT NOT NULL,
|
||||
role TEXT,
|
||||
permissions TEXT,
|
||||
tier TEXT,
|
||||
added_at TEXT,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY (hold_did, member_did)
|
||||
);
|
||||
`)
|
||||
// repo_pages, hold_captain_records and hold_crew_members come from the real
|
||||
// schema now; setupTestDB creates them.
|
||||
|
||||
// Register the hold DID so captain verification resolves locally instead
|
||||
// of hitting the network.
|
||||
@@ -886,19 +770,6 @@ func TestProcessCaptain_VerifiesHoldService(t *testing.T) {
|
||||
db := setupTestDB(t)
|
||||
defer db.Close()
|
||||
|
||||
execStatements(t, db, `
|
||||
CREATE TABLE hold_captain_records (
|
||||
hold_did TEXT PRIMARY KEY,
|
||||
owner_did TEXT NOT NULL,
|
||||
public BOOLEAN NOT NULL,
|
||||
allow_all_crew BOOLEAN NOT NULL,
|
||||
deployed_at TEXT,
|
||||
region TEXT,
|
||||
successor TEXT,
|
||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
`)
|
||||
|
||||
holdDID := "did:web:realhold.example.com"
|
||||
userDID := "did:plc:notahold"
|
||||
atproto.SetDirectory(&fakeDirectory{byDID: map[string]*identity.Identity{
|
||||
|
||||
@@ -201,6 +201,8 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
|
||||
slog.Info("Lease manager initialized",
|
||||
"component", "leases", "enabled", cfg.Leases.Enabled, "holder", s.Leases.HolderID())
|
||||
|
||||
reportManifestKeyBackfill(s.Database)
|
||||
|
||||
// The cleanup worker starts later, once the hold authorizer exists: it
|
||||
// clears the crew denial backoffs as its first act and needs a handle on it.
|
||||
|
||||
@@ -975,6 +977,30 @@ func (s *AppViewServer) waitForLeasedWorkers() {
|
||||
s.Leases.Wait(leaseDrainTimeout)
|
||||
}
|
||||
|
||||
// reportManifestKeyBackfill logs how much of manifests.manifest_key is still
|
||||
// unfilled.
|
||||
//
|
||||
// The column is populated by the ordinary Jetstream backfill rather than by its
|
||||
// migration, because the value is a truncated sha256 and SQLite cannot compute
|
||||
// one. So there is no single moment at which it becomes complete, and the
|
||||
// follow-up work that moves layers and manifest_references onto the key is only
|
||||
// safe once this reaches zero. Logging it is how anyone finds out.
|
||||
func reportManifestKeyBackfill(database *sql.DB) {
|
||||
remaining, total, err := db.ManifestKeyBackfillProgress(database)
|
||||
if err != nil {
|
||||
slog.Warn("Could not check manifest_key backfill progress", "error", err)
|
||||
return
|
||||
}
|
||||
if remaining == 0 {
|
||||
slog.Info("manifest_key backfill complete", "manifests", total)
|
||||
return
|
||||
}
|
||||
slog.Info("manifest_key backfill in progress",
|
||||
"remaining", remaining,
|
||||
"total", total,
|
||||
"hint", "the Jetstream backfill fills these as it re-upserts each manifest")
|
||||
}
|
||||
|
||||
// startCleanupWorker runs the periodic expiry sweep under the cleanup lease,
|
||||
// after clearing the crew denial backoffs once on acquiring it.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user