Skip to content

fix: Pruning window - #2438

Open
sergerad wants to merge 3 commits into
sergerad-lockfree-store-statefrom
sergerad-fix-pruning
Open

fix: Pruning window#2438
sergerad wants to merge 3 commits into
sergerad-lockfree-store-statefrom
sergerad-fix-pruning

Conversation

@sergerad

@sergerad sergerad commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #2437.

History pruning deleted rows purely by age, but a row written before the cutoff can still be a key's applicable value for blocks inside the retention window, so historical vault, storage-map, and code reconstruction silently returned incomplete state.

Fix: explicit validity intervals

This PR replaces the is_latest flag with the fact pruning actually needs: each row of accounts, account_vault_assets, and account_storage_map_values now carries a validity interval [block_num, valid_until).

  • On update, the write path closes the previous row's interval by setting valid_until to the new row's block_num. Current rows are open-ended.
  • The open end is the sentinel i64::MAX rather than NULL, keeping every validity predicate a single range comparison that partial indexes can serve.
  • Migration 004_validity_intervals.sql backfills existing rows in one UPDATE … FROM + LEAD() window pass per table, drops is_latest, and rebuilds the affected indexes.

Effects

  • Correctness: pruning is now exact — a row is deleted iff its interval ends at or below the cutoff (valid_until <= cutoff), i.e. iff it can no longer serve any block inside the retention window. Baseline rows survive by construction. Account codes reduce to one predicate: a code lives iff some row with valid_until > cutoff references it — which covers in-window rows, current rows, and baselines in a single range scan.
  • Pruning cost: vault/storage prunes are index range scans over rows that expired since the last prune (O(recent churn) per block, via new partial cleanup indexes on valid_until). The codes prune is an index-only scan of the covering idx_accounts_code_validity. Verified via EXPLAIN QUERY PLAN.
  • Reads: select_account_vault_at_block collapses from a MAX-per-key self-join to interval membership (block_num <= X AND valid_until > X). All latest-row lookups keep their previous query plans (verified plan-identical between schema v3 and v4).
  • The prune deletes interpolate the sentinel as a SQL literal deliberately: SQLite only uses a partial index when the query provably implies its predicate, and it cannot prove anything about a bound parameter.

Tests

  • Migration backfill exercised against a real v3 database with versioned rows (validity_interval_migration_backfills_from_versioned_rows).
  • Baseline retention and reconstruction-at-the-cutoff regression asserts in test_prune_history (these fail against the old pruning predicate).
  • Code-blob lifecycle: baseline code retained while reachable, collected once superseded below the cutoff.
  • Schema hash pin updated to 4 migrations.

Operator note

This bumps the store schema to version 4; existing databases must run the explicit migrate command before start (verify_latest_schema rejects un-migrated databases). The backfill is a single pass per table and runs inside the migration transaction.

Changelog

[[entry]]
scope       = "node"
impact      = "fixed"
description = "History pruning no longer deletes account state rows still required to reconstruct historical vault, storage-map, or code state within the retention window."
[[entry]]
scope       = "node"
impact      = "breaking"
description = "Store database schema bumped to version 4 (validity intervals replace is_latest); operators must run the database migration before starting the node."

@sergerad
sergerad changed the base branch from next to sergerad-lockfree-store-state August 5, 2026 02:10
Comment thread crates/store/src/db/models/queries/accounts.rs Outdated
@sergerad
sergerad marked this pull request as ready for review August 5, 2026 23:47
@sergerad
sergerad force-pushed the sergerad-fix-pruning branch from c0507dd to 6982012 Compare August 6, 2026 01:48

-- accounts ---------------------------------------------------------------------------------------

ALTER TABLE accounts ADD COLUMN valid_until BIGINT NOT NULL DEFAULT 9223372036854775807;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we sure we don't want this to be null instead? The magic number is a bit difficult to reference in sql.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I second this. At least for me having NULL in valid_until if the entry is latest would look better.

Comment on lines +11 to +16
UPDATE accounts SET valid_until = next_versions.next_block_num
FROM (
SELECT account_id, block_num,
LEAD(block_num) OVER (PARTITION BY account_id ORDER BY block_num) AS next_block_num
FROM accounts
) AS next_versions

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

TIL about LEAD. I've always done this in a more roundabout fashion.

Comment on lines +109 to +112
/// Builds a version-3 database with versioned `is_latest` rows and verifies that migration 4
/// backfills each row's `valid_until` with its successor's `block_num` (or the open sentinel).
#[test]
fn validity_interval_migration_backfills_from_versioned_rows() -> Result<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rather move this test to the actual migration itself.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean as part of the sql or as part of the rs file that runs the migration?

Comment thread crates/store/src/db/models/queries/accounts.rs Outdated

@kkovaacs kkovaacs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Apart from the i64::MAX -> NULL suggestion this looks good to me!


-- accounts ---------------------------------------------------------------------------------------

ALTER TABLE accounts ADD COLUMN valid_until BIGINT NOT NULL DEFAULT 9223372036854775807;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I second this. At least for me having NULL in valid_until if the entry is latest would look better.

@sergerad
sergerad force-pushed the sergerad-fix-pruning branch from 6982012 to f1d38cf Compare August 6, 2026 08:34
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.

Fix pruning related snapshot / sqlite inconsistency

3 participants