-
Type:
Task
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: Reconciliation
-
Storage Engines, Storage Engines - Persistence
-
None
-
None
The review of page deltas (https://github.com/wiredtiger/wiredtiger/pull/12076) identified several issues:
- The documentation string for the delta-related config options should say "reconciliation may write deltas" rather than "reconciliation will write deltas"
- The ref_changes variable should be read using acquire read
- The change in the disagg branch dropped the assertion in split_multi_inmem_final about restoring saved updated to the new page. We can tweak the assertion (see the PR for details) and reinstate it.
- We could possibly optimise the number of flags used, see the PR for details.
- In wt_txn_read_upd_list_internal, there's some new functionality to search the history store if we see an update that is restored from a delta and not visible to the reader. We don't need to perform this search if the dhandle is metadata.
- In rec_row_garbage_collect_fixup_update_list, we can set the new tombstone's transaction id to WT_TXN_NONE instead of the previous update's transaction ID (see the PR for details.)
- There's a likely bug in wti_rec_row_leaf. We can only garbage collect an update if it has no stop timestamp – there's a proposed fix on the PR.
- Variables need to be alphabetised in several places.
- There's a mistake in cell.h where we outline the structure of wt_delta_cell_leaf. It lists the key length twice, the second key length should be the value length.
- The comment is wrong about deletes, there's a suggested fix on the PR.
- The original change removed an assertion in rec_fill_tw_from_upd_select, where we check that a tombstone written to disk should have the full value. We can reinstate this assertion by first checking WT_BTREE_DISAGGREGATED, there is more information on the PR.
- In rec_write_page_status, it's unclear where r->rec_start_pinned_stable_ts is set.
- The comment in rec_split_write about needing to assign a new page ID for the root every time needs to be changed – we don't support deltas for root pages yet.
- The if (build_delta) case in rec_split_write ought to be a separate function.
- rec_split_discard unnecessarily assigns WT_BLOCK_INVALID_PAGE_ID to multi->block_meta.page_id.
- In that same function, it's unclear whether we really need the mod->rec_result = 0 assignment.