[thweetkomputer]

Context

Fix the Stream recovery crash in CI run 39365508086, amd64 shard 5. PrivatePlanYieldsWorkerButRetainsKeyAndRecovers/extent loses its XRANGE connection after restarting with three workers; the server log reports MaterializePublishedIndexLocation asserting owner < kMaxMemoryWorkers.

Behavior before and after

A Stream range captures an immutable directory, releases store state, and reads its boundary pages before building the snapshot pin list. Those reads can allow transaction cleaning to relocate and retire the captured physical blocks. The old directory still owns its compact coordinates, but no longer owns their allocations; materializing those coordinates can assert, or borrow a reused block's identity.

After boundary reads, the reader reacquires store state, verifies the population and logical version, and refreshes both the root and grouped physical directory before capturing pins. It retains the computed ranks and admitted probe payload because the logical graph is unchanged.

Implementation

• Refresh the matching current graph in ReadValueForTransferLocked before PrepareOrderedRangeSnapshotPins, with no suspension between graph lookup and pin-list capture.
• Add a deterministic two-worker regression: suspend the upper-bound read, relocate the root and pages, retire the old allocation, then finish the range and verify the exact messages and pin cleanup.

Design decisions and alternatives

Use the existing worker-local asynchronous store lock and physical pin protocol. Keeping the old allocation alive throughout boundary discovery would require broader pinning before the selected interval is known. Refreshing the physical graph preserves the existing bounded range-pin model.

Documentation and comments

Added the lifetime invariant beside the refresh and comments explaining the regression's scheduling. Architecture is unchanged: this restores the documented requirement to capture current physical identities before suspension and pin the selected graph.

Test plan

Local validation uses the existing ARM64 Debug build with test faults enabled and aggressive optimization disabled.

• Before the fix, the new regression aborts at impl.h:430 with the same owner assertion. Its captured stack goes through PrepareOrderedRangeSnapshotPins and ReadValueForTransferLocked.
• cmake --build build_ci --target lavik_unit_tests lavik lavik_compact_collection_write_e2e_test lavik_grouped_ordered_write_e2e_test --parallel 4 — passed.
• build_ci/lavik_unit_tests --gtest_filter='GroupedCollectionTwoOwnerReadTest.StreamRangeRefreshesPhysicalGraphAfterBoundaryReads' --gtest_repeat=20 — all 20 passed.
• build_ci/lavik_unit_tests --gtest_filter='*LoadValueAwaitableTestPeer*:*ReadTest*' — all 96 passed.
• LAVIK_TEST_DATA_DIR=/mnt/dev ctest --test-dir build_ci -R '^lavik_(compact_collection_write|grouped_ordered_write)_e2e$' --output-on-failure — both suites passed: compact collection 54 passed (417.76 s); ordered collection 133 passed and 1 skipped (972.97 s). The skipped GroupedRdbStreamE2e.LargeListOverOneGiBImportsAndExportsWithoutAggregate requires explicit opt-in for its private 4 GiB image and >1 GiB RDB.
• pre-commit run --files src/storage/engine/transfer_api.cpp tests/load_value_awaitable_test.cpp — passed.
• git diff --check — passed.

Risk assessment

The change is limited to Stream range snapshot capture. It adds a store-lock reacquisition and current-index lookup after boundary discovery, without changing durable formats, range selection, or whole-object transfers. Population and logical-version changes fail the read instead of reusing computed ranks for a different graph.

Rollback plan

Revert this commit; no migration is required. Reverting restores the relocation race.

Reviewer guide

Start with the deterministic interleaving in StreamRangeRefreshesPhysicalGraphAfterBoundaryReads, then check that the production refresh updates the root, manifests, and grouped handle together before pin-list capture.

Follow-up work

None.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7c0399ca-5fc7-4c80-b5ae-bae3b97558ca

📥 Commits

Reviewing files that changed from the base of the PR and between 7ef96dd and 4ee7564.

📒 Files selected for processing: 2

• src/storage/engine/transfer_api.cpp
• tests/load_value_awaitable_test.cpp

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}