Skip to content

perf(storage): Reduce block recovery index overhead - #1296

Open
leiysky wants to merge 3 commits into
foyer-rs:mainfrom
leiysky:dev/optimize-block-recovery
Open

perf(storage): Reduce block recovery index overhead#1296
leiysky wants to merge 3 commits into
foyer-rs:mainfrom
leiysky:dev/optimize-block-recovery

Conversation

@leiysky

@leiysky leiysky commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove the recovery-local global HashMap and install recovered entries directly into the block indexer.
  • Install entries in reusable, approximately 32 MiB batches; partition each batch once and update independent shards concurrently.
  • Reduce allocation and hashing overhead with exact per-batch shard preallocation, ModHasher, tombstone prefiltering, and post-dedup compaction.
  • Scan blocks with buffer_unordered, then restore their original order before error aggregation and index installation.
  • Fast-path RecoverMode::None without scanning block contents.

Semantics and Compatibility

This change is internal to recovery: it does not change the public API or the on-disk format.

The upstream recovery behavior is preserved:

  • A block scan still stops at the first sequence regression.
  • Results are restored to input-block order, so error selection and equal-sequence last-write-wins behavior remain deterministic, including across installation batches.
  • Index replacement still uses the existing sequence >= current.sequence() rule.
  • Tombstones still suppress entries at the same or an older sequence.
  • Spawned-task panics still propagate through the existing unwrap() behavior.
  • The global next sequence and clean-block initialization retain their previous meaning.

Performance

Benchmarks used temporary local harnesses; no benchmark-only code is included in this PR.

End-to-end recovery

Workload: 300,000 forced on-disk entries, 512-byte values, 30 reopens with RecoverMode::Quiet; the first reopen verifies representative keys.

Comparison Recover average Result
Original main snapshot (9c24c8d) 33.652 ms baseline
Initial PR implementation (f3c6f7b) 19.664 ms 1.71x faster than that main snapshot
Previous PR head, follow-up run (f3c6f7b) 19.609 ms follow-up baseline
Current semantics-preserving implementation ~9.80 ms ~2.0x faster than the previous PR head

100M-entry scaling

The original synthetic index-installation benchmark plateaued at approximately 3.3 s from concurrency 4 onward, showing that allocation, hash-table insertion, and memory bandwidth dominate beyond that point.

After adding bounded batching, a temporary 8-worker sanity run installed 100 million streamed unique entries in 1.41 s (entry generation included; device scanning and final compaction excluded). This is not an apples-to-apples comparison with the original harness because the input-retention strategy differs; it is only a regression check for the new batching path.

Memory and Tradeoffs

  • Recovery still retains block scan results until all scan errors have been aggregated, preserving upstream error behavior.
  • The additional flat and per-shard installation buffers are now bounded to approximately 64 MiB in total instead of scaling with the entire recovered-entry set.
  • The final index still scales with the number of unique live keys; a 100M-unique-key recovery therefore remains a multi-GiB workload.
  • Parallel shard installation increases CPU and memory-bandwidth pressure and is bounded by recover_concurrency.
  • The recovery-only insertion path discards replaced-address results because recovery has no consumer for them.
  • Recovery logs distinguish scanned entries from compacted live entries.

Review Follow-ups

  • Shard selection uses mixed high bits before applying the shard mask/modulo, avoiding correlation with the low bits consumed by ModHasher inside each shard.
  • Recovery logs explicitly report both scanned and live entry counts.
  • Recovery installation is bounded and reuses its flat batch buffer, avoiding full-set duplication before deduplication.

Validation

  • cargo fmt --all -- --check
  • git diff --check
  • cargo check -p foyer-storage --lib
  • cargo test -p foyer-storage — 24 passed
  • cargo clippy -p foyer-storage --all-targets -- -D warnings -A dead-code
  • Focused tests cover shard-bit distribution, duplicate-heavy compaction, and differential equivalence across recovery-batch boundaries, including equal-sequence replacement.

@leiysky
leiysky force-pushed the dev/optimize-block-recovery branch from 9a40c0e to aa42eac Compare May 24, 2026 12:24
@codecov

codecov Bot commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.16514% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
foyer-storage/src/engine/block/recover.rs 93.22% 4 Missing ⚠️
Files with missing lines Coverage Δ
foyer-storage/src/engine/block/indexer.rs 99.13% <100.00%> (+1.45%) ⬆️
foyer-storage/src/engine/block/recover.rs 87.60% <93.22%> (-2.12%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@leiysky
leiysky force-pushed the dev/optimize-block-recovery branch from aa42eac to f3c6f7b Compare May 24, 2026 12:44
@leiysky
leiysky marked this pull request as ready for review May 24, 2026 12:45
@leiysky
leiysky requested review from MrCroxx and Xuanwo as code owners May 24, 2026 12:45
Copilot AI review requested due to automatic review settings May 24, 2026 12:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes block-engine recovery by eliminating a recovery-local global dedup table and instead streaming recovered entries directly into the in-memory indexer, relying on the indexer’s existing sequence-based deduplication. It also improves indexer batch insertion efficiency and introduces a custom hasher for indexer shards.

Changes:

  • Remove recovery-local HashMap dedup and insert recovered entries into the Indexer incrementally during scanning.
  • Apply recovered tombstones via a batched Indexer::remove_batch call after entry insertion.
  • Optimize Indexer::insert_batch sharding/batching and switch shard maps to ModHasher.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
foyer-storage/src/engine/block/recover.rs Streams recovered entries into the indexer per block and batches tombstone removals; adjusts recovery logging.
foyer-storage/src/engine/block/indexer.rs Changes shard map hasher to ModHasher and rewrites insert_batch to pre-bucket by shard and reserve capacity per shard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread foyer-storage/src/engine/block/indexer.rs
Comment thread foyer-storage/src/engine/block/recover.rs Outdated
Signed-off-by: leiysky <leiysky@outlook.com>
@leiysky
leiysky force-pushed the dev/optimize-block-recovery branch from f3c6f7b to 83dd4cc Compare July 26, 2026 12:15
Signed-off-by: leiysky <leiysky@outlook.com>
@leiysky
leiysky force-pushed the dev/optimize-block-recovery branch from 83dd4cc to 7d97e68 Compare July 26, 2026 12:16
Signed-off-by: leiysky <leiysky@outlook.com>
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