Mega Beta5 - #48
Conversation
Attempt: gen_size_check now reinserts only when an unsized field's serialized size changed; the same-size case rebuilds the full row and writes it in place at the SAME slot via data.update instead of the corrupting mem::swap. STATUS: does NOT work yet. Even the 8-byte inline case reads back wrong — the data.update path publishes via stage_published_row and the subsequent read is served from a stale/duplicate published slot. Pushed dirty for visibility; the publication-path interaction is the remaining blocker. Safe reinsert path is on master; this branch is experimental.
Add DataPages::update_in_place: re-serialize the full row into the SAME slot
and republish it LIVE (unghost + publish_wrapped_row), fixing the ghosting that
made data.update read back stale. gen_size_check now reinserts only when a
field's serialized size changed; same-size writes in place, re-resolving the
link under the mutation gate.
WORKING: same_length (inline), different_length, update_many_times, and the
single/multithread in-place unsized tests — long-string corruption is FIXED.
REMAINING: update_parallel_more_strings{,_more_threads} hit InvalidLink under
concurrency. Root: the reinsert branch drops its guard (drop(_guard)) and a
concurrent same-key reinsert can free a link that #diff_process_insert /
size-check still reference. Pre-existing reinsert-path window, now exposed
because more ops take the in-place path. Needs the guarded section to not use a
link across the drop.
…remains (PR#48) update_in_place now falls back to a full reinsert when the same-slot write does not fit (equal field sizes don't guarantee equal total serialized length, and a concurrent reinsert may have moved the row). Correctness-first: reinsert always works. Fixed the InvalidLink hard failures. STATUS: deterministic tests pass (same_length, different_length, update_many_times, update_parallel_more_strings_more_threads, single/multithread in-place). REMAINING: update_parallel_more_strings is FLAKY (~4/8) — a lost-update race: two concurrent same-key updates interleave and a stale value wins (value mismatch, not InvalidLink). Suspect the size-check select reads before the per-key mutation gate fully serializes. Under analysis.
Same-size updates to a non-indexed unsized (String) column are now written in place at the existing slot instead of a full delete-and-reinsert. - DataPages::update_in_place: re-serialize the row into the SAME slot and republish it LIVE (unghost + publish_wrapped_row), avoiding both the dangling out-of-line ArchivedString pointer from mem::swap and the ghosting that made a plain data.update read back stale. - gen_size_check: reinsert only when a field's size changed; the in-place path holds the full-row lock (serializing the read-modify-write against concurrent same-key updates — fixes a lost-update race) and falls back to reinsert when the total serialized length does not fit the slot. - Scope: only when NO updated column is indexed. Indexed columns keep the reinsert path so secondary-index maintenance and unique-constraint checks run. Full integration suite: 356 passed / 0 failed. update_parallel_more_strings 8/8 under stress. KV overwrite ~298K -> ~459K ops/s (+54%).
b76a60a to
5d2bf3a
Compare
|
Logical WTI persistence review follow-up is included in this branch at 788c8ee.
4/5. The fixed 64 inline mutex-stripe footprint is documented, reads never touch it, and DefaultHasher striping is explicitly documented as per-key exclusion only with no key/range-order meaning. The synthetic index=0 and max_value=value fields are documented as a logical marker rather than structural metadata. The shadow validates the marker and derives the actual node position and maximum through native WTI CDC; replacement coverage verifies same-key link changes. Final local gate on the exact PR head: all-feature Clippy clean; 159 library tests passed; 359 integration tests passed; 3 pre-existing tests ignored. |
F1: full-row update() now takes the in-place same-size path when the table has
NO secondary indexes (a full-row update rewrites every index otherwise).
Documented why gen_non_unique_update (indexed column) and indexed full-row
updates correctly stay on reinsert; the custom single-column updates already
get the fast path via gen_size_check.
F2: update_in_place documents it emits NO persistence CDC and must not be used
on persisted tables (would silently lose durability); the persisted generator
keeps the reinsert path.
F3: comment that correctness relies on update_in_place's slot-length re-check +
reinsert fallback, not on the pre-lock size decision being current.
F4: drop the redundant row.clone() inside update_in_place (row is consumed).
F5: add concurrent_reads_during_in_place_update_never_tear — 3 readers vs a
writer doing 20k same-size in-place updates assert no torn reads. Passes,
confirming PublishedRow::replace swaps versions atomically.
Full integration suite: 359 passed / 0 failed.
|
Addressed all 5 self-review findings in
Full integration suite: 359 passed / 0 failed. |
The overwrite in-place fast path re-resolves the row's current Link and republishes through the primary index, so a backend whose link lookup or publication differs could keep the wrong slot or tear a read. Parametrize the unsized in-place suite (same-length stays in place, length-change round-trips, concurrent reads never tear) over WorkTablesIndex, Congee, and Arctic via the `using` keyword, so any divergence fails the test rather than corrupting one index type silently. All 9 pass.
F1: full-row update() now takes the in-place same-size path when the table has
NO secondary indexes (a full-row update rewrites every index otherwise).
Documented why gen_non_unique_update (indexed column) and indexed full-row
updates correctly stay on reinsert; the custom single-column updates already
get the fast path via gen_size_check.
F2: update_in_place documents it emits NO persistence CDC and must not be used
on persisted tables (would silently lose durability); the persisted generator
keeps the reinsert path.
F3: comment that correctness relies on update_in_place's slot-length re-check +
reinsert fallback, not on the pre-lock size decision being current.
F4: drop the redundant row.clone() inside update_in_place (row is consumed).
F5: add concurrent_reads_during_in_place_update_never_tear — 3 readers vs a
writer doing 20k same-size in-place updates assert no torn reads. Passes,
confirming PublishedRow::replace swaps versions atomically.
Full integration suite: 359 passed / 0 failed.
Summary
Beta 5 hardening rollup. This PR is rebased on current master and now contains the complete fixes for:
Regression coverage
The persisted-vacuum test proves reuse survives two reloads, a reused insert does not grow .wt.data, and consumed free ranges cannot overwrite a previously reused row.
Validation
Fixes #31.
Fixes #33.