Preserve 5.2 rollback compatibility for first-time table creates

[kriszyp]

⊙ Problem

A confirmed 5.3 → 5.2 rollback opens tables first created on 5.3 as empty because 5.2.15 does not read generation-suffixed store names. Writes during rollback then disappear on re-upgrade. Closes #3102.

❓ Your call: Implement option 2 as requested: restore compatibility for future first-time creates. Existing stamped tables and recreates remain unreadable by 5.2.15; patching the old reader or migrating those stores is separate work.

💡 Solution

First-time RocksDB creates use legacy primary/index names. A create with local name history uses a fresh generation. Crash recovery keeps a separate journal identity, and local-only drops leave untimed name history without advertising a replicated drop.

⚖️ Alternatives

Planning review: Framing-Verdict: chosen-approach-sound. Adopted its compatibility and lifecycle amendments: bare creating names live in creatingStores, which shipped 5.3 recovery ignores; untimed local-drop markers preserve naming history; retirement checks its tombstone before touching stores.

Patching 5.2 requires users to install a newer rollback target. Synchronous commit draining reverses #2604's nonblocking drop behavior. Conditional naming without create journaling leaves crash orphans unreclaimable. The existing journal supplies recovery without a second mechanism.

❓ Your call: Review depth is four full passes, the test-only delta and the final cleanup repair delta. The full passes cost roughly 25–30 minutes each; the diff grew from about 342 to 518 lines. Accepted changes protect legacy ownership, retire stale stores and coordinate overlapping journals. The closing pass corrected the causal interpretation of the baseline fixture, so the newly reachable cleanup defect is fixed here. The repair review confirms both journal orderings, and its conditional concurrent-create concern is refuted by the shared native catalog lock. Preserve the stated crash boundary and existing safeguards.

❓ Your call: Retain the late tombstone re-read and the field-compatibility comments: the former is required by the existing reentrant drop regression, and the latter explains shipped readers' destructive interpretation. A hypothetical live owner of the stale dropper's primary cannot arise under the preserved name-history rule. CI version provisioning stays in #3104, whose actual diff updates both the environment list and the older consumer.

🔧 Changes

Product and architecture tour

Store names and compatibility

Table state	Physical names after this change	Readable by 5.2.15
First local create, no name history	T/, T/str	Yes
Recreate, including after local-only cleanup	T/@uuid, T/str@uuid	Unsupported
Existing stamped 5.3.0/5.3.1 table	Its existing names	No
Existing legacy table	Its existing names	Yes

The catalog lock protects the name-history decision and independent create journal. The native database lock spans the under-lock existing-primary recheck, store selection, publication and failed-create cleanup; another worker must reload the published table or wait for cleanup. Incoming primary-attribute generations are replaced with the local choice. Peer drop history can require stamping on a node that never held the table. Look hardest at publication ordering and the creatingStores field: shipped 5.3 recovery interprets the old stores field destructively even when a bare primary was published.

The publication flag follows the primary write immediately, so failure to remove the journal cannot roll back a live table. Failure cleanup drops the recorded physical names and retains the journal when a physical drop fails. Recovery matches a published create by its primary name, including a dropping primary whose retirement belongs to its dropper. Unpublished creates are reclaimed by recorded names; old suffix-only journals remain supported.

Drops and durable name history

An untimed /dropped/T marker records local name history without advertising a replicated drop; timed drop readers continue skipping it. The tombstone identity guard precedes store capture and retirement, with the existing post-drop guard retained for reentrant callbacks. A stale drop retires only its own primary/index handles whose names the live catalog does not own, waits for admitted writes and sweeps its incarnation's blobs. It cannot journal or drop replacement-only indexes.

The existing lifecycle note records the naming, journal and rollback constraints; the design index points to that amended note.

Preserve asynchronous interrupted-drop recovery: rolling back while a bare predecessor still awaits reclamation can expose its rows on 5.2. Closing that crash window would require synchronous recovery before readiness and reverse #2604's lifecycle. Ordinary completed drops retire their own stores before success.

Recovery ownership and proof

The live catalog wins when a 5.2 writer reuses a bare name while ignoring an old journal. The active dropper retains its primary handle for the final blob sweep, even when another handle already dropped the family. Reclamation still removes unowned predecessor indexes and rechecks ownership before a deferred blob sweep starts and when it finishes under the catalog lock. The legacy-writer blob regression proves the live blob survives and an unowned index is removed. The additional current-version restart proves rollback-created rows survive reopening after reclamation; the same test checks rows and secondary indexes through a real 5.2.15 round trip. The two journal-order crash cases require blob unlinking before retirement completes.

