fix(core): complete catalogue functions on startup; fix flaky covering-index fuzz op

[bluestreak01]

This PR carries two code fixes that both surfaced from CI on this branch, plus a small documentation change. Each is in its own clearly-labeled section below.

---

1. fix(core): complete catalogue & metadata lookups during startup hydration

Problem

The catalogue functions return a snapshot() of MetadataCache. That cache is populated lazily on a background thread by MetadataCache.onStartupAsyncHydrator(). A catalogue query that runs before hydration finishes — right after a restart, or after a backup restore brings a server up — observes an empty or partial table list and silently misses tables (and their columns).

This was caught by the enterprise cross-repo CI for this PR: BackupTest.testFailedRestoreParallelTaskFailure ran select table_name from tables on a freshly-started server and got an empty result because the query beat the async hydrator by a few milliseconds (metadata hydration completed was logged ~6 ms after the query executed).

Root cause

MetadataCacheReader.snapshot() only copies tables already present in the cache's tableMap. The startup hydrator fills that map entry-by-entry on a background thread, so until it completes the snapshot is incomplete. The catalogue functions did no on-demand reconciliation against the authoritative table registry.

Fix — catalogue enumerators (tables / all_tables / columns / pg_attribute)

• Add MetadataCache.hydrateAllTables(): reconciles the cache against the table registry (engine.getTableTokens) and hydrates any missing tables on demand. Under a single read lock it collects tables that exist in the registry but are missing from the cache, then hydrates them with a per-table write lock (matching onStartupAsyncHydrator). Views are skipped (no _meta).
• Call it before snapshotting in all four snapshot() callers — tables(), all_tables(), information_schema.columns(), and pg_catalog.pg_attribute — which via cursor-factory reuse also covers pg_attribute's prefixed variant and information_schema.questdb_columns.

Fix — point-lookup paths (SHOW COLUMNS / SHOW CREATE TABLE / SHOW CREATE MATERIALIZED VIEW / parquet pruning)

The same root cause hits single-table lookups that resolve a TableToken from the synchronously-loaded registry but then read the lazily-hydrated cache.

• Add MetadataCache.hydrateTableOnDemand(token), the point-lookup analogue of hydrateAllTables(): if the cache is not yet complete and the one table is missing, hydrate just that table under a write lock (best-effort; a per-table failure is swallowed so the caller reports it missing rather than failing hard).
• Wire it into the paths that resolve-then-read: SHOW COLUMNS (ShowColumnsRecordCursorFactory), SHOW CREATE TABLE (ShowCreateTableRecordCursorFactory), SHOW CREATE MATERIALIZED VIEW (ShowCreateMatViewRecordCursorFactory), and the parquet row-group-pruning probe AbstractPartitionFrameCursorFactory.hasParquetFormatPartitions().
• Without this, during the hydration window those paths would report a registered table/matview as non-existent, or — for parquet — silently skip row-group pruning (a correctness-neutral but performance-relevant regression).

Reconcile machinery: cacheComplete latch & concurrency

• hydrateAllTables()/hydrateTableOnDemand() short-circuit on a single volatile cacheComplete flag, so once the cache is known complete they add no per-query work. The flag is latched when a reconcile observes every registered table present, when the startup hydrator finishes hydrating every table, or when the give-up budget below is exhausted; it is reset by clearCache() (checkpoint/restore, tests).
• Mutual exclusion with clearCache(): the completeness latch is published under the read lock that observed the cache as complete, so it cannot interleave with clearCache() (write lock) and leave an emptied cache marked complete. Pinned by MetadataCacheTest.testCompletenessLatchIsMutuallyExclusiveWithClearCache and a concurrent stress test.
• Same-pass latch: when a reconcile hydrates the last missing table it latches cacheComplete in that same pass (re-checking the token snapshot it already collected, under the read lock) instead of leaving the flag off and forcing the next catalogue query to run a second, redundant full reconcile just to observe "nothing missing".

Give-up budget and its tradeoff

A table whose _meta cannot be read (genuinely corrupt, or transiently unavailable at startup) never lands in the cache and so always looks "missing"; without a cap it would be re-read — and logged CRITICAL — on every catalogue query forever.

• MAX_INCOMPLETE_RECONCILE_PASSES = 8: after that many consecutive zero-progress reconcile rounds, hydrateAllTables() gives up and latches cacheComplete anyway.
• Tradeoff: a genuinely unhydratable table is then hidden from the catalogue until a writer touches it (registerName/hydrateTable re-hydrate it) or the next clearCache()/restart. This matches master's worst case — master never reconciles, so such a table is hidden there too — in exchange for bounded work and log output.
• The budget counts only zero-progress rounds: any round that hydrates ≥1 previously-missing table resets it, so a post-restart introspection storm making incremental progress (e.g. catalogue queries racing the startup hydrator) cannot exhaust it in a burst.

