-
Type:
Task
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: Cursors, Layered Tables
-
Storage Engines - Foundations
-
124.63
-
None
-
None
Follow-up to WT-17933, proposed by ivan.kochin@mongodb.com in the PR review
(PR 14323 thread).
Layered tombstone encoding is kept correct today by convention and by the dist/s_tombstone_encoding.py static check: every site that moves a value between a constituent cursor and the outside world must remember the encode/decode step. Ivan's idea is to make the conversion structurally inseparable from the transfer itself, so a new read or write path gets it for free:
- __clayered_deleted_encode() gets exactly one caller: __clayered_put() - storing a value into a constituent becomes indivisible from encoding (modify rewrites included).
- __clayered_deleted_decode() is called only from a value-promotion wrapper (surfacing a constituent value on the user cursor) and a modify-base wrapper, plus the drain conversion which keeps its own contract.
- The cursor code never reads or writes a constituent value except through these named wrappers; the only stores outside __clayered_put() are the raw delete marker and the plain-modify fast path.
This also simplifies s_tombstone_encoding.py: the reviewed caller inventories collapse to the wrappers.
Ivan prototyped the full change (wrappers.patch attached to the PR thread). Notes for whoever picks this up:
- Two behavioral points from the prototype deserve focused review and tests: namespace-path modify rewrites re-search instead of reusing the held cursor position and the encode scratch buffer's lifetime moves into __clayered_put().
After this work is done, assess if tombstone encoding linters still bring enough value to keep them in the codebase: dist/s_tombstone_encoding.py, dist/test_tombstone_encoding.py. In fact, there should be enough justification to keep them.
Original comment:
I thought about how to make it harder for developers to forget the encode/decode step in future changes. The approach I ended up with is to couple the conversion with the read/write itself: every place that moves a value between a constituent cursor and the outside world now goes through a small set of wrappers that do the conversion internally. Anyone writing a new read or write path will naturally reach for these wrappers and get the encoding for free. It should also simplify s_tombstone_encoding.py, since the reviewed caller inventories collapse down to the wrappers. The resulting structure: - __clayered_deleted_encode() has exactly one caller: __clayered_put(). Storing a value into a constituent is now indivisible from encoding — modify rewrites also go through it, via a new WTI_CLAYERED_PUT_MODIFY_REWRITE op. - __clayered_deleted_decode() has two callers in the layered cursor logic: __clayered_promote_value() (surfacing a constituent value on the user cursor) and __clayered_modify_apply_decoded() (decoding the base value a modify applies against). The only other caller is __wt_clayered_ingest_to_stable_value(), the drain conversion, which keeps its own contract. So the cursor code never reads or writes a value except through these named wrappers. The only stores outside __clayered_put() are the raw tombstone delete marker and the plain modify() fast path (FIXME-WT-18057). Two behavioral notes: - A modify rewrite of an encoded-namespace value now re-searches instead of using the held cursor position, because __clayered_put() sets the key. This only affects the rare namespace paths; the common plain-modify path is untouched. - The encode scratch buffer moved into __clayered_put() and is freed before it returns. That is safe because a successful insert or update re-points the constituent's value at internal update memory, and a failed non-overwrite insert returns a cursor-local copy of the existing value. It's not a small change, so if you don't think it makes things better, I don't want to block this PR on it. Here is the full diff. [wrappers.patch](https://github.com/user-attachments/files/30579398/wrappers.patch)
- is related to
-
WT-17933 Stop persisting encoded ingest tombstones prefixed values on disk
-
- Closed
-
-
WT-18057 Layered tombstone decode guard triggered while persisting the recovery doc of an inflight chunk operation
-
- Closed
-
- related to
-
WT-17933 Stop persisting encoded ingest tombstones prefixed values on disk
-
- Closed
-