fix(pruner): reclaim storage-v2 changeset static files during history pruning#403
fix(pruner): reclaim storage-v2 changeset static files during history pruning#403nekomoto911 wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94560bef1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if limiter.is_limit_reached() { | ||
| done = false; | ||
| break |
There was a problem hiding this comment.
When a storage-v2 changeset block contains more rows than the per-segment limit, this branch marks done = false but static-file rows are not removed individually. finalize_history_prune then rolls the checkpoint back to the previous block, so the next prune run starts at the same block and replays the same first rows forever, leaving StorageHistory pruning stuck; the static path needs to stop only on block boundaries or otherwise persist row progress.
Useful? React with 👍 / 👎.
| if done && let Some(last_block) = last_changeset_pruned_block { | ||
| provider | ||
| .static_file_provider() | ||
| .delete_segment_below_block(StaticFileSegment::StorageChangeSets, last_block + 1)?; |
There was a problem hiding this comment.
When the last storage change in the processed range is earlier than the prune horizon (for example, no storage changes in blocks 101..1000), this deletes below last_block + 1 rather than the completed range_end + 1. The checkpoint can advance on the following empty run, but no delete is attempted then, so jars like [0..499] that are wholly below the horizon remain until some future storage change happens, defeating the reclamation this path is adding.
Useful? React with 👍 / 👎.
94560be to
200df2e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 200df2e8f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| provider | ||
| .static_file_provider() | ||
| .delete_segment_below_block(StaticFileSegment::AccountChangeSets, last_block + 1)?; |
There was a problem hiding this comment.
When changesets_in_static_files is enabled, this deletes account changeset jars before the history-index pruning result, prune checkpoint, and surrounding DB transaction are durable (Pruner::prune_segments saves the checkpoint after segment.prune, and PruneStage commits after run_with_provider). If finalize_history_prune, checkpoint saving, or the final commit fails after this call succeeds, the next run starts from the old checkpoint but the static walker sees the deleted blocks as empty, so the corresponding AccountsHistory entries can no longer be discovered and pruned; the storage-history path has the same ordering issue. Please make the filesystem reclamation happen only after the DB prune/checkpoint commit succeeds, or make it recoverable from the committed checkpoint.
Useful? React with 👍 / 👎.
… pruning Under `changesets_in_static_files`, account/storage history pruning only pruned the (empty) DB changeset tables, so changeset static-file jars below the prune horizon were never reclaimed and changeset disk usage grew unbounded with chain length while logical pruning kept working. Dispatch history pruning on `changesets_in_static_files`: the static-file path walks changesets, prunes the history index, then reclaims whole jars below the horizon via the new `delete_segment_below_block`, which keys off each jar's range end and never deletes the highest jar (so a jar straddling the horizon is kept). Ports #21346 to the merge branch, without the RocksDB history path (not present here). Closes #402
200df2e to
1481039
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1481039a7e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if limiter.is_limit_reached() { | ||
| done = false; | ||
| break |
There was a problem hiding this comment.
When changesets_in_static_files is enabled and a single block has more account changes than the per-segment delete limit, this stops after consuming only the first rows from that block. finalize_history_prune then rolls the checkpoint back to the previous block because done is false, but the static-file rows are not removed, so the next run starts at the same block and replays the same prefix forever instead of making progress.
Useful? React with 👍 / 👎.
| if done && let Some(last_block) = last_changeset_pruned_block { | ||
| provider | ||
| .static_file_provider() | ||
| .delete_segment_below_block(StaticFileSegment::AccountChangeSets, last_block + 1)?; |
There was a problem hiding this comment.
When a completed account-history range has no account changes near range_end (or no account changes at all), this either deletes only below the last changed block or skips deletion entirely, while finalize_history_prune still advances the checkpoint to range_end. The following prune run starts after that range and will not revisit these empty blocks, so whole AccountChangeSets jars that are already below the prune horizon remain on disk indefinitely.
Useful? React with 👍 / 👎.
1 participant
Closes #402
Under
changesets_in_static_files, account/storage history pruning only pruned the (empty) DB changeset tables, so the changeset static-file jars below the prune horizon were never physically reclaimed — changeset disk usage grew unbounded with chain length while logical pruning kept working correctly.Dispatch account/storage history pruning on
changesets_in_static_files: the static-file path walks the changesets, prunes the history index, then reclaims whole jars below the horizon via the newStaticFileProvider::delete_segment_below_block, which keys off each jar's range end and never deletes the highest jar — so a jar straddling the horizon (still holding live blocks) is kept whole.Ports #21346 to the merge branch, without the RocksDB history path (not present here).