Keep a table being created invisible to catalog scans on other threads, so replication can never announce a partial attribute list

[kriszyp]

A table being created is now invisible to catalog scans on other worker threads until its catalog is complete: table() writes the primary-key row (<table>/, the row initStores needs before it will load a table) after every attribute row, and treats that write — not the class registration that follows it — as the point the table is published, and initStores skips a table whose catalog has attribute rows but no primary row (and no longer counts it as defined, so a worker still holding a dropped same-name generation evicts it, disposing it, instead of serving it through the recreate). A create that throws from the moment its primary store opens is registered nowhere and releases what it had opened — the audit delete-removal callback, its storage-reclamation handler (through a new per-handler removeStorageReclamationHandler, because a RocksDB column family shares its reclamation path with the whole database), and on RocksDB the primary and index column-family handles. The invariant and its crash semantics are recorded in DESIGN.md. This closes the harper-pro cluster failure replicate across many databases → {"error":"unknown attribute 'name'"} (red on the sync-core PR HarperFast/harper-pro#775) at its source.

Root cause, from the failing job's server-log artifact. Every worker thread rebuilds its own Table map by scanning the __dbis__ catalog (resetDatabases() → initStores) on any schema-change ITC signal, including one for an unrelated database. On RocksDB the catalog rows are individual putSync writes and the cross-thread update-attributes lock is taken only by writers, so on node 127.0.0.4 the http/1 thread — rescanning because main/0 had just signalled db6 — landed inside main/0's create_table db7.test {id, name} (10:43:40.316–.361), built db7.test with attributes: [id], and emitted updateTable; harper-pro forwarded that snapshot to its peers over the system connection. Node 127.0.0.2's replication thread had not yet loaded db7 (its own main/0 create signalled at .380, the thread processed it at .453), so at .397 it logged (Re)creating { table: 'test', schemaDefined: true, attributes: [ { name: 'id', type: 'ID', isPrimaryKey: true } ], database: 'db7' } and applied it through ensureTable → table() as an authoritative definition, whose removal reconcile deleted the local name descriptor. create_table had already returned success; every later complete announcement hit the schemaDefined honor-local guard (Schema for 'db7.test' is defined locally, but attribute 'name: String' from '127.0.0.4' does not match local attribute which does not exist, 6–7 times per node), and search_by_value on name failed.

This is not a regression of the 30db07ac..9a24b055 core range. The same test failed identically on harper-pro main on 2026-08-21 (runs 32506365300 and 32457825567) before any commit in that range existed, and the only commit that touches this surface, #2258, is inert for a caller that does not pass origin: 'cluster' — nothing on harper-pro main does; that half of the fix is the open HarperFast/harper-pro#750. It is the pre-existing race that #2258 was written against (its message names this flake); harper-pro's CI hits it in roughly a third of runs, and the local oracle reproduces it deterministically once the create window is widened (see Verification). The receiver-side additive rule (#2258 + #750) contains the damage from a partial snapshot; this change stops the snapshot from being produced, which also covers every other consumer of the scanning thread's Table.attributes (describe, REST/GraphQL validation, MQTT on that worker).

For the human reviewer

1. Write ordering rather than an atomic RocksDB write batch (catalog-ordering-vs-write-batch). The planning-mode review returned Framing-Verdict: better-alternative-exists: stage the new table's catalog rows (NEXT_TABLE_ID, attribute rows, primary row, reconcile removals) into one RocksTransaction and commitSync() it under the lock, as Table.ts already does for the replication cursor. I chose ordering because the guarantee the bug needs — no thread can ever build or announce a partial Table — is identical under both; the batch's extra benefit is confined to the catalog rows of a crashed create (a create that merely throws now removes its own rows), while the column families opened before a crash are orphaned either way. Cost side: the batch is RocksDB-only and has to be threaded through five catalog write sites inside a loop shared with the existing-table path, plus abort handling; the ordering change is one deferred put that also holds on LMDB, where exclusiveLock() already is an environment-wide write transaction. Reversible: the batch can be layered on later without touching the loader rule. A "no" means rewriting the new-table catalog writes as a staged transaction before merge.
2. An interrupted create is an absent table, not a partial one (interrupted-create-semantics, skip-vs-salvage-partial-catalog). initStores refuses to load a table whose catalog has attribute rows but no primary row; the alternative is to derive the primary from an attribute row carrying isPrimaryKey, which the loader still accepts for pre-5.x catalogs. So a create that crashes mid-way leaves orphan attribute rows and the column families opened before the crash (unchanged), and the table does not load — one warn per table per thread, then debug — until create_table is re-run, which writes the rows, reuses the families, and reconciles orphan rows away regardless of origin (rows found under the lock for a table with no primary row can only be aborted state). Previously it loaded as a table missing declared attributes. Salvaging would re-open the partial-announcement hole for any writer that persists a primary-key attribute row before the marker. Operators of API-only create_table tables see the new failure shape; @table-declared tables self-heal on redeploy.
3. Rollback scope of a create that throws (rollback-scope, publish-point). The class is published only with the primary row, so a create that throws after the primary store opens is rolled back and the retry rebuilds from scratch (re-running makeTable(), consuming another NEXT_TABLE_ID): the attribute rows it wrote are removed, Table.cleanup() releases the audit delete-removal callback and its store slot, the storage-reclamation handler (new per-handler removeStorageReclamationHandler — a RocksDB column family shares its reclamation path with the whole database), the TTL cleanup timer and the expiresAt eviction interval (with a disposed guard so a run in flight does not re-arm), and on RocksDB the primary and index column-family handles are closed; makeTable() releases its own registrations when it throws, and openIndex() closes a RocksDB handle whose format resolution or custom-index constructor throws. On LMDB the stores are left alone: an LMDB store is a per-environment handle slot shared with every thread and still inside the creator's write transaction (closing it there fails the transaction with MDB_BAD_DBI, which the failure-injection test caught). The alternative — publishing early with a "loading" flag so the failing worker can repair in place — was rejected because nothing in the create loop looks the table up by name, and it would give the "no partial table anywhere" property back. Past the publish point nothing unwinds the catalog: published flips immediately after the primary-row write, before the class is registered. The row is durable from there — on LMDB releaseLock()'s finally commits this create's write transaction even while an error unwinds — so a throw from the registration or from the relationships-persistence block must leave the rows alone; rolling back there deletes the attribute rows while the primary row stays committed, which is the primary-only schema this PR exists to prevent. A class the registration never accepted is unreachable, so it is released through Table.cleanup() (reclamation handler, audit delete-removal callback, TTL timer, expiresAt interval). Declined: its RocksDB primary and index handles are left open. Unlike the pre-publish rollback, where nothing else can be holding the table, the table is durable here and another thread may already be serving it, so closing its column families from the failing thread risks invalidating a handle in use; the price is a bounded native-handle leak on a path that only fires if the databases map itself rejects the assignment.
4. Index registration timing (index-registration-timing). An unpublished class registers a newly opened index before its descriptor persists so the rollback can close it; an existing table keeps the original post-persistence registration, so a failed descriptor write during add_attribute leaves its live index map unchanged. A reviewer may prefer a local list of opened handles for the rollback instead of the early registration.
5. Evicted classes are now disposed, not just unlinked (cleanup-on-class-eviction, dispose-via-existing-static-cleanup, evict-stale-generation-during-create-window). The rescan cleanup pass used to drop a class whose table (or database) was dropped elsewhere with a bare delete, leaving its audit delete-removal callback, reclamation handler, and timers registered for the life of the process; it now calls the class's cleanup() first. That method's meaning broadened from "remove the audit callback" to an irreversible dispose (timers cleared, any reclamation pass awaiting it settled) — it has no callers in core or harper-pro today, so the name was kept; renaming to dispose() is cheap now and expensive once harper-pro pins a core commit. A component that captured tables.X and keeps using it across a drop/recreate now holds a disposed class instead of a stale-but-armed one pointing at dropped column families. And a worker whose rescan finds the recreate's incomplete catalog drops the generation it still holds a few ms early — "table not found" instead of "stale reads" on that worker for the create window — which is the behaviour the thread test pins. Settling the reclamation pass that awaits a scheduled cleanup is now the scan's job, not the scheduling's: when a later scheduleCleanup() supersedes a pending pass, the replacement adopts its awaiters and settles them when its own scan completes, and runs immediately whenever it adopts any — runReclamationHandlers blocks its whole path on that promise, and the replacement's own slot can be a full interval out. Settling without a scan is left only where no scan can follow: scanInterval 0, dispose, and a closed root store. No focused test drives a supersede while a reclamation run awaits the pass — that promise is reachable only through runReclamationHandlers(), which walks every path registered in the process, so a test for it would drive every other table's cleanup scan as a side effect; the ordering was verified by tracing the serialized scan chain.
6. A create discards every row it finds for its table (create-treats-every-existing-row-as-aborted), even for attribute names it redeclares — where to look hardest: this reconcile is now also active for cluster-origin creates, which origin: 'cluster' otherwise exempts — clean-slate semantics, at the cost of an aborted attempt's lastIndexedKey/indexFormat, so a retry rebuilds an index from scratch rather than resuming. Cheap to soften later.
7. The replication proof stops at the core event boundary (replication-proof-stops-at-the-core-event-boundary). The worker-thread test asserts no updateTable fires mid-create, which is the boundary this repo owns; peer forwarding lives in harper-pro, and the executed harper-pro oracle runs below are the end-to-end evidence. Nothing here would catch a future decoupling of updateTable from peer forwarding.
8. The rollback closes column families but does not drop them (rollback-closes-but-does-not-drop). A retry reuses the families the failed attempt opened (the same remnant fallback an interrupted drop relies on), so a retry with different compression/codec options inherits the aborted attempt's family settings. Dropping them would be a dropSync() under the same lock; left as is because the retry path is the one openRocksDatabase already documents for remnant families.
9. Interrupted-catalog warn is latched per table per thread (warn-latch-for-process-lifetime). First observation warns with the attribute names, later scans log at debug until the table loads; the state needs an operator to re-run create_table, and a warn per rescan on every thread (a multi-KB line for a wide table) was the alternative. Trivially reversible.
10. Pre-existing gaps, not fixed here — worth their own issues. (a) A worker still holding a class from a dropped same-name table that receives a direct table() call (not a rescan) takes the existing-table branch and mutates the stale class; it never revalidates the class against the durable primary descriptor under the lock. (b) initStores runs the interrupted-drop reconcile from an unlocked catalog snapshot and completeInterruptedDrop never re-reads the tombstone under the lock, so a thread that read a dropping row before another thread completed the drop and recreated the table can delete the new generation's column families and rows. (c) dropDatabase() deletes a database's table map without disposing the classes in it, so a create/drop-database cycle retains every table's reclamation handler, audit callback and timers for the life of the process — the rescan path is disposed here, the direct drop is not. (d) The de-index reconcile resolves its dbi as Table.indices[attributeTableName] while that map is keyed by attribute name, so it is always undefined and a de-indexed attribute's column family is never dropped (on the create path the map is also still empty there, so the key alone would not reclaim an aborted attempt's index family). All four predate this change; the rescan half of (a) is covered here.
11. Rolling upgrades. The invariant holds only once every schema-creating node runs this code — an older peer still announces primary-first snapshots — so the receiver-side rule (Make cluster-origin table definitions additive-only so a peer's partial schema snapshot cannot destroy locally declared attributes #2258, and Apply peer table definitions additively so a partial DB_SCHEMA snapshot cannot destroy locally declared attributes harper-pro#750 which is still open) stays necessary and this is not a substitute for merging [Models] Derived caching table for @embed (low-latency writes + model-change backfill) #750. Stated in DESIGN.md.

Verification

• unitTests/resources/createTableCatalogOrder.test.js (new), run on RocksDB and on LMDB: (a) records the catalog write order for a create with a primary key and two attributes and asserts <table>/ is the last row; (b) creates a table with a relationship attribute and asserts the row written at the publish point already carries the normalized relationship list, is the last catalog write, and is not rewritten by the relationship-persistence block that follows it (it fails — two primary-row writes — with the assignment of the relationships onto the deferred row disabled); (c) injects a throw on the tag attribute-row write (its index store is already open) and asserts the class is not registered and the primary row was never written, then retries and asserts the class, every attribute, the index, and the primary row; (d) on RocksDB, spawns a worker thread that first loads a dropped generation of the same name, then scans while the recreate is paused after its first attribute row — asserting the table is not loaded (neither generation), no updateTable was emitted, and, as a positive control, that the attribute row was readable and the primary row was not — then rescans after the create returns and asserts [id, name, tag] and the primary row (skipped on LMDB, where the scan blocks on the creator's write transaction instead of observing the catalog); and (e) injects a failure at the publish boundary — a throwing setter on databases.test[table], so the create fails at exactly the registration step — then asserts the unreachable class released its reclamation handler, that every catalog row survived, and that resetDatabases() reloads a table with all three attributes whose primary store and tag index both serve traffic. (e) fails on both engines with published set after setTable() instead of before it (the 'name' row must survive a failure after the publish point). (a) and (d) fail on origin/main (CatalogOrderTest/,CatalogOrderTest/name,CatalogOrderTest/tag; a scan during the create must not load the table, got attributes id,name) with a forced rebuild between.
• npm run test:unit:resources: green on RocksDB (1794 passing) and LMDB (1501 passing); unitTests/server/storageReclamation.test.js green. npm run test:unit:main: 5112 passing, 1 failing — configValidator … does not warn when a relative rootPath resolves within the limit, which asserts path.resolve('relative/root', 'operations-server') stays under the 107-byte socket limit from process.cwd(); this worktree's cwd is 99 bytes, so it fails on origin/main from here too. (components … nonInteractiveSpawn git credential scoping also fails from a shell that exports GIT_CONFIG_GLOBAL; this run used env -u GIT_CONFIG_GLOBAL -u GIT_EDITOR and it passes.) npm run test:integration:all (run at bf65b069, not re-run for the publish-point commits, which only move a flag and add an error-path release): 1888 pass, 0 fail, 6 cancelled — integrationTests/server/ollama-backend.test.ts dies at import under this machine's Node v26.2 (ERR_IMPORT_ATTRIBUTE_MISSING on json/systemSchema.json), unrelated. The cleanup-scheduling change was re-checked against the eviction/TTL integration files it can reach — integrationTests/database/eviction-*.test.ts, ttl-*.test.ts, apiTests/ttl.test.ts: 40 pass, 0 fail.
• End-to-end route: harper-pro integrationTests/cluster/replicationLoad.test.mjs in a harper-pro worktree (origin/main, own core submodule), npm run build between every switch. Unfixed core (ad9854be), plain: passed once — the race is timing-dependent (~1 in 3 CI runs). Unfixed core + a temporary, uncommitted 150 ms Atomics.wait after the primary row put (widening the mid-create window): fails with {"error":"unknown attribute 'name'"}; node logs show two partial (Re)creating … attributes: [id] announcements per node and 6–7 defined locally … does not exist errors per node — the CI artifact's signature. Fixed core (this branch's final head) + the same 150 ms wait moved to just before the deferred primary row put: passes 4/4; node logs show zero partial announcements, zero defined locally errors, and three Skipping table test: its catalog has attribute rows but no primary key row warns across two nodes — scans did land inside creates and skipped them. Fixed core, plain: 4/4, zero partial announcements.

Refs HarperFast/harper-pro#775, HarperFast/harper-pro#750, #2258

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini(auth); declined=cursor-grok,cursor-composer,domain; rounds=10 @ e6ec3b8}

_{Human-Review-Need: 3 @ e6ec3b8}

[gemini-code-assist]

Code Review

This pull request ensures that a table remains invisible to catalog scans until its creation is complete by deferring the write of the primary key descriptor until after all attribute rows are written. It also implements robust cleanup mechanisms for timers, intervals, and storage reclamation handlers when a table is disposed or fails to create. The feedback suggests adding a defensive check for removedTables in resources/databases.ts to prevent a potential TypeError if the database entry is undefined.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Barbarian reviewed bf65b06 and found 1 blocking issue. The durable primary row is written before the table is marked published. If synchronous registration or event delivery throws, rollback can corrupt the catalog and leave a disposed registered class. Mark the table published immediately after the primary-row write.

[cb1kenobi]

Reviewed e6ec3b8 and found no blocking issues. No confirmed blocking defects remain on the changed lines. Previously raised publish-ordering and reclamation issues are addressed at this commit.

—
Generated by Barber AI