Skip to content

Split KV restore/replay batches at epoch endings - #423

Open
sausagee wants to merge 1 commit into
m1from
cursor/kv-replay-epoch-batch-split-39a2
Open

Split KV restore/replay batches at epoch endings#423
sausagee wants to merge 1 commit into
m1from
cursor/kv-replay-epoch-batch-split-39a2

Conversation

@sausagee

@sausagee sausagee commented Sep 6, 2026

Copy link
Copy Markdown

Description

Clean-room rewrite on m1 (not a cherry-pick, patch, or mechanical port from aptos-labs). Upstream intent was read only to identify the bug and the invariant (aptos-labs/aptos-core commit message for KV replay splitting at epoch endings). Names, control flow, comments, and tests are original.

Bug: KV-only restore/replay batched transactions by size only. Storage usage (VersionData) is written for the last version of each batch. The first block of a new epoch reads usage at the previous epoch's last version (state_storage::on_new_block). When an epoch ending landed in the middle of a batch, that version had no usage row and later re-execution failed with VersionData at <version> is missing.

Invariant:

  • A KV-replay group never contains an epoch-ending item except as its last item.
  • No group is longer than BATCH_SIZE.
  • Concatenating groups equals the original stream order.
  • Full-execution replay already isolates epochs inside the chunk executor and is unchanged.

What changed (crate aptos-backup-cli only):

  • seal_kv_replay_batches closes a group at NewEpochEvent (same predicate as the chunk executor / replay-on-archive) or at the existing size cap.
  • replay_kv flattens those sealed groups before save_transactions_and_replay_kv, so base_version still advances in order.

How Has This Been Tested?

cargo test -p aptos-backup-cli --locked

27 passed; 0 failed; 1 ignored (backup_types::tests::test_end_to_end was already ignored).

New coverage in kv_replay_batch_tests:

  • empty input
  • size-cap only
  • epoch ending before the size cap
  • consecutive epoch endings
  • epoch ending exactly on the size cap
  • leading epoch ending
  • proptest: concatenation, non-empty groups, len <= max_len, and no epoch flag except at the tail of a group

CI: rust-lints, rust-targeted-unit-tests, general-lints, rust-cargo-deny, and the license check are green on this revision.

Key Areas to Review

  • seal_kv_replay_batches / kv_replay_item_ends_epoch in storage/backup/backup-cli/src/backup_types/transaction/restore.rs
  • replay_kv now try_flattens size-capped chunks after sealing at epoch endings

Aikido / security notes (new code only)

Aikido MCP/CLI is not available in this environment. Manual review of the new code against RUST_SECURE_CODING.md:

  • No unsafe.
  • No new parsers, network surfaces, or third-party crates.
  • max_len is the existing BATCH_SIZE constant (2 in tests, 10000 otherwise); open.len() >= max_len is usize comparison with no wrapping math.
  • debug_assert!(max_len > 0) is a programmer check; a zero cap would still emit one-item groups rather than panic in release.
  • No unwrap/expect on restore input; epoch detection uses the existing ContractEvent::is_new_epoch_event predicate.
  • Extra groups only increase DB commit granularity; they do not accept extra user data.

Please run the project Aikido workflow on the PR if that is part of CI.

Type of Change

  • Bug fix
  • Tests

Which Components or Systems Does This Change Impact?

  • Other (specify): storage backup restore / KV-only replay (aptos-backup-cli)

Checklist

  • Clean-room implementation; no aptos-labs commit SHAs, patches, or copied function bodies
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I tested both happy and unhappy path of the functionality
Open in Web Open in Cursor 

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

KV-only replay previously grouped transactions only by size, so an epoch
ending could sit in the middle of a batch. VersionData is written for the
last version of each batch; later epochs read usage at the prior epoch
ending. Seal each batch at NewEpochEvent (or the size cap) so that
version always persists.

Co-authored-by: Young Yang Liauw <sausagee@users.noreply.github.com>
@sausagee
sausagee marked this pull request as ready for review September 6, 2026 05:23
@sausagee
sausagee requested a review from areshand as a code owner September 6, 2026 05:23
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