db/state: remove FilesItem.frozen field - #22126
Merged
Merged
Conversation
`frozen` cached `stepCount >= stepsInFrozenFile` only to skip per-file refcount atomics in BeginFilesRo(). Since reader pinning moved to a single bundle-level refcount (aggregatorVisible, #21397), that optimization is obsolete. This removes the field and the guards that depended on it: the per-file refcount `!frozen` skip in the forkable/snapshot begin+close paths, the "don't delete frozen files" guard in closeFilesAndRemove, and the frozen skip in garbage(). newFilesItem() drops its now-unused stepSize/ stepsInFrozenFile params; newFilesItemWithSnapConfig and the orphaned SnapshotConfig.StepsInFrozenFile() are removed. Step 2 of #21306.
Only the not-yet-migrated forkable subsystem still uses canDelete; the aggregator reclaims via aggregatorVisible generations (retired + bundle refcnt), so mark the field Deprecated ahead of the forkable migration.
sudeepdino008
approved these changes
Jul 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the FilesItem.frozen flag and all remaining logic that depended on it in db/state, following the earlier shift to bundle-level pinning (via aggregatorVisible) and enabling upcoming pruning work for previously “protected” frozen files.
Changes:
- Eliminates
FilesItem.frozenand related constructor/config helpers (newFilesItemWithSnapConfig,SnapshotConfig.StepsInFrozenFile()). - Removes
frozen-based skips/guards in forkable begin/close paths, garbage collection, and file removal routines (files are now always refcounted / deletable when eligible). - Makes
MergeResult.FilePathsinclude domain-II (dIdx) outputs unconditionally (aligning with other domain file announcements).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| db/state/squeeze.go | Removes frozen mutation prior to removing squeezed commitment files. |
| db/state/snap_repo.go | Switches dirty-file instantiation to the simplified newFilesItem. |
| db/state/snap_repo_test.go | Updates tests to use newFilesItem after constructor simplification. |
| db/state/snap_config.go | Removes now-unused SnapshotConfig.StepsInFrozenFile() helper. |
| db/state/proto_forkable.go | Removes frozen-based refcount skipping; always refcounts visible/dirty files. |
| db/state/merge.go | Updates merge-time FilesItem creation and removes frozen-based garbage skipping. |
| db/state/inverted_index.go | Updates dirty scanning/integration paths for new filterDirtyFiles/newFilesItem signatures. |
| db/state/history.go | Updates dirty scanning/integration paths for new filterDirtyFiles/newFilesItem signatures. |
| db/state/forkable_merge.go | Switches merged-file creation to newFilesItem after helper removal. |
| db/state/forkable_debug.go | Removes frozen conditional; always decrements refcount and deletes when eligible. |
| db/state/domain.go | Removes frozen assignments and updates dirty scanning/integration to new helper signatures. |
| db/state/domain_test.go | Drops frozen-flag invariant checks from domain file property tests. |
| db/state/dirty_files.go | Removes frozen field/logic, simplifies constructors, updates deletion + scanning helpers. |
| db/state/deduplicate.go | Updates dedup output item creation to newFilesItem. |
| db/state/aggregator_files.go | Announces dIdx merge outputs regardless of “frozen” status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Jul 3, 2026
…22123) ## Summary Frozen History/InvertedIndex files never get deleted once merged, so disk usage grows unbounded under `--prune.mode` regardless of setting (erigontech#21306, Step 3). Based on erigontech#22126 (removes `FilesItem.frozen`) — diff includes its commits until that merges. - `Aggregator.RetireOldHistoryFiles` retires History/II files entirely below the retention cutoff, for every domain except `CommitmentDomain` and `RCacheDomain` (their history retention is governed by dedicated flags — `--prune.include-commitment-history` and `--persist.receipts` — not by age), plus the standalone log/trace indices. - Wired into `PruneExecutionStage` via the existing `--prune.mode`/`--prune.distance` — no new flags. - Gated by the same `remainingPruneTimeout()` budget as the other prune sub-steps: `onFilesDelete` can call the downloader over gRPC while the stage holds the live write tx. - Also fixes `staticFilesInRange`: it didn't check source files were contiguous, so a gap could silently merge a shorter range under the full range's name. ## Out of scope CommitmentDomain and RCacheDomain history, download-time filtering, block/tx segments — deferred to a follow-up (erigontech#21198, erigontech#21199). ## Tests `db/state/retire_history_test.go`, `stage_execute_prune_test.go`, `merge_test.go` (gap detection). `make lint` clean; `db/state` + `execution/stagedsync` suites green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 2 of #21306 (prune frozen files).
frozen(a cachedstepCount >= stepsInFrozenFile) existed only to skip per-file refcount atomics inBeginFilesRo(). #21397 moved reader pinning to a single bundle-level refcount (aggregatorVisible) — "one atomic add instead of dozens of per-file adds" — so the field is no longer needed for that optimization.Removed
frozenfield onFilesItem!frozenskip in the forkable / snapshot begin+close paths (files are now always refcounted)closeFilesAndRemoveand thefrozenskip ingarbage()— this is the unprotection Step 3 (prune state history files) needsnewFilesItem()'s now-unusedstepSize/stepsInFrozenFileparams,newFilesItemWithSnapConfig, and the orphanedSnapshotConfig.StepsInFrozenFile()Behavior note
The merged domain-II (
dIdx) downloader-seed filter inMergeResult.FilePathspreviously depended onfrozen; it is now unconditional, matching how domain-value and domain-history merged files were already announced.Out of scope (follow-ups)
stepsInFrozenFilefields are now write-only; removing them cleanly unwinds the exportedNewDomain/NewHistory/NewInvertedIndexsignatures (external caller incmd/integration+ tests), so it is left for a separate change.db/snapshotsync's ownfrozen(onDirtySegment) is untouched — its begin path still uses per-file refcounting (Step 1 not yet done there).