-
Type:
Improvement
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: Layered Tables
-
None
-
Storage Engines - Foundations
-
134.956
-
None
-
8
Common-concern follow-up from the review of PR #14124 (WT-17959). The refactor split __clayered_iterate_constituents() into focused helpers, but the underlying iteration-state logic (carried over from the pre-refactor code) remains complex and fragile: correctness depends on several implicit, cross-function invariants that are easy to break with an innocent-looking change. This ticket collects the concrete spots so the logic can be reworked as a whole rather than patched case by case.
Fragile places identified during review
- Implicit "flag set implies both constituents positioned" contract in the prepare-conflict branch. The else if chain that skips _clayered_position_alternate() while blocked on an ingest prepare conflict relies on (a) the iteration flag being cleared by _clayered_update_state() on any snapshot/checkpoint change and (b) the exit contract that both constituents were left positioned. A candidate edge case: the stable constituent is opened for the first time mid-walk while the cursor is blocked on an ingest prepare conflict, and the blocked key is later evicted from ingest after the checkpoint pickup — the freshly opened stable cursor may never be positioned. Related to WT-17969. (discussion)
- Alternate-advance ordering under a prepare conflict. Normally the alternate is compared and advanced against the current cursor before the current moves. When the current was blocked on a prepared key and the conflict resolves, the comparison effectively happens against the current cursor after it moved. Review concluded the current behavior is likely correct (the tombstone-resolution case is caught later), but only via subtle multi-step reasoning — the ordering invariant is not enforced or documented in the code. (discussion)
- Walk-in-progress detection based on "any constituent is positioned". Any error path that forgets to reset() a constituent silently flips the walk between fresh-positioning and steady-state advance. Proposal: a dedicated __clayered_iterate_positioned() predicate based on the current cursor's WT_CURSTD_KEY_INT / btree ref, which requires setting current_cursor when blocked on a prepare conflict (a behavior change). (discussion)
- Undocumented "NOTFOUND impossible here" invariant. Neither the author nor reviewers could initially derive why WT_NOTFOUND cannot occur at that point; a comment was added in the PR, but the invariant should be made structural or asserted. (discussion)
- Single-constituent walks leave the other constituent positioned. When __clayered_enter() supplies only one constituent for the operation, the other keeps its stale position instead of being reset, which interacts badly with item 3. (discussion)
- Proposed simplification of _clayered_advance_positioned(). Unify the prepare-blocked and steady-state paths: reposition the alternate whenever the iteration flag is not set, and compare/advance the alternate only when the current is the ingest cursor. Known open question: a prepare conflict hit inside _clayered_position_alternate() itself (also unhandled in the current code). (discussion)
Suggested outcome
- Make the positioning contract explicit: assert (not just assume) the state each branch relies on.
- Reduce the number of special-cased branches, evaluating the __clayered_advance_positioned() simplification above as a starting point.
- Cover the edge cases from items 1, 2 and 5 with tests once the reworked logic makes them expressible.
- is related to
-
WT-17969 Follower layered cursor aborts in __clayered_reopen_stable when a read-timestamped reader's parked stable key vanishes from a legitimately-adopted newer checkpoint
-
- In Code Review
-
-
WT-17959 Refactor __clayered_iterate_constituents
-
- Closed
-
-
WT-17995 Document layered cursor internal contracts
-
- Open
-
- related to
-
WT-17969 Follower layered cursor aborts in __clayered_reopen_stable when a read-timestamped reader's parked stable key vanishes from a legitimately-adopted newer checkpoint
-
- In Code Review
-
-
WT-17959 Refactor __clayered_iterate_constituents
-
- Closed
-
-
WT-17995 Document layered cursor internal contracts
-
- Open
-