feat(npm): npm_virtual_repositories schema (S31 plan: 1/19)
Stacked MRs
| Part | MR | Scope | Reviewable LOC | Target |
|---|---|---|---|---|
| 1/3 | !1979 (merged) (merged) | the RepositoryStore.Delete foreign-key comments, and the interrupted-down recovery rule |
72 | main |
| 2/3 | this MR | the npm_virtual_repositories migration and the shape it declares |
1078 | main |
| 3/3 | !1988 (merged) | the absences the table carries, and the rows they admit | 686 | dmeshcharakou/s31-npm-virtual-step-1 |
Merge order is 1, 2, 3. The two links are not equally strong, so both are stated plainly.
Part 2 before part 3 is a build dependency. Part 3's cases query the table part 2 creates, and its file uses the table name, the identifier-name constants and the seed helpers that part 2's file declares. It neither compiles nor passes without part 2.
Part 1 before part 2 is a correctness one, not a build one. Part 2 compiles and passes against
main on its own. What part 1 buys is that part 2 does not leave RepositoryStore.Delete's comment
saying seven inbound foreign keys to repositories when there are eight: part 1 deletes that
enumeration, and part 2 adds the eighth and updates the one copy that survives, wantCascadeTargets in
the schema guard. Merged the other way round, part 2 ships a comment the same change made wrong.
Nothing in GitLab enforces either link. No merge-request dependency is set. Part 1 has since merged and part 2 was retargeted to main, so the 1-to-2 link is discharged and only this table records that it existed; the 2-to-3 order is still carried by the target-branch chain.
Parts 2 and 3 are both row 1 of the S31 plan
Status table, so both carry the 1/19 marker. One row shipping as several merge requests is this
plan's own recorded precedent. Part 1 is not a plan step: it is review fallout from part 2's branch
that turned out not to depend on the new table.
What this changes
A virtual npm repository stores nothing itself. It serves packages from an ordered list of other repositories. Before any of that can be built, there has to be a row that says "this repository is an npm virtual one", and something for the upstream list to hang off. That is all this MR adds.
npm_virtual_repositories is three uuid columns: namespace_id, id, and repository_id. It is a
type marker and nothing more. Everything a repository has (its name, visibility, format, kind) is read
from the parent repositories row, and everything it serves comes from the upstreams it composes, so
there is no cached content here and no credential. The table is hash-partitioned on namespace_id
into 64 partitions, with a unique index on (namespace_id, repository_id), matching ADR-007 exactly.
Nothing reads or writes it yet. repositoryCreateKindGate still refuses RepositoryKindVirtual
outright, so the table cannot be populated by the running service and this MR cannot change how it
behaves today. The association table the upstream list needs is step 2.
The migration comment carries a long catalogue of what the table deliberately does not have, and
why: no url or auth token (a virtual repository dials no upstream, so a copy here would be a second
unaudited home for a token), no cache-validity or health columns (it caches nothing and has nothing of
its own to probe), no soft_deleted_at (ADR-007 puts soft deletion on the parent), and no timestamps
(the row lives and dies with its parent). That catalogue is load-bearing, because
scripts/ci/check-migration-immutability.sh freezes this file the moment it merges. The tests that
hold those absences honest are part 3, which is where the reader should look for them; this MR
carries the tests for the shape the migration positively declares.
Two things about the schema worth a moment
The composite foreign key is not a style choice. repositories is itself hash-partitioned on
namespace_id, so a single-column reference to it cannot be declared at all. Carrying namespace_id
in the reference also keeps the child in the same partition as its parent and rejects a row that pairs
one namespace's child with another namespace's repository. It is ON DELETE CASCADE, which ADR-007
fixes for every format child table; NO ACTION would refuse the delete of an otherwise empty virtual
repository.
One index covers both foreign keys. The unique index on (namespace_id, repository_id) is the
whole access path and the whole FK coverage. PostgreSQL does not auto-index FK columns, so without it
the integrity check on a repositories delete degrades to a sequential scan of the child partition.
Leading with namespace_id keeps partition pruning and still answers the equality-on-both-columns
probe the cascade issues, so a separate (repository_id, namespace_id) index would be redundant.
!1944 (merged) and the migration order
The earlier note on this MR concerned !1944 (merged), whose migration
20260826072147_add_npm_and_maven_remote_last_health_status_checks.sql timestamps earlier than
this one's 20260826105942 while internal/datastore/migrations/migrations.go sets
goose.WithAllowOutofOrder(false). Merging this first would have left !1944 (merged)'s migration out of order,
migrations.Up would fail, and onFatal would abort the process for Kubernetes to restart into the
same failure.
!1944 (merged) has since merged, so that is settled. This branch is rebased on top of it, goose up applies
every 20260826 migration in order locally, and the head is 20260826105942. structure.sql was
regenerated with scripts/db/dump-structure.sh on PostgreSQL 17, the version GL_PG_CURR_VERSION
names, and comes out byte-identical to the committed file.
What a review of the branch found
I reviewed the branch before opening this and fixed seven real problems, five of them in a file that
cannot be edited after merge. Two now live elsewhere in the stack: the RepositoryStore.Delete
foreign-key enumeration is part 1 (this MR keeps only the surviving copy, wantCascadeTargets), and
repositoryReapWalk's "a virtual repository owns no rows of its own" is part 3, next to the tests for
the row it now owns. Two sibling comments say the same thing and travel with it, in
lifecycle_reap_repository_test.go and reconcile_namespace_integration_test.go; the precise form the
internal/managementapi sites already use is "owns no artifact rows". Inserting that word is not a
one-line change: check-comment-caps.sh re-checks a whole block once the diff touches it, and the
three sit in grandfathered blocks of 18, 15 and 32 lines against caps of 1, 2 and 2, so part 3 has to
carry a compression or a sidecar move with it. The five in the migration file are below.
max_locks_per_transaction was described as a hard per-transaction ceiling. It is not; it sizes
the shared lock table together with max_connections and caps no single transaction. The file said
otherwise in three places, and falsified itself doing so: the parent CREATE it measures at 66
relations already exceeds the 64 it named as the limit.
sql/20260804120000_create_container_remote_repositories.sql states the accurate version and records
in so many words that the npm and Maven remote files phrase it the other way, so this branch had
reverted a correction the repo already landed. This matters beyond wording: that sentence was the only
recorded reason for the "do not batch the partition statements in a DO $$ ... LOOP" rule, so an
author who tests the stated ceiling, finds a transaction happily taking 130 locks, and concludes the
rule is folklore would collapse the loop and hold locks on the live repositories tree to commit. The
layout is unchanged; only its justification.
The insertable-shape list said five and was missing one. The comment enumerates the row shapes the
absent CHECK admits, and claims to be exhaustive. It named a soft-deleted parent but not a
soft-deleted namespace: namespaces carries deleted_at, purged_at, blocked_at, disabled_at
and suspended_at, and fk_nvr_namespace_id_namespaces checks only that the row exists. A half-list
is worse than none, because the reader takes the shapes it does not mention as forbidden.
Also fixed: two present-tense references to a write path that does not exist and is explicitly refused
(the same earlier comment pass had converted two sibling references to future tense and missed these);
a false claim that every npm sibling carries a plain index, when the two repository-level siblings
carry none and are the ones a transcription would come from; and an upTimeout claim that described
it as bounding one statement's runtime when it bounds the whole Up pass and shares that budget with
the advisory-lock retry.
The absence catalogue also moved out from between two column declarations, where it pushed the third column 59 lines from the second, into the table comment where it belongs.
Tests
Twelve cases: eight about the shape the migration positively declares, and four about its foreign keys.
_PartitionedTableShapepins the parent, its 64pNNpartitions as a sequence,HASH(namespace_id), and the primary key and its name._Columnspins the three columns with their types, nullability andcolumn_default. Every sibling suite pins defaults and this one did not, which has teeth forid: a laterDEFAULT gen_random_uuid()would hand out v4 ids against the UUIDv7 convention with nothing failing._UniqueRepositoryIndexShapepins the one index, its uniqueness and its ordered key columns, so a reordering that loses partition pruning fails here._ConstraintNamesFitIdentifierLimitchecks every identifier against 63 characters, because truncation past it is silent and builds a duplicate._EveryPartitionInheritsTheParentIndexesand_PartitionRoutingByHashOfNamespaceIDcover the partition fan-out from both ends._DownLockBudgetand_DownReversesEveryUpObjectread the migration file itself, so they hold without applying anything.Constraints_ForeignKeyNamesAndActionsreads both foreign keys back frompg_constraintby name and pins each one'sconfdeltype. It is also what binds the two FK name constants to the schema: their only other reader measures their length against a literal in the same file, so before this a rename in the migration failed no test. The action half is the only reliable guard on thenamespacesedge, since no insert can isolate it.Constraints_RepositoriesFKRejectsAbsentRepository,Constraints_RejectsAbsentNamespaceandConstraints_RepositoriesFKCascadesOnRepositoryDeletecover the behaviour, mirroring the threenpm_remotecases. Checked by mutation: renaming the constant fails the catalog case, and so does expectingawhere the migration declaresON DELETE CASCADE.
Run locally against PostgreSQL 16: the 8 cases pass with no skips, and the full
./internal/datastore/migrations package passes at 194s, which includes TestMigrations_UpDownUp
proving this migration applies, reverts and re-applies, and TestHeadVersion agreeing with the moved
knownHeadVersion. wantCascadeTargets gains npm_virtual_repositories and the internal/datastore
delete and reap tests pass. Also go vet -tags=integration, gofmt, squawk (0 issues), pg_format
idempotency, scripts/ci/check-comment-caps.sh, and golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false, which reports nothing in the new
file.
Size
1078 reviewable lines: 582 source, all of it the migration, 485 test (482 the shape and
foreign-key suite, 3 the knownHeadVersion and wantCascadeTargets lines), and 11 documentation
(4 the S20-A correction, 7 the lifecycle_reap_repository.md cascade list). A further 1273 lines are
generated and excluded (structure.sql 1170, go-jet 103).
The plan applies the 500-line ceiling to reviewable source, and exempts the three schema steps on a
named argument rather than on the 2.15:1 test-ratio convention it uses elsewhere: "Partition DDL is
what justifies them: the measured band for a sibling partitioned table is 376 to 600 lines of parent
plus 64 partitions plus indexes, and goose.WithAllowOutofOrder(false) forbids splitting one table
across two migrations." See "How the ceiling is applied" in
docs/plans/2026-08-21-s31-npm-virtual.md. At 582 source this sits inside that 376 to 600 band, and
it is now the whole of the source in this MR.
The migration itself cannot be split further: one table cannot span two migrations without tripping
the out-of-order guard the ## !1944 and the migration order section above is about. Everything that
could be split out was, into parts 1 and 3.
E2E scenarios
No scenario in docs/testing/ is added or affected, and the out-of-scope line for virtual npm
repositories in docs/testing/e2e/npm.md, which names S31 as their owner, is still correct. This step
adds a table with no reader, no writer, no route and no HTTP surface, so there is no end to end flow to
script. The plan gives the S31 catalog section a single owning step, step 19.
Follow-ups
- #548 tracks the schema-wide
lock_timeoutdecision this file defers to. The Up takes ShareRowExclusive onnamespacesand the whole 65-relationrepositoriestree with nothing bounding the wait, which is the existing posture for every partitioned create in the tree, not something this MR introduces.
Database Review Evidence
Collected by the db-review-prep skill, migration mode only.
Query mode does not apply: the three-dot diff against origin/main changes six Go files, and each one is either a *_test.go file or jet-generated code, so no hand-written file in this branch dispatches a statement.
Migrations
Note
Timings are from CI (db:migrate matrix, goose verbose) against an
empty database, in apply / rollback order per PG version.
Production-scale validation via Database Lab is not yet available. See
Database review evidence
for the matrix rationale and how to read the numbers.
| Migration | PG 16 | PG 17 | PG 18 |
|---|---|---|---|
20260826105942_create_npm_virtual_repositories.sql |
OK (396.98ms / 219.74ms) | OK (362.85ms / 256.32ms) | OK (471.6ms / 233.91ms) |
Migration notes: no anomalies flagged.
- No version-specific regression: the slowest apply (PG 18, 471.6ms) is 1.2x the second-slowest (PG 16, 396.98ms), against a 2x flag threshold.
- Well inside the boot budget: 471.6ms is the worst apply on an empty database, against the 5-minute per-migration cap in Time budget.
- Apply and rollback stay close: apply is 1.4x rollback on PG 17, 1.8x on PG 16 and 2.0x on PG 18, which is the band the other partitioned creates sit in. On PG 17,
create_repositories.sqlis 1.9x,create_npm_remote_files.sql1.7x andcreate_container_remote_repositories.sql1.4x. - The apply cost matches the closest sibling by shape almost exactly:
create_npm_repositories.sql, the other hash-partitioned per-format repository marker table, applies in 362.92ms on PG 17 against this table's 362.85ms.
Evidence from the merged-results pipeline #2797656312 for MR head d1c7b7f3. The db:migrate re-apply phase is idempotency verification only and is excluded from the table.
Related to #882 (closed)