feat(datastore): blob_storage_blobs_by_namespace triggers and seed (S22 plan: 2b/21)
What this step delivers
blob_storage_blobs_by_namespace reached main empty and unmaintained.
This step makes it exactly consistent with blob_storage_blobs from the moment its migration commits.
One migration, 20260819073434_seed_blob_storage_blobs_by_namespace.sql, does both halves in one transaction:
fn_blob_storage_blobs_shadow_insertandfn_blob_storage_blobs_shadow_delete, plus theAFTER INSERTandAFTER DELETErow triggers that call them. The triggers are created first.- The one-time seed that copies the rows
blob_storage_blobsalready holds. The seed filters onEXISTS (SELECT 1 FROM namespaces n WHERE n.id = b.namespace_id), because the shadow table referencesnamespaces(id)and the base table does not.
Ordering inside that transaction is load-bearing.
CREATE TRIGGER blocks until in-flight writers finish, and it blocks new writers after that.
Every row the triggers do not fire for is therefore visible to the seed's snapshot.
If the seed ran first, a row committed between its snapshot and trigger creation stays missing from the shadow table forever.
internal/datastore/blob_storage_blobs_by_namespace.go adds the namespace-scoped read that reconciliation uses, BlobStorageBlobsByNamespaceStore.SumSizeByNamespaceID.
The shadow table is partitioned by namespace_id, so this read reaches one partition.
This step ships no write path, and no HTTP route reaches the store yet.
Three decisions the plan leaves open, each made in the code and worth a reviewer's eye:
- The
AFTER INSERTcopy is a plainINSERTwith noON CONFLICT. A collision can only mean the shadow table already diverged, and a swallowed collision keeps a stale size in a billing input. - The
Downerases the shadow table rows after itsDROP TRIGGERandDROP FUNCTIONstatements. A re-appliedUpis then exact rather than only idempotent. Without it, a base row deleted while the triggers were absent survives the seed'sON CONFLICT DO NOTHINGforever. SumSizeByNamespaceIDanswers0with no error for an empty namespace and for an unknown one.NamespaceStatisticsStorereturnsErrNotFoundfor a missing row, so this is the opposite choice. The reason is that the shadow table cascades fromnamespaces, so an absent namespace cannot hold shadow rows.
Migration cost and the deploy gate
CAUTION: Deploy this migration only when upload traffic is low. It blocks every blob write until its seed commits.
CAUTION: If the seed outlasts the 5-minute boot budget, pause the rollout or scale the new ReplicaSet to zero. Every pod in that ReplicaSet re-enters Runner.Start (internal/datastore/migrations/runner.go:128) on each restart. Each attempt re-takes the write-blocking locks for the whole budget, then rolls back with nothing applied. The over-budget mode repeats per pod restart. It is not one failed boot. A revert of the image does not stop the new ReplicaSet. That ReplicaSet retries, so the rollout pause is the only remedy.
Each CREATE TRIGGER on blob_storage_blobs takes SHARE ROW EXCLUSIVE on the partitioned parent and on all 64 partitions.
The two statements take 65 relation locks between them, and the transaction holds those locks until the seed commits.
The shared lock table holds one entry per transaction, relation and mode.
The second statement asks for locks the first statement already holds, so it adds none.
Blob writes block for that whole window.
They do not queue behind a long migration.
Plain reads are unaffected by the Up.
The Down is not the mirror of the Up and blocks reads as well.
This was measured on the branch rather than argued.
With the trigger transaction left open on a stock postgres:17-alpine:
| Concurrent operation | Result |
|---|---|
Maven PUT of a jar through the running service |
no response in 15 s |
Maven GET of an existing jar |
200 in 5 ms |
INSERT INTO blob_storage_blobs with lock_timeout = 3s |
canceled, blocked on the lock |
SELECT count(*) FROM blob_storage_blobs |
answered immediately |
After ROLLBACK, the same Maven PUT returned 201 in 83 ms.
The block was the lock, not a broken handler.
The lock counts behind that table, measured at stock postgres:17-alpine settings (max_locks_per_transaction=64, the value production uses and cannot raise):
| Measurement | Value |
|---|---|
One CREATE OR REPLACE TRIGGER on blob_storage_blobs |
65 ShareRowExclusiveLock relation locks: the parent plus 64 partitions |
Each trigger after Up |
present on 65 relations, so 130 trigger relations for the two |
One DROP TRIGGER in the Down |
65 AccessExclusiveLock relation locks, held to commit, so a rollback queues artifact reads as well as writes |
The TRUNCATE in the Down |
AccessExclusiveLock on 321 relations: the parent, its 64 partitions, 128 index clones, and 128 TOAST relations. It also takes SHARE on the 64 partitions and their 64 TOAST tables, so 449 lock-table entries in all. The two partitioned index parents take no lock |
| One blob write with both shadow triggers present | 12 relation locks: 7 RowExclusiveLock and 5 RowShareLock |
| The same blob write with the triggers dropped | 5 relation locks, all RowExclusiveLock |
| Ten-namespace hard-delete in one transaction | 12,624 relation locks, committed |
That last figure is the number internal/datastore/namespaces.go already records on main, so this step adds nothing to the namespace-delete lock spike.
Triggers are not references to namespaces.
The migration window is not the whole cost.
One INSERT INTO blob_storage_blobs ... ON CONFLICT DO NOTHING transaction takes 12 relation locks with both triggers present, and 5 with them dropped.
The seven extra locks are the shadow parent, the one shadow partition the namespace routes to, and the namespaces table with four of its indexes.
The four index locks and the table lock are the foreign-key probe that the shadow's namespaces reference makes.
This increase is permanent, and it lands on the same shared lock table that max_locks_per_transaction sizes.
The per-write draw on that table more than doubles.
## Merge order records an out of shared memory (53200) failure against the same table.
These two counts exclude the AccessShareLock that the observing query takes on pg_locks.
A reader who counts that row reads 13 and 6.
Measured on PostgreSQL 17.8, against a database cloned from this branch's integration-test template.
Three more costs the plan asks this description to record:
- The atomic seed is viable only while
blob_storage_blobsis small. #440 tracks the batched alternative. That alternative trades the trigger-and-seed-in-one-transaction guarantee for a bounded lock window. - Boot caps one
migrations.Upover the whole pending set at 5 minutes, lock-wait plus apply together. This migration shares that single budget with Step 1's two migrations and Step 2a's one when they arrive in the same deploy, which is howdocs/plans/2026-08-04-s22-storage-accounting.md:426states it. All three are merged onmain—20260812130000,20260812130100and20260817162904— so a database that has already applied them leaves this seed alone in the pending set. - Two
database_query_duration_secondsseries show a step at deploy time:{name="blob_storage_blobs_insert_on_conflict_nothing"}(internal/storage/pg_blobstore.go:769) and{name="upload_sessions_commit_tx"}(internal/storage/pg_session.go:1236). Both timers close over the base insert. From this migration they also cover a shadow heap insert, two btree entries, onenamespacesprobe, and the WAL behind all of it. The migration itself is DDL and writes no metric, so no marker series shows the cause. A p99 baseline, an SLO, or an alert threshold on either name shifts at deploy time, and this migration is the reason.
The pre-flight numbers are outstanding
The plan asks for two numbers in this description before this MR opens: the blob_storage_blobs row count in each environment, and a seed time measured against a clone of the largest environment.
Neither number exists.
No agent in this chain had database access, and no counts were supplied.
The blob_storage_blobs count is declined as a before-merge input.
Closed Beta exists for customers to test their workflow, so blob_storage_blobs stays small through it.
docs/specs/S22-storage-accounting.md, in ## Data Model, already carries that premise: the atomic seed "is viable while blob_storage_blobs is small (pre-GA / closed beta)".
Precise per-environment figures are out of reach under the current time pressure, so this MR merges without them.
The rule under **The deploy is gated on both numbers.** is unchanged, and #440 keeps the batched seed as the revisit path rather than a blocker.
Asked in !1652 (comment 3702376999) and declined there.
This deviates from the plan's Pre-flight instruction for Step 2b.
Neither the plan nor the spec was amended, so both still read as written, and this paragraph is the record of the difference.
No substitute is offered. A local timing curve was deliberately not taken. The migration comments state no duration. An earlier revision of this branch asserted a sub-second seed, and the squash removed that revision. No file in this MR states a seed duration now. A local seed of 4 rows took 49.8 ms, which says nothing about a deployed environment.
The deploy is gated on both numbers. Do not apply this migration to an environment before someone measures both numbers for that environment.
Who can get them:
- The count is one
SELECT count(*)per environment, against thegitlabhq_artifact_registrydatabase on the CloudSQL instance.runway/deployment.yamlnames. Anyone with access to thegitlab-runway-stagingandgitlab-runway-productionprojects can run it. - The seed time needs a clone.
docs/dev/database-migrations.mdrecords that production-scale validation through Database Lab is not available for Artifact Registry, so the clone is an improvised CloudSQL restore and needs Production Engineering.
The spec's revisit rule for #440 is stated in row counts, so that count also decides whether #440 is a follow-up or a blocker.
Database Review Evidence
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.
Pipeline 2771574897, on merge result cdfc44be of head f1f66274.
The three db:migrate jobs and db:structure-check all report success there.
| Migration | PG 16 | PG 17 | PG 18 |
|---|---|---|---|
20260819073434_seed_blob_storage_blobs_by_namespace.sql |
OK (18.31ms / 400.55ms) | OK (11.07ms / 55.79ms) | OK (29.22ms / 196.83ms) |
These numbers are a re-run, not a correction.
The previous table came from pipeline 2769810839, at the stamp 20260818121803.
The push since then rebases the branch onto main, which re-stamps the migration to 20260819073434, and it corrects comment text in the file.
Stripped of comments and blank lines, the two revisions of the file are identical, so no executed statement changed and the difference in the table is run-to-run variance.
docs/dev/database-migrations.md asks for a re-run of this evidence after any later push, and this section is that re-run.
Migration notes:
- The apply figure copies zero rows.
The
db:migratejob starts an empty database from thepostgresservice and applies the whole chain, soblob_storage_blobsis empty when this migration runs. The number measures the fixed part alone: two function creations, and twoCREATE OR REPLACE TRIGGERstatements across 130 relations. It contains none of the per-row seed cost. This is a stronger statement than an empty-database floor: the row-proportional half of the seed is absent from the number, not merely small in it.### The pre-flight numbers are outstandingrecords what is still missing, and the deploy stays gated on it. - The
TRUNCATEin theDownreaches 321 relations, and this section said 65 before. That 65 is the count for oneDROP TRIGGERonblob_storage_blobs, which is a different statement in the same section. TheTRUNCATEtakesACCESS EXCLUSIVEon the shadow's parent, on its 64 partitions, and on the 128 index clones those partitions carry. Those are 193 non-TOAST relations. It takes the same mode on each partition's TOAST table and TOAST index as well, for 321 relations in all. Counted one entry per relation and mode, the statement draws 449 lock-table entries. It also takesSHAREon each partition and on each partition's TOAST table. The shadow's two partitioned index parents take no lock, so "65 tables and their indexes" gives 195 and is wrong as well. The migration header states this arithmetic, corrected there in the commitdocs(datastore): count the Down TRUNCATE's real lock footprint. Each partition carries two index clones, andinternal/datastore/migrations/structure.sqlshows all 64 partitions with them. - The rollback figure is a relation-count cost, measured at one row count.
The rollback drops two triggers and two functions, then clears the shadow.
The shadow is empty when that clear runs in CI, so 58.21 ms to 390.14 ms is the fixed part alone.
TRUNCATEgives each relation a new relfilenode and does not touch the tuples, so its cost does not follow the row count. That property comes from PostgreSQL semantics rather than from these figures, which sit at one row count and fix no slope. The supersededDELETEmeasured 23.77 ms to 53.44 ms against the same empty database. The shippedTRUNCATEis therefore the slower of the two at zero rows. What it buys is a rollback window that stays flat as the shadow grows. - The PG 16 rollback is 2.7x the next slowest, and the ordering reproduces.
390.14 ms against 143.05 ms on PG 17 meets the threshold this skill flags as a version-specific regression.
A second pipeline, 2769701441, reports 529.98 ms, 139.90 ms, and 58.05 ms for the same three versions.
Its migration file differs from the one at the head only in the
squawk-ignore-filecomment on line 1. Both pipelines therefore run identical statements. PG 17 and PG 18 land within 3 percent of their head-pipeline rollback figures. In both pipelines PG 16 is 2.7x to 3.8x the PG 17 figure. The apply figures do not follow that pattern, and cluster from 14.02 ms to 24.53 ms across both pipelines and all three versions. A uniformly slower PG 16 runner therefore does not account for the rollback gap. This pass names no mechanism for the version difference. The rollback path runs undergoose reset, which boot does not take, and 390.14 ms is far less than the 1 s threshold for a slow migration. - The rollback is slower than the apply on every version, by 4.2x to 24x.
Both figures are fixed costs at this row count, and the rollback's is the larger of the two.
The apply figure grows with the
blob_storage_blobsrow count, because its seed copies every row. The rollback figure does not grow with it. This ordering therefore belongs to the empty database, and it reverses at a table size these numbers do not locate. - All three PG majors apply, roll back, and re-apply with status OK.
CREATE OR REPLACE TRIGGERneeds PostgreSQL 14 or later, and the matrix covers 16, 17, and 18. The syntax is available across the whole supported range. The re-apply figures are 16.4 ms, 18.61 ms, and 14.82 ms, and this skill excludes them from the table. db:structure-checkpasses at this head. That job rebuilds the schema dump from the whole chain at PG 17 and diffs it against the committedinternal/datastore/migrations/structure.sql. The migration and the committed dump therefore agree.- A local apply agrees with the CI band.
An earlier pass of this review applied the same migration in 15.11 ms, against an empty database in an ephemeral PostgreSQL 17.10 container.
It ran at the
DELETErevision. The statements in theUpsection are byte-identical between the two revisions, so that figure applies to the shippedUp. It is a local number and a second empty-database reading. It is not a production figure.
Queries
Note
Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.10 container, which matches GL_PG_CURR_VERSION in .gitlab-ci-other-versions.yml.
The seed data is synthesized. The numbers reflect moderate cardinality and do not capture production-scale effects.
See Database review evidence for seed sizing, methodology, and the anomalies the skill flags.
Seeds A and B roll back. Seed C commits, because VACUUM cannot run inside a transaction, and the container is destroyed after it.
Expand the details for the seed shape, the rendered SQL, the bound args, and the raw plans.
The seed column is an addition to the usual table. One seed shape is not enough for this query, because the shape decides the plan.
| Method | Seed | Plan node | Index | Rows (plan / actual) | Cost | Time | Buffers (hit / read) | Partitions |
|---|---|---|---|---|---|---|---|---|
datastore.BlobStorageBlobsByNamespaceStore.SumSizeByNamespaceID |
A: 1 namespace, 5000 rows | Aggregate over Seq Scan | n/a | 5000 / 5000 | 132.01 | 0.418ms | 57 / 0 | 1/64 |
datastore.BlobStorageBlobsByNamespaceStore.SumSizeByNamespaceID |
B: 20 namespaces in one partition, 100000 rows | Aggregate over Bitmap Heap Scan | blob_storage_blobs_by_namespace_p01_namespace_id_size_idx |
5027 / 5000 | 1351.79 | 0.524ms | 83 / 0 | 1/64 |
datastore.BlobStorageBlobsByNamespaceStore.SumSizeByNamespaceID |
C: same as B, then VACUUM (ANALYZE) |
Aggregate over Index Only Scan | blob_storage_blobs_by_namespace_p32_namespace_id_size_idx |
4913 / 5000 | 194.69 | 0.421ms | 27 / 0 | 1/64 |
Query notes:
- Partition pruning holds under all three seeds.
Each plan reaches one partition of 64.
That is the property the doc comment on
BlobStorageBlobsByNamespaceStoreclaims, and it is the reason this table exists. - The Seq Scan under seed A is the seed's artifact, not the query's.
The skill seeds 5000 rows that all carry the target namespace id.
Inside the pruned partition the predicate then matches every row, and a scan of 57 pages costs less than an index lookup.
Seeds B and C put 19 other namespaces in the same partition, and the planner takes the covering index.
The general rule: for a table partitioned by hash on
namespace_id, a single-namespace seed cannot show an index path for a single-namespace predicate. - Seed C is the only one that shows what
INCLUDE (size)buys. It reportsHeap Fetches: 0and 27 buffers, against 83 buffers for the same data under seed B. An index-only scan needs all-visible heap pages. Rows inserted inside theEXPLAINtransaction are never all-visible, so seeds A and B cannot produce one. - The unbounded-
SELECTrule does not apply. The statement is a single-row aggregate with noGROUP BY. - The estimates track reality. The widest gap is seed C, at 4913 planned against 5000 actual, which is 2 percent.
- Every plan reports
read=0. All buffer traffic is shared hits at this cardinality. - A reproduction of this pass needs the method named by hand.
The skill detects a query file from the presence of
.QueryContext(,.QueryRowContext(, or.ExecContext(. This file dispatches throughinstrumentQuery, so it matches none of the three.
datastore.BlobStorageBlobsByNamespaceStore.SumSizeByNamespaceID
Summary: The plan matches the method's intent.
namespace_id is the shadow's hash partition key, so the planner prunes to one partition of 64 under every seed.
With more than one namespace in that partition it drives the aggregate from index_blob_storage_blobs_by_namespace_on_namespace_id, the covering index that carries size.
No anomaly belongs to the query. The one Seq Scan comes from a single-namespace seed against a table partitioned on that same column.
Seed shape:
- A:
namespaces=1, blob_storage_blobs_by_namespace=5000 - B:
namespaces=20, blob_storage_blobs_by_namespace=100000 - C:
namespaces=20, blob_storage_blobs_by_namespace=100000
Seeds B and C place all 20 namespaces in one partition of the shadow.
satisfies_hash_partition selects the namespace ids that hash to the target remainder.
Rendered SQL:
SELECT COALESCE(SUM(blob_storage_blobs_by_namespace.size), $1) AS "total"
FROM public.blob_storage_blobs_by_namespace
WHERE blob_storage_blobs_by_namespace.namespace_id = $2::uuid;Bound args: [0, <seeded namespace uuid>].
$1 is the COALESCE fallback that jet renders from pg.Int(0), so the statement carries two placeholders for one caller parameter.
Plan A (1 namespace, 5000 rows):
Aggregate (cost=132.00..132.01 rows=1 width=32) (actual time=0.404..0.404 rows=1 loops=1)
Buffers: shared hit=57
-> Seq Scan on blob_storage_blobs_by_namespace_p18 blob_storage_blobs_by_namespace (cost=0.00..119.50 rows=5000 width=8) (actual time=0.006..0.275 rows=5000 loops=1)
Filter: (namespace_id = 'b3581ec4-fe11-4bc8-8bdc-c73339c92f58'::uuid)
Buffers: shared hit=57
Planning:
Buffers: shared hit=290
Planning Time: 1.111 ms
Execution Time: 0.418 msPlan B (20 namespaces in one partition, 100000 rows):
Aggregate (cost=1351.78..1351.79 rows=1 width=32) (actual time=0.500..0.501 rows=1 loops=1)
Buffers: shared hit=83
-> Bitmap Heap Scan on blob_storage_blobs_by_namespace_p01 blob_storage_blobs_by_namespace (cost=139.38..1339.21 rows=5027 width=8) (actual time=0.116..0.370 rows=5000 loops=1)
Recheck Cond: (namespace_id = 'ff461097-dd01-4763-a67b-e90d18653bc4'::uuid)
Heap Blocks: exact=57
Buffers: shared hit=83
-> Bitmap Index Scan on blob_storage_blobs_by_namespace_p01_namespace_id_size_idx (cost=0.00..138.12 rows=5027 width=0) (actual time=0.103..0.103 rows=5000 loops=1)
Index Cond: (namespace_id = 'ff461097-dd01-4763-a67b-e90d18653bc4'::uuid)
Buffers: shared hit=26
Planning:
Buffers: shared hit=21
Planning Time: 0.198 ms
Execution Time: 0.524 msPlan C (same data as B, committed, then VACUUM (ANALYZE)):
Aggregate (cost=194.68..194.69 rows=1 width=32) (actual time=0.403..0.404 rows=1 loops=1)
Buffers: shared hit=27
-> Index Only Scan using blob_storage_blobs_by_namespace_p32_namespace_id_size_idx on blob_storage_blobs_by_namespace_p32 blob_storage_blobs_by_namespace (cost=0.42..182.39 rows=4913 width=8) (actual time=0.016..0.274 rows=5000 loops=1)
Index Cond: (namespace_id = '7295a7f9-b40a-4cfc-bc6c-a0486c329d0d'::uuid)
Heap Fetches: 0
Buffers: shared hit=27
Planning:
Buffers: shared hit=75
Planning Time: 0.382 ms
Execution Time: 0.421 msTimings: A planning 1.111ms, execution 0.418ms, total 1.529ms.
B planning 0.198ms, execution 0.524ms, total 0.722ms.
C planning 0.382ms, execution 0.421ms, total 0.803ms.
Spec coverage
Scope: Step 2b covers criterion 15 whole and criterion 14's shadow-table roundtrip through the read. Every other row is another step's, and the owner column names the step the plan's own "Covers acceptance criteria" lines assign it to; those rows travel with those MRs.
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC-1 | Repo-scoped increment reaches repositories after the next drain tick |
Not this step — plan Step 10. |
| AC-2 | Namespace-scoped increment reaches namespace_statistics after the next drain tick |
Not this step — plan Step 10. |
| AC-3 | Concurrent increments to one scope sum exactly | Not this step — plan Steps 5 (Redis half) and 10 (Postgres half). |
| AC-4 | Scope re-marked dirty mid-claim is captured with no lost delta | Not this step — plan Steps 5, 8, 10, 14. |
| AC-5 | Chunk past drain_chunk_stale_timeout bails, re-adds, and issues no UPDATE |
Not this step — plan Step 8. |
| AC-6 | Chunk failing every attempt re-adds its scopes before the terminal error | Not this step — plan Step 8. |
| AC-7 | Redis unavailable at increment time does not fail the operation | Not this step — plan Steps 6 (drop half) and 14 (restore half). |
| AC-8 | Reconciliation clears the buffer before its scan | Not this step — plan Step 14. |
| AC-9 | Each counter recomputed with the correct soft-delete visibility, per format | Not this step — plan Steps 11, 12, 13, 14. |
| AC-10 | Positive hit per version-type table and (format, kind) combination |
Not this step — plan Steps 11 and 13. |
| AC-11 | Drift recorded on the unit-matched histogram before the overwrite | Not this step — plan Step 14. |
| AC-12 | Crash between HINCRBY and SADD still captured by reconciliation |
Not this step — plan Step 14. |
| AC-13 | Hash TTL refreshed on every write; dirty sets carry no TTL | Not this step — plan Steps 5 and 10. |
| AC-14 | namespace_statistics and blob_storage_blobs_by_namespace migrations apply cleanly and roundtrip through the ORM/jet layer; repositories.last_reconciled_at defaults to 'epoch' |
Shadow roundtrip through the read, this step: TestBlobStorageBlobsByNamespaceStore_SumSizeByNamespaceID (all five subtests), TestBlobStorageBlobsByNamespaceStore_MatchesTheBaseTableSum, TestBlobStorageBlobsByNamespaceMigration_SeedCoversRowsCommittedBeforeIt (apply and rollback of the trigger-and-seed migration), TestBlobStorageBlobsByNamespaceSeedMigration_DownDropsTriggersAndFunctions. namespace_statistics and the column: plan Step 1. Shadow-table schema shape: plan Step 2a. |
| AC-15 | The AFTER INSERT/AFTER DELETE triggers keep the shadow exactly consistent, and the namespace read over the shadow equals SUM(size) over SELECT DISTINCT against blob_storage_blobs |
This step, whole: TestBlobStorageBlobsShadowTrigger_InsertCopiesTheRowInTheSameTransaction, TestBlobStorageBlobsShadowTrigger_DeleteRemovesTheShadowRow, TestBlobStorageBlobsShadowTrigger_DeleteToleratesAMissingShadowRow, TestBlobStorageBlobsShadowTrigger_RejectsABlobForAnAbsentNamespace, TestBlobStorageBlobsByNamespaceStore_MatchesTheBaseTableSum, TestBlobStorageBlobsByNamespaceMigration_SeedCoversRowsCommittedBeforeIt; DDL: TestBlobStorageBlobsByNamespaceSchema_MaintenanceTriggers, TestBlobStorageBlobsByNamespaceSchema_TriggersCloneToEveryPartition, TestBlobStorageBlobsByNamespaceSeedMigration_UpCreatesTriggersBeforeSeed, TestBlobStorageBlobsByNamespaceSeedMigration_NoBatchingDoBlock, TestBlobStorageBlobsByNamespaceSchema_NamespaceDeleteCascadesTheShadow. |
| AC-16 | Every namespace has a zero-valued namespace_statistics row by construction |
Not this step — plan Step 1. |
| AC-17 | npm publish increment and unpublish Δartifacts on the pipeline |
Not this step — plan Step 17. |
| AC-18 | OCI increments at CompleteUpload, MountBlob, and manifest PUT |
Not this step — plan Step 18. |
| AC-19 | OCI decrements at the delete handlers, not inside the deleters | Not this step — plan Step 18. |
| AC-20 | Maven upload emits all four increments post-commit; the stub is retired | Not this step — plan Step 19. |
| AC-21 | Repository cascade hard-delete emits through S20-A's purger (gated on #464 (closed)) | Not this plan — travels with the S20-A plan. |
| AC-22 | One asynq task per namespace candidate, never one per repository | Not this step — plan Steps 14 (task half) and 15 (fan-out half). |
| AC-23 | A namespace with no statistics row gets one on its first pass (UPSERT) | Not this step — plan Step 13. |
| AC-24 | In-flight reconciliation tasks never exceed reconciliation_max_in_flight |
Not this step — plan Step 14. |
| AC-25 | Source-first ordering at every emit site | Not this step — plan Step 6 (contract) and Steps 17, 18, 19 (per-site rows). |
| AC-26 | last_reconciled_at stamped only after every repository is written back |
Not this step — plan Step 14. |
| AC-27 | Trigger fire selects only stale namespaces; the orphan sweep reaches the rest | Not this step — plan Step 15. |
| AC-28 | A namespace with an outstanding task is enqueued at most once at a time | Not this step — plan Step 15. |
| AC-29 | reconciliation_backlog exposed by a single-writer scrape-time collector |
Not this step — plan Step 16. |
| AC-30 | Chunk skips a scope reconciled since it captured its baseline | Not this step — plan Steps 7, 8, 14. |
| AC-31 | counter_dirty_set_size sampled once per tick, before SPOP |
Not this step — plan Step 10. |
| AC-32 | Config load rejects each invalid configuration | Not this step — plan Steps 3 and 3b. |
| AC-33 | The six S22 metrics registered with the specified names, types, and label sets | Not this step — plan Steps 8, 10, 14, 16; the alert-wiring half is not verifiable in this plan. |
| AC-34 | Reconciliation saturation re-enqueues rather than shedding or blocking | Not this step — plan Step 14. |
| AC-35 | Namespace-scoped chunk with no statistics row drops its delta and deletes :flushed |
Not this step — plan Steps 7, 8, 14. |
| AC-36 | A failing recovery SADD loses no delta |
Not this step — plan Steps 8 and 14. |
| AC-37 | Management-API deletes emit for both tombstoned and hard-deleted targets (gated on #313) | Not this plan — deferred, ships with #313. |
| AC-38 | A persistently failing reconciliation task stays re-enqueueable | Not this step — plan Step 14. |
Error cases
Every row is another step's: this step adds no pipeline path, and the spec lists no error case for the shadow's triggers, its seed, or the read over it. The shadow's own two-reaper lifetime — the cascade and the AFTER DELETE trigger, whichever fires first — is Data Model text rather than an Error Case row, and it is covered by TestBlobStorageBlobsShadowTrigger_DeleteToleratesAMissingShadowRow and TestBlobStorageBlobsByNamespaceSchema_NamespaceDeleteCascadesTheShadow.
| # | Condition | Tests |
|---|---|---|
| E-1 | Redis unavailable at increment time | Not this step — plan Steps 6 and 14. |
| E-2 | Redis unavailable at drain-trigger time | Not this step — plan Step 10. |
| E-3 | Chunk job's Postgres UPDATE fails |
Not this step — plan Step 8. |
| E-4 | Chunk job's :flushed DEL fails after the UPDATE succeeded |
Not this step — plan Step 8. |
| E-5 | Chunk job exhausts all retry attempts | Not this step — plan Step 8. |
| E-6 | Recovery SADD itself fails |
Not this step — plan Steps 8 and 14. |
| E-7 | Chunk dequeued later than drain_chunk_stale_timeout |
Not this step — plan Step 8. |
| E-8 | Worker dies mid-chunk after merging into :flushed |
Not this step — plan Steps 8 and 14. |
| E-9 | Two chunks run the same scope concurrently | Not this step — plan Steps 8 and 14. |
| E-10 | Trigger's EnqueueTx fails while the process is alive |
Not this step — plan Step 10. |
| E-11 | Crash between a trigger's SPOP and its EnqueueTx commit |
Not this step — plan Step 10; accepted crash-only gap. |
| E-12 | Assigned repository or namespace row hard-deleted before its chunk drains | Not this step — plan Steps 7 and 8. |
| E-13 | Namespace-scoped chunk drains a namespace with no statistics row | Not this step — plan Steps 8 and 14. |
| E-14 | Crash between a scope's HINCRBY and its SADD |
Not this step — plan Step 14. |
| E-15 | Crash between reconciliation's pre-scan clear and its SET counter = V |
Not this step — plan Step 14. |
| E-16 | Reconciliation scan races a concurrent increment | Not this step — plan Step 14. |
| E-17 | A drain chunk and a reconciliation process one scope concurrently | Not this step — plan Steps 7, 8, 14. |
| E-18 | Reconciliation finds a discrepancy | Not this step — plan Step 14. |
| E-19 | Namespace has no statistics row when its reconciliation task runs | Not this step — plan Steps 13 and 15. |
| E-20 | Reconciliation task fails before its final UPSERT | Not this step — plan Steps 14 and 15. |
| E-21 | A namespace can never be reconciled | Not this step — plan Steps 14, 15, 16. |
Security considerations
| # | Concern | Tests |
|---|---|---|
| S-1 | Redis keys carry internal UUIDs only, so no key injection or cross-slot relocation | Not this step — no Redis surface here; plan Steps 5 and 6. |
| S-2 | Counter values are non-secret, but deduplicated_size_bytes and components_count feed billing, so a bug has financial impact |
Partial, this step: TestBlobStorageBlobsByNamespaceStore_MatchesTheBaseTableSum pins the shadow against blob_storage_blobs itself, which is the only comparison that catches a silently divergent billing input — reconciliation treats the shadow as authoritative and nothing downstream disagrees with it. The drift metrics and the reconciliation schedule the concern also names are plan Steps 14 and 16. |
| S-3 | Redis and Postgres connectivity reuse the existing clients; no new credential surface | This step: NewBlobStorageBlobsByNamespaceStore takes the shared *postgres.Client and opens no connection of its own, pinned by TestNewBlobStorageBlobsByNamespaceStore_NilClientPanics (the client is constructor-injected, so there is no path that builds one). |
e2e scenario catalogs
No scenario is added, and none is affected.
docs/testing/e2e/ holds four format catalogs, and every row in them is a user-level journey driven through a client tool or an API surface.
This step ships no route, no response body, and no configuration knob.
No HTTP path reaches the new store.
The catalogs already settled this.
docs/testing/e2e/maven.md:133, inside e2e.maven.lifecycle.delete-package, reads: "Blob storage is reclaimed by GC after the retention grace period, so freed space is not asserted here (size accounting is S22)."
The user-facing surface for size accounting arrives in later S22 steps.
Divergences from the merged plan
Test fallout the plan does not name
blob_storage_blobs_by_namespace references namespaces, and the AFTER INSERT trigger copies every blob_storage_blobs row into it.
A fixture that commits a blob for an invented namespace id therefore fails with 23503.
This MR adds one fixture file and changes six existing test files for that one reason:
internal/storage/pg_namespace_fixture_integration_test.go(new, theseedNamespacesFordecorator)internal/storage/pg_blobstore_integration_test.gointernal/storage/pg_chunked_conformance_integration_test.gointernal/storage/pg_session_lifecycle_conformance_integration_test.gointernal/storage/realdriver_factory_integration_test.gointernal/remote/fetch_integration_test.gointernal/virtual/resolve_integration_test.go
All seven files are test-only.
The plan describes the absent-namespace rejection as a production edge case, and no Files entry in the plan lists these paths.
The plan text stays as written, and this paragraph is the record of the difference.
Two more test files change for a different trigger-driven reason.
internal/datastore/npm_files_list_integration_test.go and internal/datastore/npm_leaf_deleter_integration_test.go each gain a hand-written UPDATE blob_storage_blobs_by_namespace SET size = ..., in the helpers setBlobSize and npmLeafSetBlobSize.
There is no AFTER UPDATE trigger, so an in-place size change to a base row never reaches the shadow table.
A fixture that changes a size therefore writes both tables itself.
Both files are +24 lines, which is the ## Reviewable LOC row "Tests, shadow fixture repair in internal/datastore".
These two files are the only place in the tree that works around the absent AFTER UPDATE trigger.
The MutableColumns note sits on the type, not on a write path
The plan asks for the MutableColumns doc comment on this file's write path.
This step ships no write path.
Guardrail 19 allows a forward reference only in future tense and without godoc doc-link brackets.
The note therefore sits on the BlobStorageBlobsByNamespaceStore type doc, in future tense, in internal/datastore/blob_storage_blobs_by_namespace.go.
A reader who compares the plan against the code finds a difference at that sentence, and the code is the correct side.
The plan text stays as written.
The docs/dev/storage.md correction is beyond every Files entry
docs/dev/storage.md documented blob_storage_blobs.namespace_id as a foreign key to namespaces(id).
internal/datastore/migrations/structure.sql carries one fk_blob_storage_blobs* constraint, and that constraint belongs to the shadow table.
The base table has none, and that absence is what every new comment in this MR rests on.
The line was wrong before this MR.
This MR is what makes it load-bearing, because a blob write for a namespace with no row now fails with 23503.
This MR corrects the line and adds the same precondition to the commit-protocol DB transaction step.
No Files entry in the plan lists this document, so the change is beyond the plan.
The spec listed COPY ... FREEZE as a trigger bypass, and this MR removes it
docs/specs/S22-storage-accounting.md:132 listed COPY ... FREEZE beside TRUNCATE and partition DETACH, as an operation that bypasses the row triggers.
That fact is false on every PostgreSQL version the CI matrix runs.
COPY fires AFTER INSERT ... FOR EACH ROW triggers like any other insert.
COPY ... FREEZE against the partitioned parent raises ERROR: cannot perform COPY FREEZE on a partitioned table.
Against one leaf partition it is legal, and the cloned row trigger still fires there.
The review measured this behavior on PostgreSQL 16.14, 17.10 and 18.4.
This MR removes COPY ... FREEZE from that list.
It also adds one sentence to the spec, copied word for word from docs/dev/storage.md, so the two documents state the same behavior.
The branch already carried the correct fact in docs/dev/storage.md and in the seed migration's comment.
Without this edit, main carries two opposite statements about one server behavior, and the false one sits in the spec.
That spec governs the remaining S22 steps, including the two that declare a dependency on 2b.
The rule on that line does not change.
Only its membership and its stated mechanism change.
No Files entry in the plan lists this document, so the change is beyond the plan.
TRUNCATE and a partition DETACH do not reach the divergence in their bare form
docs/dev/storage.md:553 said none of the four operations raises an error when it runs, and the seed migration's comment said a TRUNCATE and a partition DETACH "both succeed".
Both statements are false, and the review reported it.
The foreign keys that reference blob_storage_blobs refuse the two bare statements.
The review measured this on PostgreSQL 16.14, 17.10 and 18.4, and this MR reproduced it on postgres:17-alpine, against a database built from this branch's structure.sql:
| Statement | Result |
|---|---|
TRUNCATE public.blob_storage_blobs |
0A000, and the DETAIL names blob_storage_attachments |
TRUNCATE ONLY public.blob_storage_blobs |
42809 cannot truncate only a partitioned table |
| the base table plus its six direct referencers | 0A000, one level out at container_blobs |
TRUNCATE ... CASCADE |
runs. Base rows 0, shadow rows 1 |
DETACH of a partition with one attached blob |
23503 |
DETACH of a partition with none |
runs. Base rows 0, shadow rows 1 |
The 0A000 arrives with every referencing table empty, because heap_truncate_check_FKs reads the constraint graph rather than the rows.
docs/dev/storage.md and the migration comment now state what each of the two operations does.
TRUNCATE reaches the divergence only as TRUNCATE ... CASCADE, and DETACH reaches it only for a partition that holds no attached blob.
Two other sentences named a preceding TRUNCATE as the cause of a stranded shadow row, one of them in the spec, so all three take the same edit.
The hazard does not change, and the operator rule does not change.
Only the statement forms that reach the hazard change.
No Files entry in the plan lists these two documents, so the change is beyond the plan.
The plan's max_locks_per_transaction reason is wrong, and the migration states the correct one
docs/plans/2026-08-04-s22-storage-accounting.md:724 reads "A few hundred relation locks is inside max_locks_per_transaction's default budget, so this does not fail".
That reason is wrong.
The setting sizes the shared lock table together with max_connections, and it caps no single transaction.
The arithmetic in the same paragraph of the plan contradicts the reading as well, because it counts 130 relations against a default of 64.
That count of 130 is also the wrong figure for the two statements.
They take 65 relation locks between them, and 130 is the number of pg_trigger rows they write.
The comparison against a default of 64 holds at 65 as well, so the conclusion of the paragraph does not change.
The conclusion the plan reaches is right, and only its stated reason is wrong.
The migration comment carries the correct reason, and so do docs/specs/S22-storage-accounting.md:123 and the postgres service comment in .gitlab-ci.yml.
The plan text stays as written, and this paragraph is the record of the difference.
The plan's Pre-flight instruction is not satisfied
Both pre-flight numbers are outstanding.
## Migration cost and the deploy gate carries that record and names who can close it.
Reviewable LOC
git diff --stat origin/main...HEAD reports 21 files, 2135 insertions and 70 deletions, which is 2205 reviewable LOC.
Guardrail 18 therefore applies.
The split by file group:
| Group | Files | Insertions / deletions |
|---|---|---|
| Migration SQL | 1 | +282 / -0 |
| Production Go | 3 | +143 / -9 |
| Tests for this step | 4 | +1399 / -26 |
Tests, shadow fixture repair in internal/datastore |
2 | +48 / -0 |
| Tests, trigger fallout in other packages | 7 | +197 / -26 |
Generated structure.sql |
1 | +33 / -0 |
docs/dev/storage.md |
1 | +27 / -4 |
docs/plans/2026-08-04-s22-storage-accounting.md |
1 | +2 / -2 |
docs/specs/S22-storage-accounting.md |
1 | +4 / -3 |
These nine rows cover all 21 files, and they sum to the 2135 insertions and 70 deletions of the totals line.
Production code is 425 of the 2135 insertions, and most of that is comment.
The migration file is 282 lines: 52 lines of SQL statements, 6 -- +goose directives, and 224 lines of comment or blank.
internal/datastore/blob_storage_blobs_by_namespace.go is 132 lines, of which 51 are code.
The plan's dependency table estimates Step 2b at about 410 lines, and tests are the whole of the difference.
Splitting this further does not help, for three reasons:
- The migration cannot be split. The spec makes trigger creation and the seed one transaction the basis of its exact-consistency argument, so the two halves must commit together.
- Tests are 1644 of the 2135 insertions, across 13 files. A test-only MR ahead of the migration fails, because its assertions need the triggers.
- The trigger fallout in
internal/storage,internal/remoteandinternal/virtualis 197 insertions across 7 files. Those files break as soon as the migration lands, so the repair belongs in the MR that lands it.
Merge order
Step 2a merged first, as !1510 (merged).
main therefore already carries blob_storage_blobs_by_namespace and its 64 partitions, and this MR is what starts to maintain them.
Nothing in this step widens a predicate ahead of the code that serves it.
One finding is reported rather than fixed here.
internal/format/npm/TestMiddlewareIntegration_Resolve failed once with out of shared memory (SQLSTATE 53200) on a local machine before this MR opened.
The cause is the 64-partition ON DELETE CASCADE that Step 2a added.
It meets concurrent namespace deletes at a stock max_locks_per_transaction.
The failure did not reproduce in this MR's runtime pass, alone or inside a five-package parallel run.
The cause is on main and not in this diff, so the repair is not in this MR.
One open merge request collides with this branch in internal/remote/fetch_integration_test.go.
This branch changes newRemoteTestBlobStore(t) storage.BlobStore to newRemoteTestBlobStore(t *testing.T, slug string) (storage.BlobStore, uuid.UUID).
!1325 adds a newRemoteTestBlobStore(t) call to the same file, and it is open and in Draft.
Whichever of the two merges second leaves the internal/remote integration build broken.
No pre-merge job compares two open branches, so the pipeline of the first one stays green.
The second merger rebases and takes the new signature.
A second merge request decided this migration's stamp, and it has merged.
!1567 (merged) merged on 2026-08-19 and carries 20260818132908_add_npm_packages_tombstoned_name_index.sql, which sorts after this step's earlier stamp.
internal/datastore/migrations/migrations.go:76 sets goose.WithAllowOutofOrder(false), so goose refuses a migration that sorts before one it already applied.
This branch is therefore rebased onto main and the seed is re-stamped to 20260819073434, which sorts after !1567 (merged)'s migration.
The re-stamp is one filename and the two references in internal/datastore/migrations/migrations_checksum_test.go, which are a comment and knownHeadVersion.
!1567 (merged) changed that same file, and the rebase settled the constant against main.
lint:migration-ordering and lint:migration-immutability both report success on the pipeline this description cites.
Related to #515
This is a bot message