Skip to content

fix: handle cached snapshot records with recovery regressions - #41

Open
Konstantin V Shvachko (shvachko) wants to merge 2 commits into
microsoft:mainfrom
shvachko:test/full-cache-snapshot-recovery
Open

Konstantin V Shvachko (shvachko) wants to merge 2 commits into
microsoft:mainfrom
shvachko:test/full-cache-snapshot-recovery

Conversation

@shvachko

@shvachko Konstantin V Shvachko (shvachko) commented Oct 7, 2026 •

Copy link
Copy Markdown

Purpose

Upstream follow-up to #38, adding regression tests and explicit recovery requirements to its existing snapshot fix.

This branch preserves Sheikh Nasrullah (@snash4)'s original fix commit f76efa8642706e30f960fcc73327861791c125a3 unchanged, followed by the tests/documentation commit d4d3613cf26347381fb16e12a078ac383ccecdaa. Relative to upstream main, it contains only the cache-conversion fix, native regression tests, and requirements documentation. There are no locking changes or downstream workarounds.

This replaces the incorrectly targeted snash4#1; the contribution belongs in microsoft/bf-tree.

Regression requirements

  • Cover both snapshot sweep and writer-side capture of full-cache pages.
  • Force full-page scan promotion and assert that cached records and phantom deletion records are exercised.
  • Check every key in the test domain and compare the complete ordered scan against an independent BTreeMap oracle, detecting missing, extra, and incorrect records.
  • Exercise overwrites, deletions, recreation, and new keys across three snapshot/recovery cycles.
  • For writer-side capture, hold CPR phases deterministically, confirm capture occurred, and distinguish the pre-write snapshot from the post-write live tree.
  • Recover private copies and verify that the original snapshot artifacts remain unchanged.

The tests are in src/snapshot.rs; requirements are documented in doc/snapshot-recovery.md. The ordinary-thread regression module does not replace the existing CPR/Shuttle suites.

Validation

With the existing PR 38 fix, both new tests pass on Windows ARM64 and Linux ARM64 using Rust 1.93.0 in release mode.

cargo +1.93.0 test --release snapshot::tests::full_cache_recovery -- --test-threads=1

Negative control: removing only PR 38's no-op handling makes each test fail independently with Base page should not have op type: Cache. The existing fix was restored before final positive validation.

Downstream complete-state integration results:

Configuration Result
Windows ARM64 / HASH 111 cases, 26,025 assertions passed; three additional recovery-only runs passed
Linux ARM64 / HASH 111 cases, 26,025 assertions passed
Linux ARM64 / FST 111 cases, 26,089 assertions passed
Windows ARM64 / FST Blocked by a separate native allocation-lock panic at src/storage.rs:150:44: Result::unwrap() on Err(())

The failing downstream run was stopped after it stalled. Passing runs do not erase that failure: full downstream acceptance remains blocked, and this snapshot-only contribution does not claim to fix the independent locking problem.

Review

Yi Shan (@acmshanyi) (Yi Shan), requesting your review of the snapshot fix, regression coverage, and requirements.

A maintainer will need to add the formal reviewer request for acmshanyi: my API request was denied with HTTP 404 due to repository permissions.

Exercise sweep and writer-side capture with deterministic full-page promotion, cached deletions, an independent complete-state oracle, and three recovery cycles. Document the requirements and negative-control reproduction for the existing cache-conversion fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shvachko

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants