From aaefc0c006019918d56879ce6b133048cc0638e5 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Mon, 25 May 2026 13:02:40 +0700 Subject: [PATCH 1/2] db/state: only last reader deletes merged files deleteMergeFile no longer removes files eagerly. It removes the item from dirtyFiles and marks canDelete=true; physical removal is left to the last reader inside RoTx.Close. The merge holds a rotx pinning these files, so it is itself such a reader. --- db/state/dirty_files.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/db/state/dirty_files.go b/db/state/dirty_files.go index fa3a8ade50c..ca68aa97940 100644 --- a/db/state/dirty_files.go +++ b/db/state/dirty_files.go @@ -360,18 +360,10 @@ func deleteMergeFile(dirtyFiles *DirtyFiles, outs []*FilesItem, filenameBase str dirtyFiles.Delete(out) out.canDelete.Store(true) - // if merged file not visible for any alive reader (even for us): can remove it immediately - // otherwise: mark it as `canDelete=true` and last reader of this file - will remove it inside `aggRoTx.Close()` - if out.refcount.Load() == 0 { - out.closeFilesAndRemove() - - if filenameBase == traceFileLife && out.decompressor != nil { - logger.Warn("[agg.dbg] deleteMergeFile: remove", "f", out.decompressor.FileName()) - } - } else { - if filenameBase == traceFileLife && out.decompressor != nil { - logger.Warn("[agg.dbg] deleteMergeFile: mark as canDelete=true", "f", out.decompressor.FileName()) - } + // Mark `canDelete=true` only. The last reader of this file removes it inside RoTx.Close. + // The merge holds a rotx pinning these files, so it is itself such a reader. + if filenameBase == traceFileLife && out.decompressor != nil { + logger.Warn("[agg.dbg] deleteMergeFile: mark as canDelete=true", "f", out.decompressor.FileName()) } } } From 3dc1e1b7a9d469a8a39b5fadb0c7d3b0aad738aa Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Mon, 25 May 2026 15:23:07 +0700 Subject: [PATCH 2/2] db/state: pin dirty files during cleanAfterMerge so last reader unlinks them Since deleteMergeFile no longer unlinks files eagerly, a subsumed file with no live reader (e.g. RemoveOverlapsAfterMerge, or files already non-visible by the final merge step) would be marked canDelete but never closed - leaking the FD and leaving the file on disk. POSIX hides this via unlink-while-open; Windows fails RemoveAll with "Access is denied". cleanAfterMerge now holds a dirty-files rotx (DebugBeginDirtyFilesRo) pinning the subsumed files, so its Close is a guaranteed last reader that unlinks any file no other reader still holds. Add a portable regression test asserting subsumed files are unlinked from disk after RemoveOverlapsAfterMerge. --- db/state/aggregator.go | 6 ++++ db/state/dirty_files.go | 4 +-- db/state/merge_cleanup_test.go | 57 ++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 db/state/merge_cleanup_test.go diff --git a/db/state/aggregator.go b/db/state/aggregator.go index 0ce0f36bcd2..7a81a75d01b 100644 --- a/db/state/aggregator.go +++ b/db/state/aggregator.go @@ -2011,6 +2011,12 @@ func (a *Aggregator) IntegrateMergedDirtyFiles(in *MergeResult) { func (a *Aggregator) cleanAfterMerge(in *MergeResult) { var deleted []string + // Pin every dirty file so this cleanup is itself a reader of the subsumed files. + // deleteMergeFile only marks them canDelete; dirtyRo.Close is then the last reader + // that unlinks any file no other reader still holds. Deferred first so it runs last. + dirtyRo := a.DebugBeginDirtyFilesRo() + defer dirtyRo.Close() + at := a.BeginFilesRo() defer at.Close() diff --git a/db/state/dirty_files.go b/db/state/dirty_files.go index ca68aa97940..db93bf2575b 100644 --- a/db/state/dirty_files.go +++ b/db/state/dirty_files.go @@ -360,8 +360,8 @@ func deleteMergeFile(dirtyFiles *DirtyFiles, outs []*FilesItem, filenameBase str dirtyFiles.Delete(out) out.canDelete.Store(true) - // Mark `canDelete=true` only. The last reader of this file removes it inside RoTx.Close. - // The merge holds a rotx pinning these files, so it is itself such a reader. + // Mark `canDelete=true` only. The last reader removes the file inside RoTx.Close; + // callers must hold a rotx pinning these files so such a reader is guaranteed to exist. if filenameBase == traceFileLife && out.decompressor != nil { logger.Warn("[agg.dbg] deleteMergeFile: mark as canDelete=true", "f", out.decompressor.FileName()) } diff --git a/db/state/merge_cleanup_test.go b/db/state/merge_cleanup_test.go new file mode 100644 index 00000000000..0e4b1eec50d --- /dev/null +++ b/db/state/merge_cleanup_test.go @@ -0,0 +1,57 @@ +// Copyright 2024 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package state + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dir" +) + +// TestCleanAfterMerge_UnlinksSubsumedFiles guards the "last reader removes the file" invariant. +// deleteMergeFile no longer unlinks files eagerly, so cleanup must hold a rotx pinning the +// subsumed files; otherwise their FDs stay open and the files linger on disk. POSIX hides this +// (unlink-while-open succeeds) but Windows locks open files, so the check is on-disk presence. +func TestCleanAfterMerge_UnlinksSubsumedFiles(t *testing.T) { + t.Parallel() + const stepSize = uint64(10) + _, agg := testDbAndAggregatorv3(t, stepSize) + dirs := agg.Dirs() + + // 0-1 and 1-2 are proper subsets of 0-2; no external reader pins them. + ranges := []testFileRange{{0, 1}, {1, 2}, {0, 2}} + generateAccountsFile(t, dirs, ranges) + generateStorageFile(t, dirs, ranges) + generateCodeFile(t, dirs, ranges) + generateCommitmentFile(t, dirs, ranges) + require.NoError(t, agg.OpenFolder()) + + require.NoError(t, agg.RemoveOverlapsAfterMerge(t.Context())) + + files, err := dir.ListFiles(dirs.SnapDomain, ".kv") + require.NoError(t, err) + var leaked []string + for _, f := range files { + if strings.Contains(f, ".0-1.") || strings.Contains(f, ".1-2.") { + leaked = append(leaked, f) + } + } + require.Empty(t, leaked, "subsumed files must be unlinked from disk, got leaked: %v", leaked) +}