Native memory hygiene

The reconcile / on-demand paths hydrate on the caller's thread, acquiring a thread-local Path (NATIVE_PATH_THREAD_LOCAL). They release it via Path.clearThreadLocals() after hydrating — mirroring onStartupAsyncHydrator() — so a short-lived (non-pooled) caller thread does not leak the native buffer. (CI caught the leak via ViewCompilerJobTest.testConcurrentEventProcessing's memory-leak assertion, which spawns and joins worker threads that hit these paths.)

Steady-state cost

The reconcile is only needed while the one-shot startup hydration is in flight. Once it completes, writers keep the cache in sync incrementally, so hydrateAllTables() short-circuits on a single volatile flag — no allocation, no lock, the snapshot() version fast-path is untouched. If the startup pass aborts abnormally the flag stays unset and the reconcile keeps running as a self-healing fallback.

Impact (Postgres Wire)

The column enumerators are the higher-impact path: PostgreSQL drivers and BI tools query them automatically on connect. pg_class enumerates tables straight from the registry, so during the window pg_class is complete while pg_attribute is not — the standard pg_class -> pg_attribute introspection join would render existing tables with zero columns, breaking driver schema views and autocomplete right after a restart.

Regression tests

Catalogue enumerators:

• TablesFunctionFactoryTest.testCatalogueIsCompleteBeforeStartupHydration — tables() / all_tables() return the complete set in the pre-hydration window.
• InformationSchemaColumnsFunctionFactoryTest.testColumnsCompleteBeforeStartupHydration — same for information_schema.columns() (which also backs pg_attribute); verified it fails with a partial column list when the reconcile is removed.

Point-lookup paths:

• MetadataCacheTest.testShowColumnsBeforeStartupHydration, MetadataCacheTest.testHydrateTableOnDemandPopulatesMissingTable.
• ShowCreateTableTest.testShowCreateTableBeforeStartupHydration, ShowCreateTableTest.testShowCreateMatViewBeforeStartupHydration (materialized view).
• ParquetRowGroupPruningTest.testRowGroupPruningSurvivesEmptyMetadataCacheWindow — evicts the table from the cache, then asserts a filtered query still skips row groups (pruning is applied via on-demand hydrate).

Reconcile machinery / latch / budget:

• MetadataCacheTest.testHydrateAllTablesShortCircuitsWhenCacheComplete — pins the steady-state fast path (no-op once complete, even if the cache was cleared).
• MetadataCacheTest.testReconcileLatchesCompleteAfterHydratingMissingTables — one reconcile both hydrates and latches (no redundant second pass).
• MetadataCacheTest.testReconcileBudgetCountsOnlyZeroProgressRounds — progress rounds do not consume the give-up budget.
• MetadataCacheTest.testReconcileGivesUpAfterRepeatedHydrationFailures — give up after MAX_INCOMPLETE_RECONCILE_PASSES zero-progress rounds.
• MetadataCacheTest.testStartupHydratorDoesNotLatchCompleteWhenATableFailsToHydrate, testCompletenessLatchIsMutuallyExclusiveWithClearCache, testConcurrentReconcileAndClearCacheNeverMarksEmptyCacheComplete — never latch over an incomplete/emptied cache.

---

2. test(core): fix flaky covering-index fuzz op on stale WAL metadata

Problem

ReplicationFuzzTest.testPrimaryMigration (and other fuzz tests that enable addCoveringIndexProb) intermittently fail with:

java.lang.RuntimeException: io.questdb.griffin.SqlException:
  [48] indexes are only supported for symbol type [column=new_col_12, type=BYTE]
    at io.questdb.test.fuzz.FuzzAddCoveringIndexOperation.executePostDrain(FuzzAddCoveringIndexOperation.java:112)
    at io.questdb.test.cairo.fuzz.FuzzRunner.executeCoveringIndexOps(FuzzRunner.java:1211)
Caused by: io.questdb.griffin.SqlException: [48] indexes are only supported for symbol type [column=new_col_12, type=BYTE]
    at io.questdb.griffin.SqlCompilerImpl.alterTableColumnAddIndex(SqlCompilerImpl.java:1176)

The SQL engine is behaving correctly here — you genuinely cannot index a non-symbol column. The bug is in the fuzz operation.

