__wt_btree_is_outdated_disagg uses || instead of && between DISAGGREGATED and READONLY checks

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • Storage Engines, Storage Engines - Transactions
    • 5.739
    • None
    • None

      code:c
      static WT_INLINE bool
      __wt_btree_is_outdated_disagg(WT_SESSION_IMPL *session)
      {
      return ((F_ISSET(S2BT(session), WT_BTREE_DISAGGREGATED) ||
      F_ISSET_ATOMIC_32(S2BT(session), WT_BTREE_READONLY)) &&
      F_ISSET(session->dhandle, WT_DHANDLE_OUTDATED));
      }
      code

      The two flag checks for WT_BTREE_DISAGGREGATED and WT_BTREE_READONLY are combined with ||, so the function returns true when the btree is merely read-only (even if not disaggregated), or merely disaggregated (even if not yet read-only), as long as the dhandle is outdated.

      This contradicts the documented intent of every caller and commit that introduced/uses this check:

      • The original introduction (WT-17820) computed this as a single bitmask check F_ISSET(S2BT(session), WT_BTREE_DISAGGREGATED | WT_BTREE_READONLY), and its commit message says splits are rejected "on handles that are simultaneously disaggregated, read-only, and outdated".
      • evict_page.c's __evict_page_dirty_update comment: "A page on an outdated disaggregated read-only btree that is not clean-evictable..." — describing a single btree state where both properties hold together.
      • WT_BTREE_READONLY was later split out into the atomic flags word (flags_atomic), which is why it needed a separate F_ISSET_ATOMIC_32 call instead of being OR'd into the same bitmask — but the boolean combinator should have stayed AND-like ("both flags set"), not become a true logical || ("either flag set").

      Impact: with the current ||, the guards in _wt_split_insert/wt_split_multi/_wt_split_rewrite (bt_split.c) and the eviction-walk skip / dirty-page-discard logic in evict_walk.c and evict_page.c can trigger for btrees that are outdated and read-only but never disaggregated, or outdated and disaggregated but not yet read-only — over-broadly blocking splits and altering eviction behavior for handles the logic wasn't meant to cover.

      Fix: change || to && so the function only returns true when the btree is both disaggregated and read-only (in addition to the dhandle being outdated).

            Assignee:
            Shoufu Du
            Reporter:
            Shoufu Du
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: