From 83290aa638ad67823b5e047916a83ffb8049b5c4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 07:36:02 -0600 Subject: [PATCH 1/7] Keep first-time table stores readable after a minor rollback Keep legacy names until a table name has local history, journal unstamped creates independently, and preserve drop ownership during recovery. Co-Authored-By: OpenAI Codex GPT-6.1 --- DESIGN.md | 2 + .../upgrade/first-create-downgrade.test.ts | 99 +++++++ resources/DESIGN.md | 22 ++ resources/Table.ts | 17 +- resources/databases.ts | 254 +++++++++++++++++- .../resources/dropTableGeneration.test.js | 159 ++++++++++- 6 files changed, 525 insertions(+), 28 deletions(-) create mode 100644 integrationTests/upgrade/first-create-downgrade.test.ts diff --git a/DESIGN.md b/DESIGN.md index 0df77c61b3..e6bffae6a4 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -12,6 +12,8 @@ Index of the design notes for the harper core: one line per note, grouped by the ## resources/ — records, transactions, tables, blobs, audit +- [First-time create names and recovery](resources/DESIGN.md#table-drops-the-dropping-tombstone-ghost-tables-and-lifecycle-stamps) — Legacy names preserve minor rollback readability; name history and create journals keep replacements and crash recovery separate. + - [File overview](resources/DESIGN.md#file-overview) — What each file in `resources/` owns. - [`Resource.ts` — base class](resources/DESIGN.md#resourcets--base-class) — Static entry points versus instance methods, and the `transactional()` wrapper. - [`Table.ts` — section map](resources/DESIGN.md#tablets--section-map) — Section markers for the `makeTable()` factory. diff --git a/integrationTests/upgrade/first-create-downgrade.test.ts b/integrationTests/upgrade/first-create-downgrade.test.ts new file mode 100644 index 0000000000..63cb67ae93 --- /dev/null +++ b/integrationTests/upgrade/first-create-downgrade.test.ts @@ -0,0 +1,99 @@ +/** + * Tables first created after a minor upgrade remain readable and writable by 5.2, + * including secondary indexes and writes carried through a re-upgrade (#3102). + */ +import { suite, test, after } from 'node:test'; +import assert from 'node:assert'; +import { delimiter, join } from 'node:path'; +import { readFileSync } from 'node:fs'; +import { + startHarper, + killHarper, + teardownHarper, + sendOperation, + type ContextWithHarper, +} from '@harperfast/integration-testing'; + +const previousPath = process.env.HARPER_PREVIOUS_MINOR_PATH?.split(delimiter).find((path) => { + const { version } = JSON.parse(readFileSync(join(path, 'package.json'), 'utf8')); + return version.startsWith('5.2.'); +}); + +suite( + 'first-time table create survives 5.2 rollback', + { skip: !previousPath || process.env.HARPER_RUNTIME === 'bun' || process.platform === 'win32', timeout: 300_000 }, + (ctx: ContextWithHarper) => { + after(async () => teardownHarper(ctx)); + + test('preserves primary rows, secondary indexes and rollback writes', async () => { + const previousBin = join(previousPath!, 'dist', 'bin', 'harper.js'); + await startHarper(ctx, { config: {}, env: { TC_AGREEMENT: 'yes' }, harperBinPath: previousBin }); + await killHarper(ctx); + await startHarper(ctx, { config: {} }); + const schemas = [undefined, 'new_on_current']; + await sendOperation(ctx.harper, { operation: 'create_schema', schema: 'new_on_current' }); + const records = [ + { id: 'a', category: 'current' }, + { id: 'b', category: 'current' }, + ]; + for (const schema of schemas) { + await sendOperation(ctx.harper, { + operation: 'create_table', + schema, + table: 'born_on_current', + primary_key: 'id', + attributes: [ + { name: 'id', type: 'ID' }, + { name: 'category', type: 'String', indexed: true }, + ], + }); + await sendOperation(ctx.harper, { operation: 'upsert', schema, table: 'born_on_current', records }); + } + await sendOperation(ctx.harper, { operation: 'create_table', table: 'recreated', primary_key: 'id' }); + await sendOperation(ctx.harper, { operation: 'drop_table', table: 'recreated' }); + await sendOperation(ctx.harper, { operation: 'create_table', table: 'recreated', primary_key: 'id' }); + await sendOperation(ctx.harper, { operation: 'upsert', table: 'recreated', records: [{ id: 'stamped' }] }); + await killHarper(ctx); + await startHarper(ctx, { config: {}, env: { CONFIRM_DOWNGRADE: 'yes' }, harperBinPath: previousBin }); + + const read = (schema: string | undefined, attribute = 'id', value = '*') => + sendOperation(ctx.harper, { + operation: 'search_by_value', + schema, + table: 'born_on_current', + search_attribute: attribute, + search_value: value, + get_attributes: ['id', 'category'], + }); + assert.deepStrictEqual( + await sendOperation(ctx.harper, { + operation: 'search_by_value', + table: 'recreated', + search_attribute: 'id', + search_value: '*', + get_attributes: ['*'], + }), + [], + '5.2 does not support generation-stamped recreates' + ); + const rollbackRecord = { id: 'c', category: 'rollback' }; + for (const schema of schemas) { + assert.deepStrictEqual(await read(schema), records); + assert.deepStrictEqual(await read(schema, 'category', 'current'), records); + await sendOperation(ctx.harper, { + operation: 'upsert', + schema, + table: 'born_on_current', + records: [rollbackRecord], + }); + } + await killHarper(ctx); + await startHarper(ctx, { config: {} }); + for (const schema of schemas) { + assert.deepStrictEqual(await read(schema), [...records, rollbackRecord]); + assert.deepStrictEqual(await read(schema, 'category', 'current'), records); + assert.deepStrictEqual(await read(schema, 'category', 'rollback'), [rollbackRecord]); + } + }); + } +); diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 4ead587170..6651aea659 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -606,6 +606,7 @@ Consequence for callers that wrap the source in a hashing `Transform`: calling ` ## Table drops, the `dropping` tombstone, and ghost tables +<<<<<<< HEAD A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the `__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore persists a `dropping: true` flag on the table's primary catalog entry (`T/`) before any @@ -616,6 +617,27 @@ dies or a drop fails partway, the tombstone survives; both the boot-time schema interrupted drop instead of resurrecting the table. Without this, surviving catalog rows are silently re-opened with create-if-missing on the next start, which resurrects "deleted" tables (with their data, if the column families were never actually removed). +======= +`dropTableGeneration.test.js` enforces legacy primary/index names for first-time RocksDB creates (harper#3102), distinct names for recreates even after local-only reclamation, and recovery via a separate create-journal identity; `creatingStores` must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published legacy primary. Previously stamped tables retain their names; replicated drop markers can also force a first local create to be stamped. + +A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the +`__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore persists a +`dropping: true` flag on the primary catalog entry (`T/`) before any destructive work, then drops the +column families (awaited - a failed drop must surface as the operation's error), then removes the +catalog rows. If the process dies or a drop fails partway, the tombstone survives; the boot-time schema +load (`completeInterruptedDrop`) and a same-name `table()` create both complete the interrupted drop +instead of re-opening the surviving rows with create-if-missing, which resurrected "deleted" tables. + +**Lifecycle stamps (harper#1212).** The tombstone is node-local, so a peer offline for a replicated +`drop_table` would bring the table back through the schema handshake. Two durable facts give every node +one rule: the primary row carries `createdTime` from create (`declareTable`, kept from a peer's propagated +definition), and the tombstone carries `droppedTime` (`dropTable({ droppedTime })` applies a peer's), which +every completion path promotes to a `/dropped/` row (`promoteTombstoneToDropMarker`) before removing +the tombstone — no second-write crash cut. Both come from `tableLifecycleTime()`, the record-version clock; +`isDeadGeneration(createdTime, droppedTime)` is strict (equal survives, a missing stamp is 0). The marker +outlives a same-name recreate, only a newer drop overwrites it, the load parser skips `/dropped/` rows, and +`getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. `unitTests/resources/dropTableLifecycle.test.js`. +>>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) ## The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`) diff --git a/resources/Table.ts b/resources/Table.ts index ca3f108932..10f19b5a28 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3131,6 +3131,13 @@ export function makeTable(options): TableResourceClass { // has stopped and the wrapper has retired their storage. if (!(await retireFullTextStorage())) return false; const removed = withUpdateAttributesLock(rootStore, `table '${databaseName}.${tableName}'`, () => { + const currentPrimary = (dbisDb as any).getSync(primaryCatalogKey); + if ( + !currentPrimary?.dropping || + (currentPrimary.tableId != null && currentPrimary.tableId !== tableId) || + currentPrimary.dropGeneration !== dropGeneration + ) + return false; const stores = storeNamesFor(dbisDb, tableName, generation); const retiredStores = recordRetiredGeneration( dbisDb, @@ -3164,20 +3171,14 @@ export function makeTable(options): TableResourceClass { if (!droppedStores.has(columnName) && (rootStore as any).columns.includes(columnName)) dropColumnFamily(rootStore, columnName); } - const currentPrimary = (dbisDb as any).getSync(primaryCatalogKey); - if ( - !currentPrimary?.dropping || - (currentPrimary.tableId != null && currentPrimary.tableId !== tableId) || - currentPrimary.dropGeneration !== dropGeneration - ) - return false; for (const key of dbisDb.getKeys({ start: tableName + '/', end: tableName + '0' })) { if (key !== primaryCatalogKey) dbisDb.remove(key); } dbisDb.remove(primaryCatalogKey); return true; }); - if (removed) await dbisDb.committed; + if (!removed) return false; + await dbisDb.committed; const label = `${databaseName}.${tableName}`; const settled = await settlePhysicalDrops(rootStore, label); await sweepDroppedTableBlobs(primaryStore, label); diff --git a/resources/databases.ts b/resources/databases.ts index 0ad7821877..f82a0e7f41 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -595,6 +595,7 @@ type GenerationRow = { generation: string; phase: 'creating' | 'retired'; stores?: string[]; + creatingStores?: string[]; primaryStore?: string; blobSweepFailures?: number; }; @@ -649,6 +650,203 @@ export function recordRetiredGeneration( }); return merged; } +<<<<<<< HEAD +======= +const DROPPED_ROW_PREFIX = '/dropped/'; +const DROPPED_ROW_END = '/dropped0'; +const LIFECYCLE_STEP = 0.001; +export interface TableDropMarker { + table: string; + droppedTime: number; + tableId?: number; +} +/** + * The record-version clock, floored strictly above `after`: a stamp synthesized here for a fact that + * follows another (a recreate after a known drop, a drop of a generation stamped by a faster peer clock) + * must order after it even when this node's clock has not caught up. + */ +export function tableLifecycleTime(after?: number): number { + const now = getNextMonotonicTime(); + return Number.isFinite(after) && now <= after ? after + LIFECYCLE_STEP : now; +} +export function isDeadGeneration(createdTime: number | undefined, droppedTime: number): boolean { + return (Number.isFinite(createdTime) ? createdTime : 0) < droppedTime; +} +function droppedRowKey(tableName: string): string { + return DROPPED_ROW_PREFIX + tableName; +} +function dropMarkerStoreFor(databaseName: string): { rootStore: RootDatabaseKind; attributesDbi: any } | undefined { + if (!databases[databaseName]) return; + const rootStore = database({ database: databaseName, table: null }); + const attributesDbi = (rootStore as any)?.dbisDb; + return attributesDbi ? { rootStore, attributesDbi } : undefined; +} +/** + * Read-compare-write of the newest drop time, serialized against every other writer: the catalog lock + * on RocksDB (held by the caller when `exclusive` is false), a write transaction on LMDB. A live + * tombstone is read again inside that section, so a drop that joined and raised its time cannot be + * promoted from a copy read earlier. A tombstone written before the stamps existed has no time, and no + * marker is synthesized for it: a time made up at completion could postdate a peer's live recreate. + */ +function writeTableDropMarker( + rootStore: RootDatabaseKind, + attributesDbi, + databaseName: string, + tableName: string, + droppedTime: number | undefined, + tableId: number | undefined, + exclusive: boolean, + tombstoneKey?: string +): boolean { + const key = droppedRowKey(tableName); + const write = () => { + if (tombstoneKey) { + const live = attributesDbi.getSync(tombstoneKey); + if (live?.dropping && Number.isFinite(live.droppedTime) && !(droppedTime >= live.droppedTime)) { + droppedTime = live.droppedTime; + tableId = live.tableId ?? tableId; + } + } + if (!Number.isFinite(droppedTime)) { + if (rootStore instanceof RocksDatabase && tombstoneKey && !attributesDbi.getSync(key)) + attributesDbi.putSync(key, { table: tableName, tableId }); + return false; + } + const existing: TableDropMarker | undefined = attributesDbi.getSync(key); + if (existing && existing.droppedTime >= droppedTime) return false; + const marker: TableDropMarker = { table: tableName, droppedTime }; + if (tableId != null) marker.tableId = tableId; + attributesDbi.putSync(key, marker); + return true; + }; + let written: boolean; + if (rootStore instanceof RocksDatabase) { + written = exclusive + ? withUpdateAttributesLock(rootStore, `drop marker for '${databaseName}.${tableName}'`, write) + : write(); + } else { + written = (rootStore as any).transactionSync(write); + } + // Committed by here on both engines; emitted outside the caller's lock because listeners re-send + // schemas, which read this catalog. + if (written) + queueMicrotask(() => { + try { + databaseEventsEmitter.emit('tableDropRecorded', databaseName, tableName); + } catch (error) { + logger.warn(`A tableDropRecorded listener failed for ${databaseName}.${tableName}`, error); + } + }); + return written; +} +export function getTableDrops(databaseName: string): TableDropMarker[] { + const store = dropMarkerStoreFor(databaseName); + if (!store) return []; + const markers: TableDropMarker[] = []; + for (const { value } of store.attributesDbi.getRange({ start: DROPPED_ROW_PREFIX, end: DROPPED_ROW_END })) { + if (value && typeof value.table === 'string' && Number.isFinite(value.droppedTime)) markers.push(value); + } + return markers; +} +/** A peer's drop of a table this node does not hold; false when the database is unknown here or the marker is not newer. */ +export function recordTableDrop(databaseName: string, tableName: string, droppedTime: number): boolean { + if (!Number.isFinite(droppedTime)) return false; + const store = dropMarkerStoreFor(databaseName); + if (!store) return false; + return writeTableDropMarker( + store.rootStore, + store.attributesDbi, + databaseName, + tableName, + droppedTime, + undefined, + true + ); +} +export function onTableDropRecorded(listener: (databaseName: string, tableName: string) => void) { + databaseEventsEmitter.on('tableDropRecorded', listener); + return { + remove() { + databaseEventsEmitter.off('tableDropRecorded', listener); + }, + }; +} +/** The primary catalog row of a live table, under either key layout. */ +function primaryCatalogRowFor(table: { + dbisDB: any; + tableName: string; + primaryKey?: string; +}): { key: string; value: any } | undefined { + const bareKey = table.tableName + '/'; + const bare = table.dbisDB?.getSync(bareKey); + if (bare) return { key: bareKey, value: bare }; + if (!table.primaryKey) return; + const legacyKey = `${table.tableName}/${table.primaryKey}`; + const legacy = table.dbisDB?.getSync(legacyKey); + return legacy?.isPrimaryKey ? { key: legacyKey, value: legacy } : undefined; +} +/** The stamp on disk, which a class loaded on another thread may not carry yet. */ +export function catalogCreatedTime(table: { dbisDB: any; tableName: string; primaryKey?: string }): number | undefined { + const createdTime = primaryCatalogRowFor(table)?.value?.createdTime; + return Number.isFinite(createdTime) ? createdTime : undefined; +} +/** + * Backfills the stamp of a generation created on a build that stored none, once a peer's stamped definition + * proves which generation it is. Only an absent stamp is written; false when one exists or the table is gone. + */ +export function stampTableCreatedTime( + table: { dbisDB: any; tableName: string; primaryKey?: string; primaryStore: any; createdTime?: number }, + createdTime: number +): boolean { + if (!Number.isFinite(createdTime)) return false; + const rootStore = table.primaryStore?.rootStore; + const attributesDbi = table.dbisDB; + if (!rootStore || !attributesDbi) return false; + const write = () => { + const row = primaryCatalogRowFor(table); + if (!row || row.value.dropping || Number.isFinite(row.value.createdTime)) return false; + row.value.createdTime = createdTime; + attributesDbi.putSync(row.key, row.value); + return true; + }; + const written: boolean = + rootStore instanceof RocksDatabase + ? withUpdateAttributesLock(rootStore, `stamp '${table.tableName}'`, write) + : (rootStore as any).transactionSync(write); + if (written) table.createdTime = createdTime; + return written; +} +/** The time a drop of this name carries for peers: its marker, or the live tombstone of a drop still completing. */ +export function pendingOrRecordedDropTime(databaseName: string, tableName: string): number | undefined { + const store = dropMarkerStoreFor(databaseName); + if (!store) return; + const marker: TableDropMarker | undefined = store.attributesDbi.getSync(droppedRowKey(tableName)); + const tombstone = store.attributesDbi.getSync(tableName + '/'); + const pending = tombstone?.dropping ? tombstone.droppedTime : undefined; + const candidates = [marker?.droppedTime, pending].filter((time) => Number.isFinite(time)) as number[]; + return candidates.length ? Math.max(...candidates) : undefined; +} +/** Right before a completion path removes the live tombstone at `tombstoneKey`; RocksDB callers hold the catalog lock. */ +export function promoteTombstoneToDropMarker( + rootStore: RootDatabaseKind, + attributesDbi, + databaseName: string, + tableName: string, + tombstoneKey: string, + tombstone: { droppedTime?: number; tableId?: number } | undefined +): void { + writeTableDropMarker( + rootStore, + attributesDbi, + databaseName, + tableName, + tombstone?.droppedTime, + tombstone?.tableId, + false, + tombstoneKey + ); +} +>>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) export function storeNamesFor(attributesDbi, tableName: string, generation: string | undefined): string[] { const names: string[] = []; for (const key of attributesDbi.getKeys({ start: tableName + '/', end: tableName + '0' })) { @@ -3635,6 +3833,7 @@ function declareTable(target: TableTarget, tableDefinition: T let refreshedLiveAttributes = false; let deferredPrimaryRow: any; let generation: string | undefined; + let createJournal: GenerationRow | undefined; let unpublishedPrimaryStore: any; let published = false; let fullTextValuesForPersistence: unknown; @@ -4177,8 +4376,33 @@ function declareTable(target: TableTarget, tableDefinition: T clearInterruptedDropEntries(rootStore.path, tableName); } if (rootStore instanceof RocksDatabase) { - generation = randomUUID(); - attributesDbi.putSync(generationRowKey(generation), { table: tableName, generation, phase: 'creating' }); + const journalId = randomUUID(); + let hasPriorStores = Boolean( + attributesDbi.getSync(droppedRowKey(tableName)) || rootStore.columns.some((name) => name.startsWith(dbiName)) + ); + if (!hasPriorStores) { + for (const { value } of attributesDbi.getRange({ start: GENERATION_ROW_PREFIX, end: GENERATION_ROW_END })) { + if (value?.table === tableName) { + hasPriorStores = true; + break; + } + } + } + if (hasPriorStores) generation = journalId; + createJournal = { + table: tableName, + generation: journalId, + phase: 'creating', + primaryStore: storeNameFor(dbiName, generation), + // Shipped 5.3 readers reclaim `stores` even when a bare primary row was published. + creatingStores: [ + storeNameFor(dbiName, generation), + ...attributes + .filter((attribute) => attribute.indexed && !attribute.isPrimaryKey && !attribute.relationship) + .map((attribute) => storeNameFor(`${dbiName}${attribute.name}`, generation)), + ], + }; + attributesDbi.putSync(generationRowKey(journalId), createJournal); primaryStore = openRocksDatabase(rootStore.path, { ...dbiInit, name: storeNameFor(dbiName, generation), @@ -4200,6 +4424,7 @@ function declareTable(target: TableTarget, tableDefinition: T primaryKeyAttribute.tableId = primaryStore.tableId; if (generation) primaryKeyAttribute.generation = generation; + else delete primaryKeyAttribute.generation; Table = makeTable({ isBranch: Boolean(target.branch), primaryStore, @@ -4687,13 +4912,13 @@ function declareTable(target: TableTarget, tableDefinition: T // below is a no-op for a create — a table is never published with an incomplete relationship list. if (deferredPrimaryRow) { attributesDbi.put(tableName + '/', deferredPrimaryRow); - if (generation) attributesDbi.remove(generationRowKey(generation)); // That write, not the registration below, is the publish point: it is durable from here // (on LMDB releaseLock()'s finally commits this create's write transaction even while an // error unwinds), so any later throw must leave the catalog alone. Rolling back past it // would delete the attribute rows out from under a live primary row and leave every // thread loading the primary-only schema this change exists to prevent. published = true; + if (createJournal) attributesDbi.remove(generationRowKey(createJournal.generation)); setTable(tables, tableName, Table); } // a table with no declared primary key has no attribute row to carry relationships, and the @@ -4806,10 +5031,12 @@ function declareTable(target: TableTarget, tableDefinition: T } } function discardUnpublishedTable() { + let cleanupFailed = false; const discard = (description: string, action: () => unknown) => { try { action(); } catch (discardError) { + cleanupFailed = true; logger.warn( `Error discarding ${description} of the failed create of ${databaseName}.${tableName}`, discardError @@ -4825,13 +5052,15 @@ function declareTable(target: TableTarget, tableDefinition: T // an LMDB store is a per-environment handle slot shared with every thread and still inside this // create's write transaction; only RocksDB column-family handles hold native state to release if (rootStore instanceof RocksDatabase) { - if (generation) { - const suffix = '@' + generation; + if (createJournal) { + const suffix = '@' + createJournal.generation; + const stores = new Set(createJournal.creatingStores); for (const columnName of [...((rootStore as any).columns as string[])]) { - if (columnName.endsWith(suffix)) + if (stores.has(columnName) || columnName.endsWith(suffix)) discard(`store ${columnName}`, () => dropColumnFamily(rootStore, columnName)); } - discard('generation journal row', () => attributesDbi.remove(generationRowKey(generation))); + if (!cleanupFailed) + discard('generation journal row', () => attributesDbi.remove(generationRowKey(createJournal.generation))); } for (const indexName in Table?.indices ?? {}) discard(`index ${indexName}`, () => Table.indices[indexName].close()); @@ -5424,13 +5653,20 @@ function reclaimGenerations(rootStore: RocksDatabase, attributesDbi, databaseNam continue; } const live = attributesDbi.getSync(value.table + '/'); - if (value.phase === 'creating' && live?.generation === value.generation && !live.dropping) { + const publishedCreate = + live && + (typeof value.primaryStore === 'string' + ? storeNameFor(value.table + '/', live.generation) === value.primaryStore + : live.generation === value.generation); + if (value.phase === 'creating' && publishedCreate) { attributesDbi.remove(key); resetGenerationReclaimDelay(rootStore); continue; } const suffix = '@' + value.generation; - const retired = new Set(value.stores ?? []); + const retired = new Set( + value.phase === 'creating' ? (value.creatingStores ?? value.stores ?? []) : (value.stores ?? []) + ); const columns = [...((rootStore as any).columns as string[])]; if (value.phase === 'retired' && value.primaryStore && columns.includes(value.primaryStore)) { if (manageThreads.ownsStoreMaintenance(rootStore.path)) diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index 195b88f693..6c33b95271 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -3,6 +3,7 @@ require('../testUtils'); const assert = require('node:assert'); const fs = require('node:fs'); +const { randomUUID } = require('node:crypto'); const { setupTestDBPath } = require('../testUtils'); const { waitFor } = require('../waitFor'); const { @@ -12,6 +13,8 @@ const { resetDatabases, openRocksDatabase, setDroppedBlobSweepBatchMsForTesting, + getTableDrops, + markDropInProgress, } = require('#src/resources/databases'); const { createBlob, getFilePathForBlob } = require('#src/resources/blob'); const { logger } = require('#src/utility/logging/logger'); @@ -89,8 +92,15 @@ describe('dropTable generation-distinct stores', function () { setMainIsWorker(true); }); - it('stamps every physical store of a RocksDB table with the generation on its catalog row', async function () { + it('keeps first-time creates readable by legacy readers and stamps every store of a recreate', async function () { if (IS_LMDB) return this.skip(); + const First = defineTable('GenStamped'); + assert.equal(dbisDb().getSync('GenStamped/').generation, undefined); + assert.equal(First.primaryStore.name, 'GenStamped/'); + assert.equal(First.indices.str.name, 'GenStamped/str'); + await First.dropTable(); + resetDatabases(); + assert.deepStrictEqual(generationRows(), [], 'the recreate is stamped even after retirement finishes'); const Stamped = defineTable('GenStamped'); const generation = dbisDb().getSync('GenStamped/').generation; assert.match(generation, /^[0-9a-f-]{36}$/, 'the primary row carries a create-time generation'); @@ -110,6 +120,64 @@ describe('dropTable generation-distinct stores', function () { await Plain.dropTable(); }); + it('keeps local-only name history after reclamation without advertising a replicated drop', async function () { + if (IS_LMDB) return this.skip(); + const First = defineTable('GenLocalDrop'); + assert.equal(First.storageGeneration, undefined); + await First.dropTable({ localOnly: true }); + resetDatabases(); + assert.ok(dbisDb().getSync('/dropped/GenLocalDrop')); + assert.ok(!getTableDrops(TEST_DB).some(({ table }) => table === 'GenLocalDrop')); + assert.deepStrictEqual(generationRows(), []); + const Fresh = defineTable('GenLocalDrop'); + assert.match(Fresh.storageGeneration, /^[0-9a-f-]{36}$/); + assert.notEqual(Fresh.primaryStore.name, First.primaryStore.name); + await Fresh.dropTable(); + }); + + it('persists the local store name when a peer primary attribute carries a generation', async function () { + const PeerDefined = table({ + database: TEST_DB, + table: 'GenPeerStamp', + origin: 'cluster', + attributes: [{ name: 'id', isPrimaryKey: true, generation: randomUUID() }], + }); + assert.equal(dbisDb().getSync('GenPeerStamp/').generation, undefined); + await PeerDefined.put({ id: 1 }); + resetDatabases(); + const Reloaded = databases[TEST_DB].GenPeerStamp; + assert.equal((await Reloaded.get(1)).id, 1); + await Reloaded.dropTable(); + }); + + it('does not retire catalog stores after a schema listener completes the drop and recreates the table', async function () { + if (IS_LMDB) return this.skip(); + const First = defineTable('GenListenerRecreate'); + let Fresh; + const removeListener = schemaHandler.addListener((message) => { + if (message.operation === 'drop_table' && message.table === First.tableName && !Fresh) + Fresh = defineTable(First.tableName, [{ name: 'replacementOnly', type: 'String', indexed: true }]); + }); + try { + await First.dropTable({ localOnly: true }); + } finally { + removeListener(); + } + assert.ok(Fresh, 'the recreate runs before the old drop resumes from its broadcast'); + await Fresh.put({ id: 1, str: 'replacement' }); + assert.equal((await Fresh.get(1)).str, 'replacement'); + assert.ok(!generationRows().some(({ value }) => value.stores?.includes(Fresh.primaryStore.name))); + assert.ok( + !generationRows().some( + ({ value }) => + value.table === Fresh.tableName && + value.stores?.some((name) => name.startsWith(`${Fresh.tableName}/replacementOnly`)) + ), + 'the resumed drop must not journal the replacement catalog' + ); + await Fresh.dropTable(); + }); + it('serves nothing from a dropped generation through a same-name recreate, primary or index', async function () { const First = defineTable('GenRecreate'); await First.put({ id: 1, str: 'original' }); @@ -154,11 +222,10 @@ describe('dropTable generation-distinct stores', function () { if (IS_LMDB) return this.skip(); const Twice = defineTable('GenDoubleDrop'); await Twice.put({ id: 1, str: 'x' }); - const { generation } = dbisDb().getSync('GenDoubleDrop/'); const family = Twice.primaryStore.name; await Promise.all([Twice.dropTable(), Twice.dropTable()]); assert.deepStrictEqual(catalogRows('GenDoubleDrop'), []); - const journal = dbisDb().getSync(`${GENERATION_ROW_PREFIX}${generation}`); + const journal = generationRows().find(({ value }) => value.table === 'GenDoubleDrop')?.value; assert.ok(journal, 'the retirement row stays until a load confirms the reclaim'); assert.ok(journal.stores.includes(family), 'the second drop must not narrow the store list'); resetDatabases(); @@ -180,7 +247,6 @@ describe('dropTable generation-distinct stores', function () { it('does not dispose a same-name replacement for a delayed old-generation drop event', async function () { const First = defineTable('GenDelayedDrop'); - const oldGeneration = First.storageGeneration; const oldTableId = First.tableId; let broadcast; const removeListener = schemaHandler.addListener((message) => { @@ -193,7 +259,7 @@ describe('dropTable generation-distinct stores', function () { removeListener(); } assert.equal(broadcast.dropTableId, oldTableId); - if (!IS_LMDB) assert.equal(broadcast.dropGeneration, oldGeneration); + if (!IS_LMDB) assert.match(broadcast.dropGeneration, /^[0-9a-f-]{36}$/); const Replacement = defineTable('GenDelayedDrop'); await Replacement.put({ id: 1, str: 'replacement' }); await schemaHandler({ @@ -213,7 +279,7 @@ describe('dropTable generation-distinct stores', function () { operation: 'drop_table', schema: TEST_DB, table: 'GenDelayedDrop', - dropGeneration: oldGeneration, + dropGeneration: broadcast.dropGeneration, dropTableId: oldTableId, }, }); @@ -292,7 +358,7 @@ describe('dropTable generation-distinct stores', function () { await Doomed.put({ id, str: 'old', blob }); blobPaths.push(getFilePathForBlob((await Doomed.get(id)).blob)); } - const { generation } = dbisDb().getSync('GenCrashRetired/'); + const generation = randomUUID(); const family = Doomed.primaryStore.name; // the drop died after writing its journal row and removing the catalog rows, before the // physical drop landed: the family is still on disk under its name @@ -300,6 +366,7 @@ describe('dropTable generation-distinct stores', function () { table: 'GenCrashRetired', generation, phase: 'retired', + stores: [family, Doomed.indices.str.name], primaryStore: family, }); for (const key of catalogRows('GenCrashRetired')) dbisDb().removeSync(key); @@ -349,12 +416,81 @@ describe('dropTable generation-distinct stores', function () { assert.deepStrictEqual(generationRows(), []); }); + it('reclaims primary and index stores of an unpublished legacy create', function () { + const generation = randomUUID(); + const stores = ['GenBareCrash/', 'GenBareCrash/str']; + dbisDb().putSync(`${GENERATION_ROW_PREFIX}${generation}`, { + table: 'GenBareCrash', + generation, + phase: 'creating', + primaryStore: stores[0], + creatingStores: stores, + }); + for (const name of stores) { + const orphan = openRocksDatabase(rootStore().path, { name }); + orphan.putSync(1, { id: 1 }); + orphan.close(); + } + resetDatabases(); + assert.ok(stores.every((name) => !rootStore().columns.includes(name))); + assert.deepStrictEqual(generationRows(), []); + }); + + it('preserves a published legacy create whose journal removal was interrupted', async function () { + const Published = defineTable('GenBarePublished'); + assert.equal(Published.primaryStore.name, 'GenBarePublished/'); + await Published.put({ id: 1, str: 'published' }); + const generation = randomUUID(); + const row = { + table: Published.tableName, + generation, + phase: 'creating', + primaryStore: Published.primaryStore.name, + creatingStores: [Published.primaryStore.name, Published.indices.str.name], + }; + dbisDb().putSync(`${GENERATION_ROW_PREFIX}${generation}`, row); + resetDatabases(); + const Reloaded = databases[TEST_DB].GenBarePublished; + assert.equal((await Reloaded.get(1)).str, 'published'); + assert.deepStrictEqual(generationRows(), []); + await Reloaded.dropTable(); + }); + + it('leaves a published legacy family to the dropper when its create journal survived publication', async function () { + const Dropping = defineTable('GenPublishedDropping'); + await Dropping.put({ id: 1, str: 'owned by the dropper' }); + const generation = randomUUID(); + dbisDb().putSync(`${GENERATION_ROW_PREFIX}${generation}`, { + table: Dropping.tableName, + generation, + phase: 'creating', + primaryStore: Dropping.primaryStore.name, + creatingStores: [Dropping.primaryStore.name, Dropping.indices.str.name], + }); + const primary = dbisDb().getSync(`${Dropping.tableName}/`); + primary.dropping = true; + primary.dropGeneration = randomUUID(); + dbisDb().putSync(`${Dropping.tableName}/`, primary); + const releaseDrop = markDropInProgress(primary.dropGeneration); + try { + resetDatabases(); + assert.ok( + rootStore().columns.includes(Dropping.primaryStore.name), + 'create recovery must not retire a live drop' + ); + assert.equal(Dropping.primaryStore.getSync(1).str, 'owned by the dropper'); + } finally { + releaseDrop(); + resetDatabases(); + } + }); + it('journals an interrupted legacy catalog whose primary row carries its attribute name', async function () { const Legacy = defineTable('GenLegacyPrimary'); const primary = dbisDb().getSync('GenLegacyPrimary/'); - const { generation } = primary; + const generation = randomUUID(); const primaryKey = 'GenLegacyPrimary/id'; - const family = `${primaryKey}@${generation}`; + const family = primaryKey; Legacy.primaryStore.dropSync(); const legacyStore = openRocksDatabase(rootStore().path, { name: family }); legacyStore.putSync(1, { id: 1, str: 'legacy' }); @@ -379,9 +515,9 @@ describe('dropTable generation-distinct stores', function () { it('keeps a migrated bare tombstone until after its named primary descriptor is removed', async function () { const Migrated = defineTable('GenMigratedPrimary'); const bare = dbisDb().getSync('GenMigratedPrimary/'); - const { generation } = bare; + const generation = randomUUID(); const primaryKey = 'GenMigratedPrimary/id'; - const family = `${primaryKey}@${generation}`; + const family = primaryKey; Migrated.primaryStore.dropSync(); const migratedStore = openRocksDatabase(rootStore().path, { name: family }); migratedStore.putSync(1, { id: 1, str: 'migrated' }); @@ -407,6 +543,7 @@ describe('dropTable generation-distinct stores', function () { }); it('completes a tombstoned drop by exact store name, leaving a live same-name generation alone', async function () { + await defineTable('GenTombstoneExact').dropTable(); const Old = defineTable('GenTombstoneExact', [{ name: 'blob', type: 'Blob' }]); const blob = await createBlob(Buffer.alloc(50_000, 4)); await Old.put({ id: 1, str: 'old', blob }); From b909b30922304655e645099dc09b2b64eb1dc40f Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 08:04:04 -0600 Subject: [PATCH 2/7] Protect legacy replacement stores during generation reclamation Retirement journals can outlive a rollback that reuses bare store names. Preserve stores owned by the live catalog and their blobs, and verify the 5.2 recreate/re-upgrade path. Co-Authored-By: OpenAI Codex GPT-6.1 --- DESIGN.md | 6 ++- .../upgrade/first-create-downgrade.test.ts | 34 ++++++++++++++- resources/DESIGN.md | 5 +++ resources/Table.ts | 12 +++++- resources/databases.ts | 41 +++++++++++++++---- .../resources/dropTableGeneration.test.js | 33 +++++++++++++++ 6 files changed, 119 insertions(+), 12 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index e6bffae6a4..c8b2b18fb5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -12,8 +12,6 @@ Index of the design notes for the harper core: one line per note, grouped by the ## resources/ — records, transactions, tables, blobs, audit -- [First-time create names and recovery](resources/DESIGN.md#table-drops-the-dropping-tombstone-ghost-tables-and-lifecycle-stamps) — Legacy names preserve minor rollback readability; name history and create journals keep replacements and crash recovery separate. - - [File overview](resources/DESIGN.md#file-overview) — What each file in `resources/` owns. - [`Resource.ts` — base class](resources/DESIGN.md#resourcets--base-class) — Static entry points versus instance methods, and the `transactional()` wrapper. - [`Table.ts` — section map](resources/DESIGN.md#tablets--section-map) — Section markers for the `makeTable()` factory. @@ -47,7 +45,11 @@ Index of the design notes for the harper core: one line per note, grouped by the - [Subscription version selection (`Table.ts`)](resources/DESIGN.md#subscription-version-selection-tablets) — Replay and live delivery share primary-version filtering, reading the current entry once per audit record rather than per subscriber; messages remain independent and raw events preserve history by default. - [Audit-store `'committed'` notification batching (`transactionBroadcast.ts`)](resources/DESIGN.md#audit-store-committed-notification-batching-transactionbroadcastts) — `'committed'` notifications are deferred with `setImmediate`, collapsed per turn and batched with yields; the ancestor key walk must strictly shrink; a subscription ended mid-delivery leaves its key's array to the loop walking it, and every delivery loop is bounded by what it started with. - [`createBlob(readable)` and `table.put()` don't synchronously drain the source](resources/DESIGN.md#createblobreadable-and-tableput-dont-synchronously-drain-the-source) — `table.put()` returns before a `Readable`-backed blob has drained; finalize a hash on the stream's end or await `storageInfo.saving`. +<<<<<<< HEAD - [Table drops, the `dropping` tombstone, and ghost tables](resources/DESIGN.md#table-drops-the-dropping-tombstone-and-ghost-tables) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop. +======= +- [Table drops, the `dropping` tombstone, ghost tables, and lifecycle stamps](resources/DESIGN.md#table-drops-the-dropping-tombstone-ghost-tables-and-lifecycle-stamps) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop; `createdTime` and a durable `/dropped/
` marker let peers retire a generation created before a drop (harper#1212). First-time creates keep legacy names for rollback; recovery preserves names owned by a live table (harper#3102). +>>>>>>> b8fca4ec8 (Protect legacy replacement stores during generation reclamation) - [The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`)](resources/DESIGN.md#the-exclusive-update-attributes-lock-is-a-bounded-synchronous-wait-and-drop-then-recreate-needs-the-column-family-eviction-fix-tablets) — The lock is a bounded `Atomics.wait` with structural release, acquired before any live-`Table` mutation; drop-then-recreate needs the `@harperfast/rocksdb-js` column-family eviction fix. - [RocksDB transaction log purges are database-wide only (`ResourceBridge.deleteTransactionLogsBefore`)](resources/DESIGN.md#rocksdb-transaction-log-purges-are-database-wide-only-resourcebridgedeletetransactionlogsbefore) — RocksDB transaction logs are per database, so a table-scoped `delete_transaction_logs_before` is rejected with 400. - [Boot replay reads bypass the primary-record cache](resources/DESIGN.md#boot-replay-reads-bypass-the-primary-record-cache) — Snapshot reads avoid retaining the durable backlog through WeakRefs during synchronous boot replay. diff --git a/integrationTests/upgrade/first-create-downgrade.test.ts b/integrationTests/upgrade/first-create-downgrade.test.ts index 63cb67ae93..c004100fea 100644 --- a/integrationTests/upgrade/first-create-downgrade.test.ts +++ b/integrationTests/upgrade/first-create-downgrade.test.ts @@ -21,7 +21,14 @@ const previousPath = process.env.HARPER_PREVIOUS_MINOR_PATH?.split(delimiter).fi suite( 'first-time table create survives 5.2 rollback', - { skip: !previousPath || process.env.HARPER_RUNTIME === 'bun' || process.platform === 'win32', timeout: 300_000 }, + { + skip: + !previousPath || + process.env.HARPER_RUNTIME === 'bun' || + process.env.HARPER_STORAGE_ENGINE === 'lmdb' || + process.platform === 'win32', + timeout: 300_000, + }, (ctx: ContextWithHarper) => { after(async () => teardownHarper(ctx)); @@ -50,11 +57,25 @@ suite( await sendOperation(ctx.harper, { operation: 'upsert', schema, table: 'born_on_current', records }); } await sendOperation(ctx.harper, { operation: 'create_table', table: 'recreated', primary_key: 'id' }); + await sendOperation(ctx.harper, { operation: 'upsert', table: 'recreated', records: [{ id: 'retired' }] }); await sendOperation(ctx.harper, { operation: 'drop_table', table: 'recreated' }); await sendOperation(ctx.harper, { operation: 'create_table', table: 'recreated', primary_key: 'id' }); await sendOperation(ctx.harper, { operation: 'upsert', table: 'recreated', records: [{ id: 'stamped' }] }); + await sendOperation(ctx.harper, { operation: 'create_table', table: 'rollback_recreate', primary_key: 'id' }); + await sendOperation(ctx.harper, { + operation: 'upsert', + table: 'rollback_recreate', + records: [{ id: 'retired' }], + }); + await sendOperation(ctx.harper, { operation: 'drop_table', table: 'rollback_recreate' }); await killHarper(ctx); await startHarper(ctx, { config: {}, env: { CONFIRM_DOWNGRADE: 'yes' }, harperBinPath: previousBin }); + await sendOperation(ctx.harper, { operation: 'create_table', table: 'rollback_recreate', primary_key: 'id' }); + await sendOperation(ctx.harper, { + operation: 'upsert', + table: 'rollback_recreate', + records: [{ id: 'legacy-recreated' }], + }); const read = (schema: string | undefined, attribute = 'id', value = '*') => sendOperation(ctx.harper, { @@ -89,6 +110,17 @@ suite( } await killHarper(ctx); await startHarper(ctx, { config: {} }); + assert.deepStrictEqual( + await sendOperation(ctx.harper, { + operation: 'search_by_value', + table: 'rollback_recreate', + search_attribute: 'id', + search_value: '*', + get_attributes: ['id'], + }), + [{ id: 'legacy-recreated' }], + 'a retired journal must not reclaim the bare table recreated on 5.2' + ); for (const schema of schemas) { assert.deepStrictEqual(await read(schema), [...records, rollbackRecord]); assert.deepStrictEqual(await read(schema, 'category', 'current'), records); diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 6651aea659..245afe53b3 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -606,6 +606,7 @@ Consequence for callers that wrap the source in a hashing `Transform`: calling ` ## Table drops, the `dropping` tombstone, and ghost tables +<<<<<<< HEAD <<<<<<< HEAD A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the `__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore @@ -620,6 +621,8 @@ silently re-opened with create-if-missing on the next start, which resurrects "d ======= `dropTableGeneration.test.js` enforces legacy primary/index names for first-time RocksDB creates (harper#3102), distinct names for recreates even after local-only reclamation, and recovery via a separate create-journal identity; `creatingStores` must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published legacy primary. Previously stamped tables retain their names; replicated drop markers can also force a first local create to be stamped. +======= +>>>>>>> b8fca4ec8 (Protect legacy replacement stores during generation reclamation) A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the `__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore persists a `dropping: true` flag on the primary catalog entry (`T/`) before any destructive work, then drops the @@ -639,6 +642,8 @@ outlives a same-name recreate, only a newer drop overwrites it, the load parser `getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. `unitTests/resources/dropTableLifecycle.test.js`. >>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) +First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. + ## The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`) The create/schema-update path's exclusive `update-attributes` lock is a synchronous bounded wait diff --git a/resources/Table.ts b/resources/Table.ts index 10f19b5a28..8752aa319d 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3171,13 +3171,23 @@ export function makeTable(options): TableResourceClass { if (!droppedStores.has(columnName) && (rootStore as any).columns.includes(columnName)) dropColumnFamily(rootStore, columnName); } + const remainingPrimary = (dbisDb as any).getSync(primaryCatalogKey); + if ( + !remainingPrimary?.dropping || + (remainingPrimary.tableId != null && remainingPrimary.tableId !== tableId) || + remainingPrimary.dropGeneration !== dropGeneration + ) + return false; for (const key of dbisDb.getKeys({ start: tableName + '/', end: tableName + '0' })) { if (key !== primaryCatalogKey) dbisDb.remove(key); } dbisDb.remove(primaryCatalogKey); return true; }); - if (!removed) return false; + if (!removed) { + await sweepDroppedTableBlobs(primaryStore, `${databaseName}.${tableName}`); + return false; + } await dbisDb.committed; const label = `${databaseName}.${tableName}`; const settled = await settlePhysicalDrops(rootStore, label); diff --git a/resources/databases.ts b/resources/databases.ts index f82a0e7f41..c9889b213c 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -5031,16 +5031,17 @@ function declareTable(target: TableTarget, tableDefinition: T } } function discardUnpublishedTable() { - let cleanupFailed = false; + let storeDropFailed = false; const discard = (description: string, action: () => unknown) => { try { action(); + return true; } catch (discardError) { - cleanupFailed = true; logger.warn( `Error discarding ${description} of the failed create of ${databaseName}.${tableName}`, discardError ); + return false; } }; discard('catalog rows', () => { @@ -5057,9 +5058,9 @@ function declareTable(target: TableTarget, tableDefinition: T const stores = new Set(createJournal.creatingStores); for (const columnName of [...((rootStore as any).columns as string[])]) { if (stores.has(columnName) || columnName.endsWith(suffix)) - discard(`store ${columnName}`, () => dropColumnFamily(rootStore, columnName)); + if (!discard(`store ${columnName}`, () => dropColumnFamily(rootStore, columnName))) storeDropFailed = true; } - if (!cleanupFailed) + if (!storeDropFailed) discard('generation journal row', () => attributesDbi.remove(generationRowKey(createJournal.generation))); } for (const indexName in Table?.indices ?? {}) @@ -5627,6 +5628,20 @@ export async function sweepDroppedTableBlobs( return { failures, cancelled: Boolean(cancelled()), batches }; } +/** Older writers can reuse bare names without updating their retired journals. */ +function liveStoreNamesFor(attributesDbi, tableName: string): Set { + let primary = attributesDbi.getSync(tableName + '/'); + if (!primary) { + for (const { value } of attributesDbi.getRange({ start: tableName + '/', end: tableName + '0' })) { + if (value?.isPrimaryKey || value?.is_hash_attribute) { + primary = value; + break; + } + } + } + return new Set(primary && !primary.dropping ? storeNamesFor(attributesDbi, tableName, primary.generation) : []); +} + function reclaimGenerations(rootStore: RocksDatabase, attributesDbi, databaseName: string) { const rows = Array.from(attributesDbi.getRange({ start: GENERATION_ROW_PREFIX, end: GENERATION_ROW_END })); const state = scheduledGenerationReclaims.get(rootStore); @@ -5668,13 +5683,20 @@ function reclaimGenerations(rootStore: RocksDatabase, attributesDbi, databaseNam value.phase === 'creating' ? (value.creatingStores ?? value.stores ?? []) : (value.stores ?? []) ); const columns = [...((rootStore as any).columns as string[])]; - if (value.phase === 'retired' && value.primaryStore && columns.includes(value.primaryStore)) { + const owned = liveStoreNamesFor(attributesDbi, value.table); + if ( + value.phase === 'retired' && + value.primaryStore && + columns.includes(value.primaryStore) && + !owned.has(value.primaryStore) + ) { if (manageThreads.ownsStoreMaintenance(rootStore.path)) scheduleGenerationBlobSweep(rootStore, attributesDbi, databaseName, key, value); continue; } for (const columnName of columns) { - if (retired.has(columnName) || columnName.endsWith(suffix)) dropColumnFamily(rootStore, columnName); + if (!owned.has(columnName) && (retired.has(columnName) || columnName.endsWith(suffix))) + dropColumnFamily(rootStore, columnName); } if ((rootStore.getStats?.()?.['columnFamily.pendingReclaims'] ?? 0) > 0) { scheduleGenerationReclaim(rootStore, attributesDbi, databaseName); @@ -5729,8 +5751,10 @@ async function finishGenerationBlobSweep( } const suffix = '@' + latest.generation; const retired = new Set(latest.stores ?? []); + const owned = liveStoreNamesFor(attributesDbi, latest.table); for (const columnName of [...((rootStore as any).columns as string[])]) { - if (retired.has(columnName) || columnName.endsWith(suffix)) dropColumnFamily(rootStore, columnName); + if (!owned.has(columnName) && (retired.has(columnName) || columnName.endsWith(suffix))) + dropColumnFamily(rootStore, columnName); } if ((rootStore.getStats?.()?.['columnFamily.pendingReclaims'] ?? 0) === 0) { attributesDbi.remove(key); @@ -5759,7 +5783,8 @@ function scheduleGenerationBlobSweep( current?.phase !== 'retired' || current.generation !== row.generation || current.primaryStore !== row.primaryStore || - !((rootStore as any).columns as string[]).includes(row.primaryStore) + !((rootStore as any).columns as string[]).includes(row.primaryStore) || + liveStoreNamesFor(attributesDbi, row.table).has(row.primaryStore) ) return; let result: Awaited>; diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index 6c33b95271..c547b9a9b8 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -218,6 +218,39 @@ describe('dropTable generation-distinct stores', function () { ); }); + it('preserves bare stores and blobs recreated by a legacy writer despite a retired journal', async function () { + if (IS_LMDB) return this.skip(); + const First = defineTable('GenLegacyRecreate', [{ name: 'oldOnly', type: 'String', indexed: true }]); + await First.put({ id: 1, str: 'retired' }); + await First.dropTable(); + const retired = generationRows().find(({ value }) => value.table === First.tableName); + assert.ok(retired); + // 5.2 ignores the drop marker and journal when it chooses physical names. + dbisDb().removeSync(retired.key); + dbisDb().removeSync('/dropped/' + First.tableName); + const Fresh = defineTable(First.tableName, [{ name: 'blob', type: 'Blob' }]); + assert.equal(Fresh.primaryStore.name, First.tableName + '/'); + const blob = await createBlob(Buffer.alloc(50_000, 3)); + await Fresh.put({ id: 2, str: 'rollback', blob }); + const blobPath = getFilePathForBlob((await Fresh.get(2)).blob); + openRocksDatabase(rootStore().path, { name: First.tableName + '/oldOnly' }).close(); + dbisDb().putSync(retired.key, retired.value); + resetDatabases(); + await waitFor(() => !dbisDb().getSync(retired.key), { timeout: 15_000 }); + const Reloaded = databases[TEST_DB][First.tableName]; + assert.equal((await Reloaded.get(2)).str, 'rollback'); + assert.deepStrictEqual( + (await fromAsync(Reloaded.search({ conditions: [{ attribute: 'str', value: 'rollback' }] }))).map(({ id }) => id), + [2] + ); + assert.ok(fs.existsSync(blobPath), 'reclaiming an old journal must not unlink a live blob'); + assert.ok( + !rootStore().columns.includes(First.tableName + '/oldOnly'), + 'unowned retired stores still get reclaimed' + ); + await Reloaded.dropTable(); + }); + it('keeps the retirement journal complete under a redundant concurrent drop', async function () { if (IS_LMDB) return this.skip(); const Twice = defineTable('GenDoubleDrop'); From 0d84dfde19593db31a10fa12ef48e9a9b56aa200 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 08:27:56 -0600 Subject: [PATCH 3/7] Retire stale drop stores before completing a rollback-visible drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard retirement of the dropper’s own physical handles against live catalog ownership, then preserve write settlement and blob cleanup. Document untimed name history and older-reader crash recovery limits. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 6 +++++- resources/Table.ts | 16 +++++++++++----- resources/databases.ts | 7 ++++--- unitTests/resources/dropTableGeneration.test.js | 8 +++++++- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 245afe53b3..107b8a08c1 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -639,10 +639,14 @@ every completion path promotes to a `/dropped/
` row (`promoteTombstoneToD the tombstone — no second-write crash cut. Both come from `tableLifecycleTime()`, the record-version clock; `isDeadGeneration(createdTime, droppedTime)` is strict (equal survives, a missing stamp is 0). The marker outlives a same-name recreate, only a newer drop overwrites it, the load parser skips `/dropped/` rows, and +<<<<<<< HEAD `getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. `unitTests/resources/dropTableLifecycle.test.js`. >>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) +======= +`getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. On RocksDB, a completion without a drop time records untimed name history instead; `getTableDrops` and `pendingOrRecordedDropTime` skip that marker, and a timed drop overwrites it. `unitTests/resources/dropTableLifecycle.test.js`. +>>>>>>> 5f8f49e86 (Retire stale drop stores before completing a rollback-visible drop) -First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. +First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. During interrupted-drop recovery, a bare predecessor can remain until its asynchronous reclamation finishes; 5.2 ignores that recovery state and can expose predecessor rows through a recreate. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. ## The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`) diff --git a/resources/Table.ts b/resources/Table.ts index 8752aa319d..3d9d016f25 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -88,6 +88,7 @@ import { sweepDroppedTableBlobs, storeNameFor, storeNamesFor, + liveStoreNamesFor, isReadOnlyMode, } from './databases.ts'; import { notifyReplicatedApplyFailure } from './replicatedApplyFailure.ts'; @@ -3185,15 +3186,20 @@ export function makeTable(options): TableResourceClass { return true; }); if (!removed) { - await sweepDroppedTableBlobs(primaryStore, `${databaseName}.${tableName}`); - return false; - } - await dbisDb.committed; + withUpdateAttributesLock(rootStore, `retire stale stores of '${databaseName}.${tableName}'`, () => { + const owned = liveStoreNamesFor(dbisDb, tableName); + const columns = new Set((rootStore as any).columns); + for (const store of [primaryStore, ...Object.values(indices)]) { + if (store.name && columns.has(store.name) && !owned.has(store.name)) + dropColumnFamily(rootStore, store.name); + } + }); + } else await dbisDb.committed; const label = `${databaseName}.${tableName}`; const settled = await settlePhysicalDrops(rootStore, label); await sweepDroppedTableBlobs(primaryStore, label); if (!settled) finishDroppedTableBlobSweep(rootStore, primaryStore, label); - return true; + return removed; } finally { releaseDropMark(); } diff --git a/resources/databases.ts b/resources/databases.ts index c9889b213c..17fa19fa48 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -685,8 +685,9 @@ function dropMarkerStoreFor(databaseName: string): { rootStore: RootDatabaseKind * Read-compare-write of the newest drop time, serialized against every other writer: the catalog lock * on RocksDB (held by the caller when `exclusive` is false), a write transaction on LMDB. A live * tombstone is read again inside that section, so a drop that joined and raised its time cannot be - * promoted from a copy read earlier. A tombstone written before the stamps existed has no time, and no - * marker is synthesized for it: a time made up at completion could postdate a peer's live recreate. + * promoted from a copy read earlier. An untimed RocksDB marker records name history only; readers of + * replicated drop times skip it, and a timed drop overwrites it. Inventing a completion time could + * postdate a peer's live recreate. */ function writeTableDropMarker( rootStore: RootDatabaseKind, @@ -5629,7 +5630,7 @@ export async function sweepDroppedTableBlobs( } /** Older writers can reuse bare names without updating their retired journals. */ -function liveStoreNamesFor(attributesDbi, tableName: string): Set { +export function liveStoreNamesFor(attributesDbi, tableName: string): Set { let primary = attributesDbi.getSync(tableName + '/'); if (!primary) { for (const { value } of attributesDbi.getRange({ start: tableName + '/', end: tableName + '0' })) { diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index c547b9a9b8..407a2d468e 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -152,7 +152,10 @@ describe('dropTable generation-distinct stores', function () { it('does not retire catalog stores after a schema listener completes the drop and recreates the table', async function () { if (IS_LMDB) return this.skip(); - const First = defineTable('GenListenerRecreate'); + const First = defineTable('GenListenerRecreate', [{ name: 'blob', type: 'Blob' }]); + const blob = await createBlob(Buffer.alloc(50_000, 4)); + await First.put({ id: 1, str: 'retired', blob }); + const blobPath = getFilePathForBlob((await First.get(1)).blob); let Fresh; const removeListener = schemaHandler.addListener((message) => { if (message.operation === 'drop_table' && message.table === First.tableName && !Fresh) @@ -164,6 +167,9 @@ describe('dropTable generation-distinct stores', function () { removeListener(); } assert.ok(Fresh, 'the recreate runs before the old drop resumes from its broadcast'); + assert.ok(!rootStore().columns.includes(First.primaryStore.name), 'the stale drop retires its bare primary'); + assert.ok(!rootStore().columns.includes(First.indices.str.name), 'the stale drop retires its bare index'); + await waitFor(() => !fs.existsSync(blobPath), { timeout: 15_000 }); await Fresh.put({ id: 1, str: 'replacement' }); assert.equal((await Fresh.get(1)).str, 'replacement'); assert.ok(!generationRows().some(({ value }) => value.stores?.includes(Fresh.primaryStore.name))); From 3ef41d5321b92d44b1eb034a3318ba35be5f4cc4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 08:53:17 -0600 Subject: [PATCH 4/7] Verify rollback data survives a further current-version restart Reopen the storage engine before the final rollback-recreate and index assertions, so retained handles cannot mask an erroneously reclaimed family. Co-Authored-By: OpenAI Codex GPT-6.1 --- .../upgrade/first-create-downgrade.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/integrationTests/upgrade/first-create-downgrade.test.ts b/integrationTests/upgrade/first-create-downgrade.test.ts index c004100fea..6b97f18d27 100644 --- a/integrationTests/upgrade/first-create-downgrade.test.ts +++ b/integrationTests/upgrade/first-create-downgrade.test.ts @@ -110,14 +110,19 @@ suite( } await killHarper(ctx); await startHarper(ctx, { config: {} }); - assert.deepStrictEqual( - await sendOperation(ctx.harper, { + const readRecreated = () => + sendOperation(ctx.harper, { operation: 'search_by_value', table: 'rollback_recreate', search_attribute: 'id', search_value: '*', get_attributes: ['id'], - }), + }); + assert.deepStrictEqual(await readRecreated(), [{ id: 'legacy-recreated' }]); + await killHarper(ctx); + await startHarper(ctx, { config: {} }); + assert.deepStrictEqual( + await readRecreated(), [{ id: 'legacy-recreated' }], 'a retired journal must not reclaim the bare table recreated on 5.2' ); From 6e6e74cfe6a901cc946a9e64dd07223e35753dde Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 10:15:12 -0600 Subject: [PATCH 5/7] Keep overlapping create recovery behind retired-primary blob cleanup Snapshot journal phases under the catalog lock and defer creating-row primary retirement to any pending retired journal. Verify both journal orderings unlink blobs before retirement completes. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 2 +- resources/databases.ts | 14 +++++++- .../resources/dropTableGeneration.test.js | 32 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 107b8a08c1..adcfbc11f9 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -646,7 +646,7 @@ outlives a same-name recreate, only a newer drop overwrites it, the load parser `getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. On RocksDB, a completion without a drop time records untimed name history instead; `getTableDrops` and `pendingOrRecordedDropTime` skip that marker, and a timed drop overwrites it. `unitTests/resources/dropTableLifecycle.test.js`. >>>>>>> 5f8f49e86 (Retire stale drop stores before completing a rollback-visible drop) -First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. During interrupted-drop recovery, a bare predecessor can remain until its asynchronous reclamation finishes; 5.2 ignores that recovery state and can expose predecessor rows through a recreate. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. +First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; when a creating and retired journal name the same primary, the creating row leaves that primary to the retired row's blob sweep. Recovery reads their current phases under the catalog lock. Their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. During interrupted-drop recovery, a bare predecessor can remain until its asynchronous reclamation finishes; 5.2 ignores that recovery state and can expose predecessor rows through a recreate. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. ## The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`) diff --git a/resources/databases.ts b/resources/databases.ts index 17fa19fa48..9484760e4f 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -5660,7 +5660,18 @@ function reclaimGenerations(rootStore: RocksDatabase, attributesDbi, databaseNam return; } try { - for (const { key, value } of rows as Array<{ key: string; value: GenerationRow }>) { + const currentRows = Array.from( + attributesDbi.getRange({ start: GENERATION_ROW_PREFIX, end: GENERATION_ROW_END }) + ) as Array<{ key: string; value: GenerationRow }>; + const retiredPrimaries = new Set( + currentRows + .filter( + ({ value }) => + value?.phase === 'retired' && value.generation && value.table && typeof value.primaryStore === 'string' + ) + .map(({ value }) => value.primaryStore) + ); + for (const { key, value } of currentRows) { try { if (!value?.generation || !value.table) { logger.warn(`Removing a malformed generation journal row ${String(key)} in ${databaseName}`); @@ -5696,6 +5707,7 @@ function reclaimGenerations(rootStore: RocksDatabase, attributesDbi, databaseNam continue; } for (const columnName of columns) { + if (value.phase === 'creating' && retiredPrimaries.has(columnName)) continue; if (!owned.has(columnName) && (retired.has(columnName) || columnName.endsWith(suffix))) dropColumnFamily(rootStore, columnName); } diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index 407a2d468e..27b9bf3644 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -524,6 +524,38 @@ describe('dropTable generation-distinct stores', function () { } }); + for (const creatingFirst of [true, false]) { + it(`sweeps an interrupted drop before reclaiming its surviving create journal (${creatingFirst ? 'creating' : 'retired'} first)`, async function () { + const Doomed = defineTable(`GenOverlap${creatingFirst}`, [{ name: 'blob', type: 'Blob' }]); + assert.equal(Doomed.storageGeneration, undefined); + const blob = await createBlob(Buffer.alloc(50_000, creatingFirst ? 5 : 6)); + await Doomed.put({ id: 1, blob }); + const blobPath = getFilePathForBlob((await Doomed.get(1)).blob); + const ids = ['11111111-1111-4111-8111-111111111111', 'eeeeeeee-eeee-4eee-8eee-eeeeeeeeeeee']; + const [creatingId, retiredId] = creatingFirst ? ids : ids.toReversed(); + dbisDb().putSync(`${GENERATION_ROW_PREFIX}${creatingId}`, { + table: Doomed.tableName, + generation: creatingId, + phase: 'creating', + primaryStore: Doomed.primaryStore.name, + creatingStores: [Doomed.primaryStore.name, Doomed.indices.str.name], + }); + const primary = dbisDb().getSync(`${Doomed.tableName}/`); + primary.dropping = true; + primary.dropGeneration = retiredId; + dbisDb().putSync(`${Doomed.tableName}/`, primary); + resetDatabases(); + await waitFor( + () => + !dbisDb().getSync(`${GENERATION_ROW_PREFIX}${creatingId}`) && + !dbisDb().getSync(`${GENERATION_ROW_PREFIX}${retiredId}`), + { timeout: 15_000 } + ); + assert.ok(!fs.existsSync(blobPath), 'journal completion must follow blob unlinking'); + assert.ok(!rootStore().columns.includes(Doomed.primaryStore.name)); + }); + } + it('journals an interrupted legacy catalog whose primary row carries its attribute name', async function () { const Legacy = defineTable('GenLegacyPrimary'); const primary = dbisDb().getSync('GenLegacyPrimary/'); From c6ff208b5a1a2ec6e6c184df8c118ac44f2928a3 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 13:27:01 -0600 Subject: [PATCH 6/7] Resolve the v5.3 cherry-pick of #3118 without #2962's drop markers The conflict hunks pulled in main's #2962 lifecycle machinery (timed drop markers, createdTime stamps, replication helpers), which is milestoned v5.4 and absent here; it is dropped. #3118 keys "this name has history" off the /dropped/
row that #2962 writes at every drop completion, so v5.3 gets only the untimed form #3118 itself introduced: both RocksDB completion paths (retireRocksStores, completeInterruptedDrop) record it before removing the tombstone, and the load parser skips /dropped/ rows. 5.4 reads the same row as its untimed marker. The design notes keep v5.3's section heading and drop the lifecycle-stamp paragraph. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QYAaU3uVxkbjHe19UNWRfK Dispatch-Task: cherry-resolve-kriszyp_harper_3118-6e6e74cf --- DESIGN.md | 6 +- resources/DESIGN.md | 33 +-- resources/Table.ts | 2 + resources/databases.ts | 204 ++---------------- .../resources/dropTableGeneration.test.js | 37 +++- 5 files changed, 51 insertions(+), 231 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index c8b2b18fb5..16913fe455 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -45,11 +45,7 @@ Index of the design notes for the harper core: one line per note, grouped by the - [Subscription version selection (`Table.ts`)](resources/DESIGN.md#subscription-version-selection-tablets) — Replay and live delivery share primary-version filtering, reading the current entry once per audit record rather than per subscriber; messages remain independent and raw events preserve history by default. - [Audit-store `'committed'` notification batching (`transactionBroadcast.ts`)](resources/DESIGN.md#audit-store-committed-notification-batching-transactionbroadcastts) — `'committed'` notifications are deferred with `setImmediate`, collapsed per turn and batched with yields; the ancestor key walk must strictly shrink; a subscription ended mid-delivery leaves its key's array to the loop walking it, and every delivery loop is bounded by what it started with. - [`createBlob(readable)` and `table.put()` don't synchronously drain the source](resources/DESIGN.md#createblobreadable-and-tableput-dont-synchronously-drain-the-source) — `table.put()` returns before a `Readable`-backed blob has drained; finalize a hash on the stream's end or await `storageInfo.saving`. -<<<<<<< HEAD -- [Table drops, the `dropping` tombstone, and ghost tables](resources/DESIGN.md#table-drops-the-dropping-tombstone-and-ghost-tables) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop. -======= -- [Table drops, the `dropping` tombstone, ghost tables, and lifecycle stamps](resources/DESIGN.md#table-drops-the-dropping-tombstone-ghost-tables-and-lifecycle-stamps) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop; `createdTime` and a durable `/dropped/
` marker let peers retire a generation created before a drop (harper#1212). First-time creates keep legacy names for rollback; recovery preserves names owned by a live table (harper#3102). ->>>>>>> b8fca4ec8 (Protect legacy replacement stores during generation reclamation) +- [Table drops, the `dropping` tombstone, and ghost tables](resources/DESIGN.md#table-drops-the-dropping-tombstone-and-ghost-tables) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop. First-time creates keep legacy names for rollback; recovery preserves names owned by a live table (harper#3102). - [The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`)](resources/DESIGN.md#the-exclusive-update-attributes-lock-is-a-bounded-synchronous-wait-and-drop-then-recreate-needs-the-column-family-eviction-fix-tablets) — The lock is a bounded `Atomics.wait` with structural release, acquired before any live-`Table` mutation; drop-then-recreate needs the `@harperfast/rocksdb-js` column-family eviction fix. - [RocksDB transaction log purges are database-wide only (`ResourceBridge.deleteTransactionLogsBefore`)](resources/DESIGN.md#rocksdb-transaction-log-purges-are-database-wide-only-resourcebridgedeletetransactionlogsbefore) — RocksDB transaction logs are per database, so a table-scoped `delete_transaction_logs_before` is rejected with 400. - [Boot replay reads bypass the primary-record cache](resources/DESIGN.md#boot-replay-reads-bypass-the-primary-record-cache) — Snapshot reads avoid retaining the durable backlog through WeakRefs during synchronous boot replay. diff --git a/resources/DESIGN.md b/resources/DESIGN.md index adcfbc11f9..6ec3b1c895 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -606,8 +606,6 @@ Consequence for callers that wrap the source in a hashing `Transform`: calling ` ## Table drops, the `dropping` tombstone, and ghost tables -<<<<<<< HEAD -<<<<<<< HEAD A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the `__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore persists a `dropping: true` flag on the table's primary catalog entry (`T/`) before any @@ -618,35 +616,8 @@ dies or a drop fails partway, the tombstone survives; both the boot-time schema interrupted drop instead of resurrecting the table. Without this, surviving catalog rows are silently re-opened with create-if-missing on the next start, which resurrects "deleted" tables (with their data, if the column families were never actually removed). -======= -`dropTableGeneration.test.js` enforces legacy primary/index names for first-time RocksDB creates (harper#3102), distinct names for recreates even after local-only reclamation, and recovery via a separate create-journal identity; `creatingStores` must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published legacy primary. Previously stamped tables retain their names; replicated drop markers can also force a first local create to be stamped. - -======= ->>>>>>> b8fca4ec8 (Protect legacy replacement stores during generation reclamation) -A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows in the -`__dbis__` store, with no transaction spanning the two. `Table.dropTable()` therefore persists a -`dropping: true` flag on the primary catalog entry (`T/`) before any destructive work, then drops the -column families (awaited - a failed drop must surface as the operation's error), then removes the -catalog rows. If the process dies or a drop fails partway, the tombstone survives; the boot-time schema -load (`completeInterruptedDrop`) and a same-name `table()` create both complete the interrupted drop -instead of re-opening the surviving rows with create-if-missing, which resurrected "deleted" tables. - -**Lifecycle stamps (harper#1212).** The tombstone is node-local, so a peer offline for a replicated -`drop_table` would bring the table back through the schema handshake. Two durable facts give every node -one rule: the primary row carries `createdTime` from create (`declareTable`, kept from a peer's propagated -definition), and the tombstone carries `droppedTime` (`dropTable({ droppedTime })` applies a peer's), which -every completion path promotes to a `/dropped/
` row (`promoteTombstoneToDropMarker`) before removing -the tombstone — no second-write crash cut. Both come from `tableLifecycleTime()`, the record-version clock; -`isDeadGeneration(createdTime, droppedTime)` is strict (equal survives, a missing stamp is 0). The marker -outlives a same-name recreate, only a newer drop overwrites it, the load parser skips `/dropped/` rows, and -<<<<<<< HEAD -`getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. `unitTests/resources/dropTableLifecycle.test.js`. ->>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) -======= -`getTableDrops` / `recordTableDrop` / `onTableDropRecorded` serve replication. On RocksDB, a completion without a drop time records untimed name history instead; `getTableDrops` and `pendingOrRecordedDropTime` skip that marker, and a timed drop overwrites it. `unitTests/resources/dropTableLifecycle.test.js`. ->>>>>>> 5f8f49e86 (Retire stale drop stores before completing a rollback-visible drop) - -First-time RocksDB creates use bare primary/index names only when this node has no drop marker, no journal row for the table, and no `T/` column family (harper#3102). Any name history requires a generation stamp, including after local-only reclamation or a replicated drop on a node that never held the table. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; when a creating and retired journal name the same primary, the creating row leaves that primary to the retired row's blob sweep. Recovery reads their current phases under the catalog lock. Their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. During interrupted-drop recovery, a bare predecessor can remain until its asynchronous reclamation finishes; 5.2 ignores that recovery state and can expose predecessor rows through a recreate. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. + +**First-time creates and 5.2 rollback (harper#3102).** First-time RocksDB creates use bare primary/index names only when this node has no `/dropped/
` row, no journal row for the table, and no `T/` column family. Every RocksDB drop completion (`retireRocksStores` in `Table.ts`, `completeInterruptedDrop`) writes that untimed name-history row (`recordTableNameHistory`) before removing the tombstone, so any name history requires a generation stamp, including after reclamation. The load parser skips `/dropped/` rows; 5.4 reads the same row as its untimed drop marker (harper#2962), which a timed drop overwrites. Incoming primary-attribute generations are replaced by this node's physical naming choice. Create journals have their own identity; when a creating and retired journal name the same primary, the creating row leaves that primary to the retired row's blob sweep. Recovery reads their current phases under the catalog lock. Their `creatingStores` field must stay separate from `stores`, which shipped 5.3 readers destructively reclaim without recognizing a published bare primary. Reclamation always preserves stores owned by the live, non-dropping catalog and never sweeps that primary's blobs: a 5.2 writer can reuse bare names while ignoring the retired journal. Previously stamped tables retain their names and remain outside 5.2 rollback support, as do stamped recreates. During interrupted-drop recovery, a bare predecessor can remain until its asynchronous reclamation finishes; 5.2 ignores that recovery state and can expose predecessor rows through a recreate. `unitTests/resources/dropTableGeneration.test.js` enforces naming, crash recovery, and live-store ownership; `integrationTests/upgrade/first-create-downgrade.test.ts` exercises the real 5.2 round trip. ## The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`) diff --git a/resources/Table.ts b/resources/Table.ts index 3d9d016f25..d1b4d9792d 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -89,6 +89,7 @@ import { storeNameFor, storeNamesFor, liveStoreNamesFor, + recordTableNameHistory, isReadOnlyMode, } from './databases.ts'; import { notifyReplicatedApplyFailure } from './replicatedApplyFailure.ts'; @@ -3182,6 +3183,7 @@ export function makeTable(options): TableResourceClass { for (const key of dbisDb.getKeys({ start: tableName + '/', end: tableName + '0' })) { if (key !== primaryCatalogKey) dbisDb.remove(key); } + recordTableNameHistory(dbisDb, tableName, currentPrimary.tableId); dbisDb.remove(primaryCatalogKey); return true; }); diff --git a/resources/databases.ts b/resources/databases.ts index 9484760e4f..bd8207d057 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -650,204 +650,19 @@ export function recordRetiredGeneration( }); return merged; } -<<<<<<< HEAD -======= const DROPPED_ROW_PREFIX = '/dropped/'; -const DROPPED_ROW_END = '/dropped0'; -const LIFECYCLE_STEP = 0.001; -export interface TableDropMarker { - table: string; - droppedTime: number; - tableId?: number; -} -/** - * The record-version clock, floored strictly above `after`: a stamp synthesized here for a fact that - * follows another (a recreate after a known drop, a drop of a generation stamped by a faster peer clock) - * must order after it even when this node's clock has not caught up. - */ -export function tableLifecycleTime(after?: number): number { - const now = getNextMonotonicTime(); - return Number.isFinite(after) && now <= after ? after + LIFECYCLE_STEP : now; -} -export function isDeadGeneration(createdTime: number | undefined, droppedTime: number): boolean { - return (Number.isFinite(createdTime) ? createdTime : 0) < droppedTime; -} function droppedRowKey(tableName: string): string { return DROPPED_ROW_PREFIX + tableName; } -function dropMarkerStoreFor(databaseName: string): { rootStore: RootDatabaseKind; attributesDbi: any } | undefined { - if (!databases[databaseName]) return; - const rootStore = database({ database: databaseName, table: null }); - const attributesDbi = (rootStore as any)?.dbisDb; - return attributesDbi ? { rootStore, attributesDbi } : undefined; -} /** - * Read-compare-write of the newest drop time, serialized against every other writer: the catalog lock - * on RocksDB (held by the caller when `exclusive` is false), a write transaction on LMDB. A live - * tombstone is read again inside that section, so a drop that joined and raised its time cannot be - * promoted from a copy read earlier. An untimed RocksDB marker records name history only; readers of - * replicated drop times skip it, and a timed drop overwrites it. Inventing a completion time could - * postdate a peer's live recreate. + * Untimed name history: once a name has been dropped, a later create takes a generation even after the + * retirement journal is reclaimed (harper#3102). 5.4 reads this row as its untimed drop marker, which a + * timed drop overwrites. RocksDB callers hold the catalog lock and write it before removing the tombstone. */ -function writeTableDropMarker( - rootStore: RootDatabaseKind, - attributesDbi, - databaseName: string, - tableName: string, - droppedTime: number | undefined, - tableId: number | undefined, - exclusive: boolean, - tombstoneKey?: string -): boolean { +export function recordTableNameHistory(attributesDbi, tableName: string, tableId: number | undefined): void { const key = droppedRowKey(tableName); - const write = () => { - if (tombstoneKey) { - const live = attributesDbi.getSync(tombstoneKey); - if (live?.dropping && Number.isFinite(live.droppedTime) && !(droppedTime >= live.droppedTime)) { - droppedTime = live.droppedTime; - tableId = live.tableId ?? tableId; - } - } - if (!Number.isFinite(droppedTime)) { - if (rootStore instanceof RocksDatabase && tombstoneKey && !attributesDbi.getSync(key)) - attributesDbi.putSync(key, { table: tableName, tableId }); - return false; - } - const existing: TableDropMarker | undefined = attributesDbi.getSync(key); - if (existing && existing.droppedTime >= droppedTime) return false; - const marker: TableDropMarker = { table: tableName, droppedTime }; - if (tableId != null) marker.tableId = tableId; - attributesDbi.putSync(key, marker); - return true; - }; - let written: boolean; - if (rootStore instanceof RocksDatabase) { - written = exclusive - ? withUpdateAttributesLock(rootStore, `drop marker for '${databaseName}.${tableName}'`, write) - : write(); - } else { - written = (rootStore as any).transactionSync(write); - } - // Committed by here on both engines; emitted outside the caller's lock because listeners re-send - // schemas, which read this catalog. - if (written) - queueMicrotask(() => { - try { - databaseEventsEmitter.emit('tableDropRecorded', databaseName, tableName); - } catch (error) { - logger.warn(`A tableDropRecorded listener failed for ${databaseName}.${tableName}`, error); - } - }); - return written; -} -export function getTableDrops(databaseName: string): TableDropMarker[] { - const store = dropMarkerStoreFor(databaseName); - if (!store) return []; - const markers: TableDropMarker[] = []; - for (const { value } of store.attributesDbi.getRange({ start: DROPPED_ROW_PREFIX, end: DROPPED_ROW_END })) { - if (value && typeof value.table === 'string' && Number.isFinite(value.droppedTime)) markers.push(value); - } - return markers; -} -/** A peer's drop of a table this node does not hold; false when the database is unknown here or the marker is not newer. */ -export function recordTableDrop(databaseName: string, tableName: string, droppedTime: number): boolean { - if (!Number.isFinite(droppedTime)) return false; - const store = dropMarkerStoreFor(databaseName); - if (!store) return false; - return writeTableDropMarker( - store.rootStore, - store.attributesDbi, - databaseName, - tableName, - droppedTime, - undefined, - true - ); -} -export function onTableDropRecorded(listener: (databaseName: string, tableName: string) => void) { - databaseEventsEmitter.on('tableDropRecorded', listener); - return { - remove() { - databaseEventsEmitter.off('tableDropRecorded', listener); - }, - }; + if (!attributesDbi.getSync(key)) attributesDbi.putSync(key, { table: tableName, tableId }); } -/** The primary catalog row of a live table, under either key layout. */ -function primaryCatalogRowFor(table: { - dbisDB: any; - tableName: string; - primaryKey?: string; -}): { key: string; value: any } | undefined { - const bareKey = table.tableName + '/'; - const bare = table.dbisDB?.getSync(bareKey); - if (bare) return { key: bareKey, value: bare }; - if (!table.primaryKey) return; - const legacyKey = `${table.tableName}/${table.primaryKey}`; - const legacy = table.dbisDB?.getSync(legacyKey); - return legacy?.isPrimaryKey ? { key: legacyKey, value: legacy } : undefined; -} -/** The stamp on disk, which a class loaded on another thread may not carry yet. */ -export function catalogCreatedTime(table: { dbisDB: any; tableName: string; primaryKey?: string }): number | undefined { - const createdTime = primaryCatalogRowFor(table)?.value?.createdTime; - return Number.isFinite(createdTime) ? createdTime : undefined; -} -/** - * Backfills the stamp of a generation created on a build that stored none, once a peer's stamped definition - * proves which generation it is. Only an absent stamp is written; false when one exists or the table is gone. - */ -export function stampTableCreatedTime( - table: { dbisDB: any; tableName: string; primaryKey?: string; primaryStore: any; createdTime?: number }, - createdTime: number -): boolean { - if (!Number.isFinite(createdTime)) return false; - const rootStore = table.primaryStore?.rootStore; - const attributesDbi = table.dbisDB; - if (!rootStore || !attributesDbi) return false; - const write = () => { - const row = primaryCatalogRowFor(table); - if (!row || row.value.dropping || Number.isFinite(row.value.createdTime)) return false; - row.value.createdTime = createdTime; - attributesDbi.putSync(row.key, row.value); - return true; - }; - const written: boolean = - rootStore instanceof RocksDatabase - ? withUpdateAttributesLock(rootStore, `stamp '${table.tableName}'`, write) - : (rootStore as any).transactionSync(write); - if (written) table.createdTime = createdTime; - return written; -} -/** The time a drop of this name carries for peers: its marker, or the live tombstone of a drop still completing. */ -export function pendingOrRecordedDropTime(databaseName: string, tableName: string): number | undefined { - const store = dropMarkerStoreFor(databaseName); - if (!store) return; - const marker: TableDropMarker | undefined = store.attributesDbi.getSync(droppedRowKey(tableName)); - const tombstone = store.attributesDbi.getSync(tableName + '/'); - const pending = tombstone?.dropping ? tombstone.droppedTime : undefined; - const candidates = [marker?.droppedTime, pending].filter((time) => Number.isFinite(time)) as number[]; - return candidates.length ? Math.max(...candidates) : undefined; -} -/** Right before a completion path removes the live tombstone at `tombstoneKey`; RocksDB callers hold the catalog lock. */ -export function promoteTombstoneToDropMarker( - rootStore: RootDatabaseKind, - attributesDbi, - databaseName: string, - tableName: string, - tombstoneKey: string, - tombstone: { droppedTime?: number; tableId?: number } | undefined -): void { - writeTableDropMarker( - rootStore, - attributesDbi, - databaseName, - tableName, - tombstone?.droppedTime, - tombstone?.tableId, - false, - tombstoneKey - ); -} ->>>>>>> 4db030f2a (Keep first-time table stores readable after a minor rollback) export function storeNamesFor(attributesDbi, tableName: string, generation: string | undefined): string[] { const names: string[] = []; for (const key of attributesDbi.getKeys({ start: tableName + '/', end: tableName + '0' })) { @@ -1491,7 +1306,8 @@ function initStores( for (const result of attributesDbi.getRange({ start: false })) { const { key, value } = result as { key: string; value: any }; if (value == null) continue; - if (typeof key === 'string' && key.startsWith(GENERATION_ROW_PREFIX)) continue; + if (typeof key === 'string' && (key.startsWith(GENERATION_ROW_PREFIX) || key.startsWith(DROPPED_ROW_PREFIX))) + continue; let [tableName, attribute_name] = key.toString().split('/'); if (attribute_name === '') { // primary key @@ -6004,7 +5820,11 @@ function completeInterruptedDrop( // catalog-removal failure would bypass the retry accounting entirely. (attributesDbi as any).removeSync(key); } - if (tombstoneEntry) (attributesDbi as any).removeSync(tombstoneEntry.key); + if (tombstoneEntry) { + if (rootStore instanceof RocksDatabase) + recordTableNameHistory(attributesDbi, tableName, tombstoneEntry.value?.tableId); + (attributesDbi as any).removeSync(tombstoneEntry.key); + } return true; } diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index 27b9bf3644..c072e3321b 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -13,11 +13,11 @@ const { resetDatabases, openRocksDatabase, setDroppedBlobSweepBatchMsForTesting, - getTableDrops, markDropInProgress, } = require('#src/resources/databases'); const { createBlob, getFilePathForBlob } = require('#src/resources/blob'); const { logger } = require('#src/utility/logging/logger'); +const { forComponent } = require('#src/utility/logging/harper_logger'); const { getPlaneBinding } = require('#src/resources/indexes/hnswPlaneBinding'); const { derivedIndexReadiness } = require('#src/resources/indexes/hnswDerivedIndex'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); @@ -120,14 +120,13 @@ describe('dropTable generation-distinct stores', function () { await Plain.dropTable(); }); - it('keeps local-only name history after reclamation without advertising a replicated drop', async function () { + it('keeps local-only name history after reclamation', async function () { if (IS_LMDB) return this.skip(); const First = defineTable('GenLocalDrop'); assert.equal(First.storageGeneration, undefined); await First.dropTable({ localOnly: true }); resetDatabases(); assert.ok(dbisDb().getSync('/dropped/GenLocalDrop')); - assert.ok(!getTableDrops(TEST_DB).some(({ table }) => table === 'GenLocalDrop')); assert.deepStrictEqual(generationRows(), []); const Fresh = defineTable('GenLocalDrop'); assert.match(Fresh.storageGeneration, /^[0-9a-f-]{36}$/); @@ -556,6 +555,38 @@ describe('dropTable generation-distinct stores', function () { }); } + it('keeps name history from a drop completed at load after its journal is reclaimed', async function () { + const Doomed = defineTable('GenLoadCompleted'); + assert.equal(Doomed.storageGeneration, undefined); + const primary = dbisDb().getSync('GenLoadCompleted/'); + primary.dropping = true; + primary.dropGeneration = randomUUID(); + dbisDb().putSync('GenLoadCompleted/', primary); + resetDatabases(); + await waitFor(() => !generationRows().some(({ value }) => value.table === 'GenLoadCompleted'), { + timeout: 15_000, + }); + assert.deepStrictEqual(catalogRows('GenLoadCompleted'), []); + assert.ok(dbisDb().getSync('/dropped/GenLoadCompleted')); + const storageLogger = forComponent('storage'); + const { warn, debug } = storageLogger; + const skipped = []; + // the first skip of a keyless catalog warns, a repeat logs at debug + storageLogger.warn = storageLogger.debug = (message) => { + if (String(message).startsWith('Skipping table')) skipped.push(message); + }; + try { + resetDatabases(); + } finally { + storageLogger.warn = warn; + storageLogger.debug = debug; + } + assert.deepStrictEqual(skipped, [], 'the load parser skips name-history rows'); + const Fresh = defineTable('GenLoadCompleted'); + assert.match(Fresh.storageGeneration, /^[0-9a-f-]{36}$/); + await Fresh.dropTable(); + }); + it('journals an interrupted legacy catalog whose primary row carries its attribute name', async function () { const Legacy = defineTable('GenLegacyPrimary'); const primary = dbisDb().getSync('GenLegacyPrimary/'); From 8e83d2db95019e0bcea3daa74fe0c097f0b836f7 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 15:32:48 -0600 Subject: [PATCH 7/7] Drop v5.3-ignored localOnly drop arguments; state the name-history row contract v5.3's dropTable() takes no options, so the picked tests' localOnly argument only repeated the ordinary drop; the test now names what it checks. The recordTableNameHistory comment states the row-shape constraint instead of issue history. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QYAaU3uVxkbjHe19UNWRfK Dispatch-Task: cherry-resolve-kriszyp_harper_3118-6e6e74cf --- resources/databases.ts | 7 ++++--- unitTests/resources/dropTableGeneration.test.js | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/resources/databases.ts b/resources/databases.ts index bd8207d057..9806c7c40f 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -655,9 +655,10 @@ function droppedRowKey(tableName: string): string { return DROPPED_ROW_PREFIX + tableName; } /** - * Untimed name history: once a name has been dropped, a later create takes a generation even after the - * retirement journal is reclaimed (harper#3102). 5.4 reads this row as its untimed drop marker, which a - * timed drop overwrites. RocksDB callers hold the catalog lock and write it before removing the tombstone. + * Once a name has been dropped, later creates of it take a generation even after the retirement journal is + * reclaimed. Keep the row untimed and shaped `{ table, tableId }`: later versions read it as an untimed drop + * marker that a timed drop overwrites. RocksDB callers hold the catalog lock and write it before removing + * the tombstone. */ export function recordTableNameHistory(attributesDbi, tableName: string, tableId: number | undefined): void { const key = droppedRowKey(tableName); diff --git a/unitTests/resources/dropTableGeneration.test.js b/unitTests/resources/dropTableGeneration.test.js index c072e3321b..6385082b5b 100644 --- a/unitTests/resources/dropTableGeneration.test.js +++ b/unitTests/resources/dropTableGeneration.test.js @@ -120,11 +120,11 @@ describe('dropTable generation-distinct stores', function () { await Plain.dropTable(); }); - it('keeps local-only name history after reclamation', async function () { + it('keeps a /dropped/ name-history row after reclamation', async function () { if (IS_LMDB) return this.skip(); const First = defineTable('GenLocalDrop'); assert.equal(First.storageGeneration, undefined); - await First.dropTable({ localOnly: true }); + await First.dropTable(); resetDatabases(); assert.ok(dbisDb().getSync('/dropped/GenLocalDrop')); assert.deepStrictEqual(generationRows(), []); @@ -161,7 +161,7 @@ describe('dropTable generation-distinct stores', function () { Fresh = defineTable(First.tableName, [{ name: 'replacementOnly', type: 'String', indexed: true }]); }); try { - await First.dropTable({ localOnly: true }); + await First.dropTable(); } finally { removeListener(); }