A creating journal leaves a primary named by a pending retired journal to that journal's blob sweep. The retired journal clears after blob unlinking; the regression covers both row orderings.

Creation and publication order

databases.ts:4388

exclusiveLock()
journalId = randomUUID()
generation = hasPriorStores ? journalId : undefined
createJournal = {
  table: tableName,
  generation: journalId,
  phase: 'creating',
  primaryStore: storeNameFor(dbiName, generation),
  creatingStores: [storeNameFor(dbiName, generation), …]
}
attributesDbi.putSync(generationRowKey(journalId), createJournal)
… // open primary/index stores and write secondary catalog rows
attributesDbi.put(tableName + '/', deferredPrimaryRow)
published = true
attributesDbi.remove(generationRowKey(journalId))

Reclamation handoff

databases.ts:5672

currentRows = … // refresh journals under the catalog lock
retiredPrimaries = new Set(…)
for ({ key, value } of currentRows) {
  …
  owned = liveStoreNamesFor(attributesDbi, value.table)
  if (value.phase === 'retired' && columns.includes(value.primaryStore) && !owned.has(value.primaryStore)) {
    scheduleGenerationBlobSweep(rootStore, attributesDbi, databaseName, key, value)
    continue
  }
  for (columnName of columns) {
    if (value.phase === 'creating' && retiredPrimaries.has(columnName)) continue
    if (!owned.has(columnName) && (retired.has(columnName) || columnName.endsWith(suffix)))
      dropColumnFamily(rootStore, columnName)
  }
  … // clear this row after drops settle; retired rows own deferred primaries
}

✅ Verification

End-to-end route: integrationTests/upgrade/first-create-downgrade.test.ts uses a real 5.2.15 install and five boots (5.2 → current → confirmed 5.2 rollback → current → current restart). It checks primary rows, ordinary secondary-index searches and rollback writes in both an existing database and a newly created database. A stamped sibling recreate with a nonempty predecessor verifies the stated compatibility boundary. A table dropped immediately before rollback is recreated on 5.2, and its replacement survives re-upgrade despite the old retirement journal. It uses HARPER_PREVIOUS_MINOR_PATH, including #3104's list form (that PR also updates the existing minor-upgrade suite to parse the list); no configured 5.2 install means a clean skip. CI provisioning of that install is supplied by #3104, not duplicated here.

• Fresh build and focused create/drop/catalog/lifecycle tests: RocksDB 59 passed, 3 skips; LMDB 40 passed, 20 skips. The two overlapping-journal cases fail before the repair and pass afterwards. Full resource suites on preceding HEAD 3567715: RocksDB 4,069 passed, 54 skips; LMDB 3,074 passed, 697 skips. The final RocksDB resource rerun passes 4,071 tests with 54 skips on a4bbe27; this repair is confined to RocksDB reclamation.
• New integration test: 1 passed, 0 failures, 0 TODOs against 5.2.15. The original first-create probe fails on isolated base f038807 with actual [] versus rows a,b. The live-retirement blob probe also fails before its fix because a live blob is unlinked.
• The schema-listener retirement test also fails on the base when the old drop journals replacement-only attributes.
• Core unit suite: 6,786 passed, 207 pending, one failure in gitCredentials.test.js:395. The same install-spawn assertion fails on built clean base f038807 because Dispatch supplies inherited GIT_CONFIG_* keys. The credential environment was preserved. Full integration on preceding HEAD 3567715: 2,334 passed, zero assertion failures, 18 skipped, one TODO; exit 1 because the Ollama before-hook cancels six tests with ERR_IMPORT_ATTRIBUTE_MISSING on systemSchema.json. The same JSON-import error reproduces with untouched base source on Node 26.2.0; the Ollama backend, test and JSON import sites are unchanged. The five-boot rollback regression and existing minor-upgrade/restart suites pass inside that full run; the five-boot regression also passes again on the repair head.
• Repository formatting, lint and design-note checks pass (scratch artifacts excluded).
• Optional integration typecheck fails with the same 242 diagnostic lines on a freshly built, clean f038807 base using identical dependencies; this file adds none.

