Places that check the metadata file but miss the disaggregated shared metadata

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Duplicate
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: Metadata
    • None
    • Storage Engines, Storage Engines - Transactions
    • 169.063
    • None
    • None

      Issue Summary

      An audit of every remaining WT_IS_METADATA() call site, requested in review on WT-18364's PR, found two places that special-case the metadata file for a reason that holds equally for the disaggregated shared metadata, and do not check it. Both share a root cause with WT-18049: the running checkpoint hides its transaction ID from the global table, and the shared metadata has no history store content.

      The two findings are coupled. Finding 1 is load bearing; finding 2 does nothing until finding 1 is fixed. Fix them in that order.

      Status

      Finding 2 is confirmed reachable by instrumentation: a clean shared metadata page carrying the running checkpoint's own writes is routinely considered evictable, in three tests across two files. User visible harm is not demonstrated — no existing test holds an older reader of the shared metadata across such an eviction. The probe patch, the exact tests, and the transaction IDs observed are in the first comment on this ticket.

      Finding 1 is reasoned from the code and not separately reproduced, but the finding 2 measurements show its effect directly: in every hit, the page's last written transaction ID equalled metadata_pinned, that is, the running checkpoint's own ID.

      Finding 1: __wt_txn_oldest_id() does not give the shared metadata the checkpoint-aware pinned ID

      src/include/txn_inline.h, in __wt_txn_oldest_id():

      if (session->dhandle != NULL && WT_IS_METADATA(session->dhandle))
          return (__wt_atomic_load_uint64_v_relaxed(&txn_global->metadata_pinned));
      

      metadata_pinned is seeded from checkpoint_txn_shared.id in _txn_oldest_scan() (src/txn/txn.c) — the ID a running checkpoint hides. On the shared metadata tree this returns the plain oldest_id instead, so the chain wt_txn_visible_all() -> txn_visible_all_id() -> _wt_txn_oldest_id() reports the running checkpoint's own updates as globally visible, and obsolescence decisions on that tree follow from that.

      Reconciliation is already covered by a different mechanism: _rec_init() clamps r->rec_start_pinned_id with the checkpoint's ID, and WT-18364 converts that site to the combined macro. Every other consumer of _wt_txn_oldest_id() on the shared metadata tree is not covered.

      Finding 2: two eviction sites skip clean pages for the metadata only

      • src/evict/evict_walk.c, __evict_try_queue_page()
      • src/include/btree_inline.h, __wt_page_can_evict()

      Both refuse to evict a clean page whose modifications are not globally visible, and both justify it the same way: there is no history store for the metadata, so an older reader could not be served after the page is gone. The shared metadata is equally history-store-free — __rec_hs_wrapup() asserts exactly that, and all three WT_REC_HS gates exclude it. The premise holds for both trees; only the metadata file is checked.

      Converting these two sites on their own has no effect. Both call _wt_txn_visible_all(), which on this tree uses the unclamped oldest_id from finding 1, so the modifications read as visible and the guard passes regardless of which macro gates it. Only the wt_page_can_evict() site was observed to be reachable; the _evict_try_queue_page() one additionally requires WT_EVICT_CACHE_CLEAN_HARD and never fired.

      Lower priority, same premise, wasted work rather than incorrectness

      • __wt_curhs_cache() in src/cursor/cur_hs.c skips caching a history store cursor when evicting the metadata. The shared metadata needs one just as little. This path already carries a FIXME-WT-6037.
      • __wt_txn_read() in src/include/txn_inline.h skips the history store lookup for the metadata. A shared metadata read searches the history store for content that cannot be there.

      Judgment calls, listed for a decision rather than as defects

      • _btree_conf() in src/btree/bt_handle.c sets WT_BTREE_IGNORE_CACHE for the metadata, and _wt_evict_app_assist_worker_check() in src/evict/evict_inline.h does not block threads working on it, because such a thread may hold resources that block checkpoints or eviction. The shared metadata's writer is the checkpoint itself, so that hazard applies in full — but IGNORE_CACHE changes cache accounting, so this is a design decision. The WT_BTREE_LOGGED half of that site is correctly metadata-only.
      • __wt_checkpoint_file() in src/checkpoint/checkpoint_txn.c asserts the metadata lock is held when checkpointing the metadata. The shared metadata has its own locking discipline; presumably deliberate, worth confirming.

      Checked and correctly narrow, no action needed

      Recorded so the audit does not have to be repeated:

      • _rec_upd_select_inmem() in src/reconcile/rec_visibility.c contains the structural twin of the converted EBUSY check, plus an assert that the tree is the metadata. It is unreachable for the shared metadata: the path requires wt_btree_stays_in_memory(), and awaits_publish in _btree_conf() excludes WT_IS_URI_METADATA. Correct today, but by construction elsewhere rather than by the check — if that exclusion changes, the assert fires.
      • Sweep: _sweep_mark(), sweep_expire(), sweep_remove_handles(), _wt_session_dhandle_sweep(). The handle is opened and held by the layered table manager's internal session with its own WT_DHANDLE_OUTDATED lifecycle.
      • _wt_txn_prepare() and _wt_txn_rollback() asserts: nothing prepares shared metadata operations.
      • _wt_time_aggregate_validate() and _wt_time_value_validate(): the shared metadata does use timestamps — WT-18049's failure was an ordered consistency check on it — so skipping validation there would be wrong.
      • The WT_METAFILE_URI and WT_METAFILE_ID comparisons in logging, recovery, the turtle file, and the wt utilities: the shared metadata is neither logged nor recorded in the turtle file.

      Proposed Solution

      1. Fix finding 1 first: give the shared metadata the checkpoint-aware pinned ID in __wt_txn_oldest_id(), or establish why oldest_id is safe there. Nothing else in this ticket has an effect until this is settled.
      2. Then convert the finding 2 sites and re-run the three tests listed in the comment with the probe in place, to confirm the guard now engages where it previously did not.
      3. Decide whether a test that holds an older reader of the shared metadata across such an eviction is worth writing, to turn the reachability result into a failing test.
      4. Use WT_IS_ANY_METADATA() from WT-18364 for whichever sites are confirmed.

      Definition of Done

      • Finding 1 either fixed, or closed with a recorded reason why oldest_id is safe on the shared metadata tree.
      • Finding 2 sites converted and the guard shown to engage, or closed with a reason.
      • The two lower-priority sites either converted or explicitly left alone.
      • The two judgment calls answered by the owner, in a comment on this ticket.

            Assignee:
            [DO NOT USE] Backlog - Storage Engines Team
            Reporter:
            Etienne Petrel
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: