Skip to content

Commit 16b3302

Browse files
committed
db/state: fix spurious accessor rebuild for post-merge subset files
After a merge, the old sub-range .ef files stay on disk while held open by active readers. OpenFolder/scanDirtyFiles re-adds them to dirtyFiles. BuildMissedAccessors then found these re-added items had no .efi accessor (already deleted as part of merge cleanup) and rebuilt them, producing orphaned .efi files at paths that dir.trackRemovedFiles had been told were deleted, triggering repeated "Removed file unexpectedly exists" warnings. Fix: skip items in fileItemsWithMissedAccessors that are proper subsets of another item already in dirtyFiles — their accessor should not be rebuilt because the superset file supersedes them. Add ERIGON_AGG_TRACE_FILE_LIFE logging to observe when the condition fires: Debug by default; set the env var to empty to escalate all to Warn, or to a substring to filter by filename base. Also add dbg.EnvStringLookup to distinguish "not set" from "set to empty string". Fixes #19797
1 parent 2d8450f commit 16b3302

7 files changed

Lines changed: 55 additions & 14 deletions

File tree

common/dbg/dbg_env.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ func EnvString(envVarName string, defaultVal string) string {
5959
return defaultVal
6060
}
6161

62+
// EnvStringLookup returns the value of the env var and whether it was set.
63+
// Unlike EnvString, it distinguishes between "not set" and "set to empty string".
64+
func EnvStringLookup(envVarName string) (string, bool) {
65+
return envLookup(envVarName)
66+
}
67+
6268
func EnvStrings(envVarName string, sep string, defaultVal []string) []string {
6369
if v, _ := envLookup(envVarName); v != "" {
6470
return strings.Split(v, sep)

db/state/dirty_files.go

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -806,8 +806,42 @@ func readDirNames(dir string) dirListing {
806806

807807
// fileItemsWithMissedAccessors returns list of files with missed accessors
808808
// here "accessors" are generated dynamically by `accessorsFor`
809-
func fileItemsWithMissedAccessors(dirtyFiles []*FilesItem, aggregationStep uint64, accessorsFor func(fromStep, toStep kv.Step) []string) (l []*FilesItem) {
810-
for _, item := range dirtyFiles {
809+
func fileItemsWithMissedAccessors(
810+
dirtyFiles []*FilesItem,
811+
aggregationStep uint64,
812+
accessorsFor func(fromStep, toStep kv.Step) []string,
813+
filenameBase string,
814+
logger log.Logger,
815+
) (l []*FilesItem) {
816+
for i, item := range dirtyFiles {
817+
// Skip items that are proper subsets of other dirty files.
818+
// After a merge, old sub-range data files stay on disk while held open by
819+
// active readers; OpenFolder/scanDirtyFiles re-adds them to dirtyFiles.
820+
// Rebuilding their accessors creates orphaned .efi files and triggers false
821+
// "Removed file unexpectedly exists" warnings.
822+
var coveringItem *FilesItem
823+
for j, other := range dirtyFiles {
824+
if i != j && item.isProperSubsetOf(other) {
825+
coveringItem = other
826+
break
827+
}
828+
}
829+
if coveringItem != nil {
830+
if traceFileLifeSet && (traceFileLife == "" || strings.Contains(filenameBase, traceFileLife)) {
831+
logger.Warn("[agg.dbg] fileItemsWithMissedAccessors: skipping subset item",
832+
"f", filenameBase,
833+
"item", fmt.Sprintf("%d-%d", item.startTxNum/aggregationStep, item.endTxNum/aggregationStep),
834+
"coveredBy", fmt.Sprintf("%d-%d", coveringItem.startTxNum/aggregationStep, coveringItem.endTxNum/aggregationStep),
835+
)
836+
} else {
837+
logger.Debug("[agg.dbg] fileItemsWithMissedAccessors: skipping subset item",
838+
"f", filenameBase,
839+
"item", fmt.Sprintf("%d-%d", item.startTxNum/aggregationStep, item.endTxNum/aggregationStep),
840+
"coveredBy", fmt.Sprintf("%d-%d", coveringItem.startTxNum/aggregationStep, coveringItem.endTxNum/aggregationStep),
841+
)
842+
}
843+
continue
844+
}
811845
for _, fName := range accessorsFor(item.StepRange(aggregationStep)) {
812846
exists, err := dir.FileExist(fName)
813847
if err != nil {

db/state/dirty_files_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
btree2 "github.com/tidwall/btree"
1111

1212
"github.com/erigontech/erigon/common/dir"
13+
"github.com/erigontech/erigon/common/log/v3"
1314
"github.com/erigontech/erigon/db/kv"
1415
)
1516

@@ -93,7 +94,7 @@ func TestFileItemWithMissedAccessor(t *testing.T) {
9394
defer dir.RemoveFile(fname)
9495
}
9596

96-
fileItems := fileItemsWithMissedAccessors(btree.Items(), aggStep, accessorFor)
97+
fileItems := fileItemsWithMissedAccessors(btree.Items(), aggStep, accessorFor, "", log.New())
9798
require.Len(t, fileItems, 1)
9899
require.Equal(t, f3, fileItems[0])
99100
}

db/state/domain.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ import (
5656
)
5757

5858
var (
59-
asserts = dbg.EnvBool("AGG_ASSERTS", false)
60-
traceFileLife = dbg.EnvString("AGG_TRACE_FILE_LIFE", "")
59+
asserts = dbg.EnvBool("AGG_ASSERTS", false)
60+
traceFileLife, traceFileLifeSet = dbg.EnvStringLookup("AGG_TRACE_FILE_LIFE")
6161
traceGetAsOf = dbg.EnvString("AGG_TRACE_GET_AS_OF", "")
6262
tracePutWithPrev = dbg.EnvString("AGG_TRACE_PUT_WITH_PREV", "")
6363
)
@@ -1129,7 +1129,7 @@ func (d *Domain) missedBtreeAccessors(source []*FilesItem, dl dirListing) (l []*
11291129
panic(err)
11301130
}
11311131
return []string{btF, exF}
1132-
})
1132+
}, d.FilenameBase, d.logger)
11331133
}
11341134

11351135
func (d *Domain) MissedMapAccessors() (l []*FilesItem) {
@@ -1146,7 +1146,7 @@ func (d *Domain) missedMapAccessors(source []*FilesItem, dl dirListing) (l []*Fi
11461146
panic(err)
11471147
}
11481148
return []string{fPath}
1149-
})
1149+
}, d.FilenameBase, d.logger)
11501150
//return fileItemsWithMissedAccessors(source, d.stepSize, func(fromStep, toStep uint64) []string {
11511151
// var files []string
11521152
// if d.Accessors.Has(AccessorHashMap) {