Release qualification: main is mergeable, but the automatic v5.3 cherry-pick reports conflicts on its generated branch, so release-line integration tests have not started. The executed local checks above cover this main-targeted PR; the backport still needs conflict resolution.

No companion documentation PR: this restores the existing rollback behavior for ordinary first creates and adds no public API/configuration. The existing stamped-table and recreate limits are explicit above.

_{🤖 Generated by OpenAI Codex; posted via @kriszyp.}

Related PRs: #2155 independent (expiration sweeps), #2955 independent (cross-worker drop enforcement), #3084 independent (replication authorization), #3104 independent (CI version fixtures), #3114 independent (v5.3 backport of those fixtures), #3100 independent (subscription resume), #2604 overlaps (narrows create naming while retaining deferred reclamation), #2431 independent (TLS/deploy anchors), #1916 independent (eviction/removal anchors)
Complexity: complicated

_{Review-Coverage: authored=codex; ran=cursor-composer,claude,gemini; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=6; full=4 @ a4bbe27}

_{Review-Attention: deep ~30m (critical: Table.ts, databases.ts; decisions: rollback-boundary, physical-naming, do-less-alternative) @ a4bbe27}

[github-actions]

Release cherry-pick v5.3: conflict

Cherry-pick onto v5.3 produced conflicts on commit(s): 4db030f2a0b8f4d064dfdf60446b4181ec597a40 b8fca4ec8a97f5549f05a2d0a4596bb775fb1f49 5f8f49e86fe741acfda7ae6ae4112b7f34b67c8e

The conflict markers are committed on branch cherry-pick/v5.3/pr-3118.
A pull request has been opened to land this patch: #3124

[gemini-code-assist]

Code Review

This pull request updates the table creation and drop lifecycle to support rollbacks to version 5.2, ensuring first-time creates on RocksDB use bare names when no prior drop history exists. It also refactors generation reclamation, adds a downgrade integration test, and updates unit tests. The review feedback suggests avoiding RocksDB-specific cleanup on LMDB in Table.ts while optimizing column array access, adding the missing table property to the GenerationRow type definition, and wrapping the test path resolution in a try/catch block to prevent potential test suite crashes.

[gemini-code-assist]

On LMDB storage engines, rootStore does not have a columns property, which means (rootStore as any).columns will be undefined. Passing undefined to new Set() is technically safe, but attempting to run this RocksDB-specific cleanup block on LMDB is unnecessary and could lead to unexpected behavior. Additionally, using optional chaining store?.name is safer in case any index store is not fully initialized or is nullish. Also, avoid copying the columns array (such as by creating a Set) because the columns getter in rocksdb-js already materializes a fresh array on each access; we can use .includes() directly to avoid redundant allocations.

					if (!removed) {
						withUpdateAttributesLock(rootStore, `retire stale stores of '${databaseName}.${tableName}'`, () => {
							const columnsArray = (rootStore as any).columns;
							if (!columnsArray) return;
							const owned = liveStoreNamesFor(dbisDb, tableName);
							for (const store of [primaryStore, ...Object.values(indices)]) {
								if (store?.name && columnsArray.includes(store.name) && !owned.has(store.name))
									dropColumnFamily(rootStore, store.name);
							}
						});
					} else await dbisDb.committed;

References

1. Do not use Array.from() or copy arrays returned by getters that already materialize a fresh array on each access (such as columns in rocksdb-js), as the loop already iterates over a snapshot and copying it would be redundant.

[gemini-code-assist]

The table property is used extensively on GenerationRow objects (e.g., value.table in reclaimGenerations and row.table in scheduleGenerationBlobSweep), but it is missing from the GenerationRow type definition. Adding it as an optional property ensures TypeScript type safety and completeness.

Suggested change

	creatingStores?: string[];
	creatingStores?: string[];
	table?: string;

[gemini-code-assist]

If HARPER_PREVIOUS_MINOR_PATH contains paths that do not have a package.json file or contain invalid JSON, readFileSync or JSON.parse will throw an error, causing the entire test suite to crash during module loading. Wrapping this in a try/catch block ensures that any such errors are caught and the path is safely skipped.

const previousPath = process.env.HARPER_PREVIOUS_MINOR_PATH?.split(delimiter).find((path) => {
	try {
		const { version } = JSON.parse(readFileSync(join(path, 'package.json'), 'utf8'));
		return version.startsWith('5.2.');
	} catch {
		return false;
	}
});