Root cause

FuzzAddCoveringIndexOperation.executePostDrain:

1. reads CairoEngine.getTableMetadata() and validates the target column by index is a SYMBOL, then takes that column's name;
2. issues ALTER TABLE ... ALTER COLUMN "<name>" ADD INDEX TYPE POSTING INCLUDE (...).

For WAL tables these two reads can see different metadata versions:

• getTableMetadata() is, by its own contract, the lagging applied metadata (not all WAL transactions have necessarily reached the table yet).
• ALTER ... ADD INDEX validates against SqlExecutionContext.getMetadataForWrite() → getLegacyMetadata() → getSequencerMetadata(), i.e. the fresh write-side metadata.

When the workload recycles a column name (drop a SYMBOL column / rename so a name is reused by a non-symbol column), the operation can observe the name as SYMBOL in the lagging metadata while the name already resolves to a non-symbol (e.g. BYTE) column in the sequencer metadata. The emitted ALTER then fails, and "indexes are only supported for symbol type" was not in the operation's swallowed-error set, so it propagated and failed the test.

This was seen on a CI run where the fuzz workload heavily renamed columns, recycling the name new_col_12 onto a BYTE column before the covering-index op ran.

Fix

Tolerate this transient metadata skew, consistent with the other races the operation already swallows (already indexed / does not exist / table busy / dropped table). Skipping the operation under this race is the established pattern for the fuzz infrastructure.

Regression test

Added FuzzAddCoveringIndexOperationTest which reproduces the skew deterministically — no fuzz seed, no servers, no concurrency (~1s): create a WAL table with a SYMBOL column, drop + re-add it as BYTE through the sequencer without draining, then invoke executePostDrain. The applied metadata still reports the column as SYMBOL while the sequencer metadata sees BYTE, reproducing the exact failure.

• Before the fix: test errors with the exact CI exception (executePostDrain:112 → alterTableColumnAddIndex:1176).
• After the fix: Tests run: 1, Failures: 0, Errors: 0.

---

3. docs: PR-bundling workflow guidance (CLAUDE.md)

Documentation-only; no code impact. Adds guidance to CLAUDE.md to bundle related fixes onto one branch/PR (CI is the throughput bottleneck, and the squash-merge collapses the branch into one master commit anyway), to give each fix its own clearly-labeled section in the PR body, and how to update a PR's branch/metadata when asked to "send a change to PR #N". Kept on this branch deliberately: it is small, merges/reverts with no code coupling, and it codifies the very multi-fix workflow this PR follows.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eb621f9a-95fe-4fae-bd70-c427e7c8d80d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-covering-index-fuzz-stale-metadata

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[bluestreak01]

/azp run macwin

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[bluestreak01]

Review — fix(core): complete catalogue functions on startup; fix flaky covering-index fuzz op

Reviewed at full depth (change-surface map, parallel correctness/concurrency + adversarial + cross-context passes, per-finding source verification). Verdict up front: approve with minor changes — no Critical defects survived verification.

What was verified correct (high-signal, since this is concurrency-heavy)

• No deadlock on the non-reentrant SimpleReadWriteLock. Every new method acquires/releases read and write locks sequentially, never nested; all 8 production callsites invoke snapshot()/hydrateTableOnDemand() outside any metadata-cache lock (hasParquetFormatPartitions runs at code-gen, not under a lock).
• The cacheComplete latch is genuinely mutually exclusive with clearCache(). Every cacheComplete=true publish happens under the read or write lock; clearCache() resets it under the exclusive write lock, which spin-waits for all readers. An emptied cache can never be marked complete. The two dedicated concurrency tests pin this.
• The stale tableTokens snapshot in latchCacheCompleteIfWarmed is not a regression: TableNameRegistryRW.registerName() hydrates a new table into the cache before publishing its token to the registry, so any token a later scan could see is already cached. This is the same invariant master relies on.
• Memory visibility (volatile cacheComplete fast path + lock-guarded tableMap) and version++/snapshot interaction are sound; spurious version bumps only force benign re-scans.
• Fuzz fix verified end-to-end: getTableMetadata() returns lagging applied metadata (its javadoc says so), while ALTER…ADD INDEX validates against fresh getSequencerMetadata(), so the SYMBOL→BYTE name-reuse skew is real and the swallow is correctly scoped. The new test reproduces it deterministically. Production validation still throws.

Moderate