db/state/history.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func (h *History) missedMapAccessors(source []*FilesItem, dl dirListing) (l []*F
208208
panic(err)
209209
}
210210
return []string{fPath}
211-
})
211+
}, h.FilenameBase, h.logger)
212212
}
213213

214214
func (h *History) buildVi(ctx context.Context, item *FilesItem, ps *background.ProgressSet) (err error) {

db/state/inverted_index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func (ii *InvertedIndex) missedMapAccessors(source []*FilesItem, dl dirListing)
258258
panic(err)
259259
}
260260
return []string{fPath}
261-
})
261+
}, ii.FilenameBase, ii.logger)
262262
}
263263

264264
func (ii *InvertedIndex) buildEfAccessor(ctx context.Context, item *FilesItem, ps *background.ProgressSet) (err error) {

db/state/snap_repo.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func (f *SnapshotRepo) DirtyFilesWithNoBtreeAccessors() (l []*FilesItem) {
181181
fname, _ := p.BtIdxFile(v, from, to)
182182
existenceFile, _ := p.ExistenceFile(v, from, to)
183183
return []string{fname, existenceFile}
184-
})
184+
}, f.schema.DataTag(), f.logger)
185185
}
186186

187187
func (f *SnapshotRepo) DirtyFilesWithNoHashAccessors() (l []*FilesItem) {
@@ -199,7 +199,7 @@ func (f *SnapshotRepo) DirtyFilesWithNoHashAccessors() (l []*FilesItem) {
199199
files[i], _ = p.AccessorIdxFile(v, RootNum(fromStep.ToTxNum(ss)), RootNum(toStep.ToTxNum(ss)), i)
200200
}
201201
return files
202-
})
202+
}, f.schema.DataTag(), f.logger)
203203
}
204204

205205
func (f *SnapshotRepo) EndRootNum() RootNum {
@@ -331,23 +331,23 @@ func (f *SnapshotRepo) FilesWithMissedAccessors() *MissedFilesMap {
331331
fileItemsWithMissedAccessors(f.dirtyFiles.Items(), f.stepSize, func(fromStep, toStep kv.Step) []string {
332332
file, _ := f.schema.BtIdxFile(version.V1_0, RootNum(fromStep*kv.Step(f.stepSize)), RootNum(toStep*kv.Step(f.stepSize)))
333333
return []string{file}
334-
})
334+
}, f.schema.DataTag(), f.logger)
335335
}
336336

337337
if f.accessors.Has(statecfg.AccessorHashMap) {
338338
mf[statecfg.AccessorHashMap] =
339339
fileItemsWithMissedAccessors(f.dirtyFiles.Items(), f.stepSize, func(fromStep, toStep kv.Step) []string {
340340
file, _ := f.schema.AccessorIdxFile(version.V1_0, RootNum(fromStep*kv.Step(f.stepSize)), RootNum(toStep*kv.Step(f.stepSize)), 0)
341341
return []string{file}
342-
})
342+
}, f.schema.DataTag(), f.logger)
343343
}
344344

345345
if f.accessors.Has(statecfg.AccessorExistence) {
346346
mf[statecfg.AccessorExistence] =
347347
fileItemsWithMissedAccessors(f.dirtyFiles.Items(), f.stepSize, func(fromStep, toStep kv.Step) []string {
348348
file, _ := f.schema.ExistenceFile(version.V1_0, RootNum(fromStep*kv.Step(f.stepSize)), RootNum(toStep*kv.Step(f.stepSize)))
349349
return []string{file}
350-
})
350+
}, f.schema.DataTag(), f.logger)
351351
}
352352

353353
return (*MissedFilesMap)(&mf)

0 commit comments

Comments
 (0)