M1 — In-code comment overstates the give-up budget's concurrency immunity (MetadataCache.java:60-86, :299-318). In-diff.
incompleteReconcilePasses counts zero-progress invocations, not wall-clock rounds. The field comment claims "a post-restart storm … cannot exhaust it in a single concurrent burst." That holds only while progress is being made. Under a total no-progress fault (one table's _meta transiently unreadable, all others cached), every concurrent hydrateAllTables() is zero-progress and bumps the counter under the write lock, so a burst of ≥8 concurrent tables()/information_schema.columns/pg_attribute queries — exactly the PG-driver introspection storm this PR targets — can spend the budget in milliseconds and latch cacheComplete=true with that table still absent, until a writer touches it or clearCache()/restart.
This is not a regression (master never reconciles, so it hides such a table immediately and indefinitely; the PR strictly improves on that with up-to-8 self-heal retries), and it is bounded. But the self-heal window is weaker than the comment implies. Recommend either tightening the comment to scope the guarantee to progress-making bursts, or making the budget round/time-based (or never latching while the cache is empty).

M2 — SHOW CREATE VIEW for plain views has the identical resolve-then-read-cache race and is left unfixed (ShowCreateViewRecordCursorFactory.java:132-137). Out-of-diff, pre-existing.
It reads metadataRO.getTable(viewToken) and throws "view does not exist" on a cache miss. Plain views (isView()==true) have no _meta, are never hydrated by the startup hydrator, and are added only by the async ViewCompilerJob — so during the startup window SHOW CREATE VIEW can report a registered view as missing, the very bug fixed for tables/matviews. hydrateTableOnDemand cannot help (it no-ops on views). This is genuinely out of the chosen mechanism's scope and lower-impact, but the PR fixes its two siblings (SHOW CREATE TABLE/MATERIALIZED VIEW) and leaves this asymmetry silent. A real fix would drop the cache-based existence check (the definition is already read from disk via ViewDefinition.readFrom). At minimum, call it out as a known remaining gap so it's a conscious decision.

Minor

• Member ordering (MetadataCache.java). Non-final fields cacheComplete (:99) and latchCompleteTestHook (:105) sit after metaMem, breaking the file's alphabetical ordering (should be cacheComplete, columnVersionReader, incompleteReconcilePasses, latchCompleteTestHook, metaMem, …). Likewise the new public methods hydrateAllTables/hydrateTableOnDemand/isCacheComplete/setLatchCompleteTestHook are placed after onStartupAsyncHydrator instead of in alphabetical position (h/i/s vs o). snapshot() is correctly placed.
• Path.clearThreadLocals() now fires on pooled query/compile worker threads (slow paths of snapshot()/hydrateTableOnDemand()), where previously only the dedicated startup-hydrator thread called it. No use-after-free is demonstrable today (immediate callers re-fetch getThreadLocal() after the call; the code-gen parquet probe holds no Path across it), and it is needed to satisfy the leak check on genuinely short-lived caller threads. But it is a thread-wide side effect on a shared hot path that is merely wasteful (not leak-preventing) on pooled threads and creates latent fragility for any future caller that retains a thread-local Path across a catalogue/SHOW/parquet probe. Worth a comment documenting the constraint, or scoping the clear to non-pooled threads.
• No single-flight coalescing during the startup-window storm. N concurrent catalogue callers each allocate an ObjHashSet, re-run engine.getTableTokens(), and re-scan — and per-table writeLock() bumps version unconditionally (O(N·M)), which under writer-priority stalls concurrent snapshots and defeats their priorVersion fast-return. Disk I/O is correctly de-duplicated via skipIfCached. Transient and self-terminating; a reasonable follow-up, not a blocker.
• No direct pg_attribute startup-window test. It is fixed and covered transitively via the shared snapshot() path that InformationSchemaColumnsFunctionFactoryTest exercises, but the PR's stated motivation is the pg_class→pg_attribute driver join — a direct test would lock in the user-visible scenario.
• Stale test javadoc. FuzzAddCoveringIndexOperationTest says it "is RED against the current operation and should go GREEN once executePostDrain tolerates…", but the fix ships in the same PR.
• Labels. Consider adding Postgres Wire and Materialized View given the change's own impact sections.

Downgraded (dismissed after source verification)

• "Give-up budget is a blocker / regression." Dismissed as a blocker: master has no reconcile at all, so its always-behavior equals the PR's give-up worst case; the PR adds self-healing master lacks. Kept only as the M1 comment-accuracy note.
• "Path.clearThreadLocals() causes a use-after-free on worker threads." No reachable caller retains a thread-local Path across the call — the code-gen parquet probe's locals are the cursor factory/model, and the SHOW factories re-fetch Path.getThreadLocal() after hydrating. Kept only as the Minor fragility note.
• "Completeness latch can fire over a stale name→token entry (drop+recreate same name)." Pre-existing: the reader's snapshot() corrects on tableId mismatch and writer drop/hydrate paths keep tableMap consistent; not worsened by this PR.
• "Parquet test relies on a process-global static counter (rowGroupsSkipped)." It is the existing pattern in that test class and the repo forbids parallel mvn, so it is safe under sequential execution.

Summary

• Verdict: approve (request the two Moderate items be addressed or consciously deferred). No Critical findings.
• Regressions/tradeoffs: none that worsen master. The give-up latch (M1) is bounded and strictly better than master; the plain-view gap (M2) is pre-existing.
• In-diff vs out-of-diff: 1 Moderate + all Minors in-diff; 1 Moderate (M2 SHOW CREATE VIEW) out-of-diff (pre-existing). The cross-context pass confirmed the catalogue-enumerator and point-lookup callsite sets are handled completely for everything that can use the disk-hydrate mechanism (pg_class, views(), mat_views() correctly read from the registry/graph, not the cache).

The locking design and reconcile/latch machinery are carefully built and unusually well-tested for a concurrency change; the substantive asks are documentation accuracy (M1) and acknowledging the plain-view gap (M2).

[bluestreak01]

Fixed: M2 — SHOW CREATE VIEW had the same resolve-then-read-cache race (now closed)

Follow-up to the SHOW CREATE TABLE / MATERIALIZED VIEW startup-window fixes. SHOW CREATE VIEW for plain views carried the identical defect and was previously left unfixed.

Root cause. ShowCreateViewRecordCursorFactory.of() resolved the view token from the synchronously loaded table registry (SqlParserCallback.getViewToken) but then gated on the lazily hydrated metadata cache, throwing view does not exist on a cache miss. Plain views:

• have no _meta file,
• are skipped by the startup hydrator (hydrateTableStartup / hydrateAllTables / latchCacheCompleteIfWarmed all continue on isView()),
• and are a no-op for hydrateTableOnDemand() (if (token == null || token.isView()) return;).

They are only ever cached by the async ViewCompilerJob (metadataRW.hydrateTable(viewMetadata)). So during the startup / embedded window — or after any clearCache() — a registered view was reported as missing. The hydrate-on-demand mechanism used for the two siblings fundamentally cannot apply here (no _meta to read).

Fix. Existence and view-ness are already guaranteed upstream by getViewToken, and the DDL is read straight from disk via ViewDefinition.readFrom. The metadata-cache read is now best-effort: it only preserves the TableReferenceOutOfDateException staleness guard when the view is already warm, and no longer fails on a miss.

try (MetadataCacheReader metadataRO = ...readLock()) {
    this.view = metadataRO.getTable(viewToken);
    if (this.view != null && !viewToken.equals(view.getTableToken())) {
        throw TableReferenceOutOfDateException.of(viewToken);
    }
}

Test. Added ViewsFunctionTest.testShowCreateViewBeforeStartupHydration, which creates a view, evicts the metadata cache via the clearCache() seam (same approach as the table/matview regression tests), and asserts SHOW CREATE VIEW still returns the DDL. It is red before / green after this change.

Validation: ViewsFunctionTest 9/9 and ShowCreateTableTest 47/47 pass. testShowCreateViewFail3 (the genuine "view does not exist" case) still passes — that error correctly originates in getViewToken (registry resolution), not the factory.

With this, all three of SHOW CREATE TABLE / MATERIALIZED VIEW / VIEW survive the pre-hydration window; the asymmetry is closed.

Commit: a84b6e7

[bluestreak01]

/azp run macwin

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mtopolnik]

[PR Coverage check]

😍 pass : 88 / 88 (100.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/griffin/engine/functions/catalogue/AllTablesFunctionFactory.java	1	1	100.00%
🔵	io/questdb/cairo/AbstractPartitionFrameCursorFactory.java	1	1	100.00%
🔵	io/questdb/cairo/MetadataCache.java	79	79	100.00%
🔵	io/questdb/griffin/engine/table/ShowCreateTableRecordCursorFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/functions/catalogue/InformationSchemaColumnsFunctionFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/functions/catalogue/PgAttributeFunctionFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/table/ShowCreateViewRecordCursorFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/table/ShowColumnsRecordCursorFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/functions/catalogue/TablesFunctionFactory.java	1	1	100.00%
🔵	io/questdb/griffin/engine/table/ShowCreateMatViewRecordCursorFactory.java	1	1	100.00%