diff --git a/db/snapshotsync/caplin_state_snapshots.go b/db/snapshotsync/caplin_state_snapshots.go index 386ecdae6d5..b4089b9a9f6 100644 --- a/db/snapshotsync/caplin_state_snapshots.go +++ b/db/snapshotsync/caplin_state_snapshots.go @@ -316,32 +316,15 @@ func (s *CaplinStateSnapshots) OpenList(fileNames []string, optimistic bool) err s.closeWhatNotInList(fileNames) var segmentsMax uint64 var segmentsMaxSet bool -Loop: for _, fName := range fileNames { f, _, _ := snaptype.ParseFileName(s.dir, fName) - var processed bool = true - var exists bool - var sn *DirtySegment - dirtySegments, ok := s.dirty[f.CaplinTypeString] if !ok { continue } filePath := filepath.Join(s.dir, fName) - dirtySegments.Walk(func(segments []*DirtySegment) bool { - for _, sn2 := range segments { - if sn2.Decompressor == nil { // it's ok if some segment was not able to open - continue - } - if filePath == sn2.filePath { - sn = sn2 - exists = true - break - } - } - return true - }) + sn, exists := findOpenSegment(dirtySegments, func(sn2 *DirtySegment) bool { return sn2.filePath == filePath }) if !exists { sn = &DirtySegment{ // segType: f.Type, Unsupported @@ -352,19 +335,17 @@ Loop: } } if err := s.openSegIfNeed(sn, filePath); err != nil { - if errors.Is(err, os.ErrNotExist) { - if optimistic { - continue Loop - } else { - break Loop - } + stop, failErr := ClassifyOpenErr(err, optimistic) + if failErr != nil { + return failErr } - if optimistic { + if stop { + break + } + if !errors.Is(err, os.ErrNotExist) { s.logger.Warn("[snapshots] open segment", "err", err) - continue Loop - } else { - return err } + continue } if !exists { @@ -375,15 +356,12 @@ Loop: if err := openIdxForCaplinStateIfNeeded(sn, filePath, optimistic); err != nil { return err } - // Only bob sidecars count for progression - if processed { - if f.To > 0 { - segmentsMax = f.To - 1 - } else { - segmentsMax = 0 - } - segmentsMaxSet = true + if f.To > 0 { + segmentsMax = f.To - 1 + } else { + segmentsMax = 0 } + segmentsMaxSet = true } if segmentsMaxSet { @@ -600,24 +578,12 @@ func (s *CaplinStateSnapshots) closeWhatNotInList(l []string) { } for _, dirtySegments := range s.dirty { - toClose := make([]*DirtySegment, 0) - dirtySegments.Walk(func(segments []*DirtySegment) bool { - for _, sn := range segments { - if sn.Decompressor == nil { - continue - } - _, name := filepath.Split(sn.FilePath()) - if _, ok := protectFiles[name]; ok { - continue - } - toClose = append(toClose, sn) - } - return true + closeAndDropNotProtected(dirtySegments, protectFiles, func(sn *DirtySegment) string { + // sn.filePath, not the promoted Decompressor.FilePath(): the field is + // set for every tree member, incl. stubs whose Decompressor is nil. + _, name := filepath.Split(sn.filePath) + return name }) - for _, sn := range toClose { - sn.close() - dirtySegments.Delete(sn) - } } } diff --git a/db/snapshotsync/caplin_state_snapshots_test.go b/db/snapshotsync/caplin_state_snapshots_test.go new file mode 100644 index 00000000000..b7ed527d8e4 --- /dev/null +++ b/db/snapshotsync/caplin_state_snapshots_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 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 snapshotsync + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tidwall/btree" +) + +func TestCaplinStateCloseWhatNotInListDropsUnopenedStub(t *testing.T) { + tree := btree.NewBTreeGOptions[*DirtySegment](DirtySegmentLess, btree.Options{Degree: 4, NoLocks: false}) + kept := &DirtySegment{Range: Range{0, 1000}, filePath: filepath.Join("snapshots", "keep.seg")} + stale := &DirtySegment{Range: Range{1000, 2000}, filePath: filepath.Join("snapshots", "drop.seg")} + tree.Set(kept) + tree.Set(stale) + s := &CaplinStateSnapshots{dirty: map[string]*btree.BTreeG[*DirtySegment]{"test": tree}} + + s.closeWhatNotInList([]string{"keep.seg"}) + + require.Equal(t, 1, tree.Len(), "unopened stale stub must be dropped") + survivor, ok := tree.Min() + require.True(t, ok) + require.Same(t, kept, survivor) +} diff --git a/db/snapshotsync/freezeblocks/caplin_snapshots.go b/db/snapshotsync/freezeblocks/caplin_snapshots.go index 3ffd266540f..57e73ef10a1 100644 --- a/db/snapshotsync/freezeblocks/caplin_snapshots.go +++ b/db/snapshotsync/freezeblocks/caplin_snapshots.go @@ -180,119 +180,51 @@ func (s *CaplinSnapshots) OpenList(fileNames []string, optimistic bool) error { var segmentsMax uint64 var segmentsMaxSet bool -Loop: for _, fName := range fileNames { f, _, ok := snaptype.ParseFileName(s.dir, fName) if !ok { continue } - var processed bool = true - switch f.Type.Enum() { - case snaptype.CaplinEnums.BeaconBlocks: - var sn *snapshotsync.DirtySegment - var exists bool - s.dirty[snaptype.BeaconBlocks.Enum()].Walk(func(segments []*snapshotsync.DirtySegment) bool { - for _, sn2 := range segments { - if sn2.Decompressor == nil { // it's ok if some segment was not able to open - continue - } - if fName == sn2.FileName() { - sn = sn2 - exists = true - break - } - } - return true - }) - if !exists { - sn = snapshotsync.NewDirtySegment( - snaptype.BeaconBlocks, - f.Version, - f.From, f.To, - true) - } - if err := sn.Open(s.dir); err != nil { - if errors.Is(err, os.ErrNotExist) { - if optimistic { - continue Loop - } else { - break Loop - } - } - if optimistic { - s.logger.Warn("[snapshots] open segment", "err", err) - continue Loop - } else { - return err - } - } + typ := f.Type.Enum() + if typ != snaptype.CaplinEnums.BeaconBlocks && typ != snaptype.CaplinEnums.BlobSidecars { + continue + } + tree := s.dirty[typ] - if !exists { - // it's possible to iterate over .seg file even if you don't have index - // then make segment available even if index open may fail - s.dirty[snaptype.BeaconBlocks.Enum()].Set(sn) - } - if err := sn.OpenIdxIfNeed(s.dir, optimistic, dirEntries); err != nil { - return err - } - // Only bob sidecars count for progression - if processed { - if f.To > 0 { - segmentsMax = f.To - 1 - } else { - segmentsMax = 0 - } - segmentsMaxSet = true + sn, exists := snapshotsync.FindOpenSegment(tree, fName) + if !exists { + sn = snapshotsync.NewDirtySegment(f.Type, f.Version, f.From, f.To, true) + } + if err := sn.Open(s.dir); err != nil { + stop, failErr := snapshotsync.ClassifyOpenErr(err, optimistic) + if failErr != nil { + return failErr } - case snaptype.CaplinEnums.BlobSidecars: - var sn *snapshotsync.DirtySegment - var exists bool - s.dirty[snaptype.BlobSidecars.Enum()].Walk(func(segments []*snapshotsync.DirtySegment) bool { - for _, sn2 := range segments { - if sn2.Decompressor == nil { // it's ok if some segment was not able to open - continue - } - if fName == sn2.FileName() { - sn = sn2 - exists = true - break - } - } - return true - }) - if !exists { - sn = snapshotsync.NewDirtySegment( - snaptype.BlobSidecars, - f.Version, - f.From, f.To, - true) + if stop { + break } - if err := sn.Open(s.dir); err != nil { - if errors.Is(err, os.ErrNotExist) { - if optimistic { - continue Loop - } else { - break Loop - } - } - if optimistic { - s.logger.Warn("[snapshots] open segment", "err", err) - continue Loop - } else { - return err - } + if !errors.Is(err, os.ErrNotExist) { + s.logger.Warn("[snapshots] open segment", "err", err) } + continue + } - if !exists { - // it's possible to iterate over .seg file even if you don't have index - // then make segment available even if index open may fail - s.dirty[snaptype.BlobSidecars.Enum()].Set(sn) - } - if err := sn.OpenIdxIfNeed(s.dir, optimistic, dirEntries); err != nil { - return err + if !exists { + // it's possible to iterate over .seg file even if you don't have index + // then make segment available even if index open may fail + tree.Set(sn) + } + if err := sn.OpenIdxIfNeed(s.dir, optimistic, dirEntries); err != nil { + return err + } + if typ == snaptype.CaplinEnums.BeaconBlocks { + if f.To > 0 { + segmentsMax = f.To - 1 + } else { + segmentsMax = 0 } + segmentsMaxSet = true } - } if segmentsMaxSet { s.segmentsMax.Store(segmentsMax) @@ -339,43 +271,8 @@ func (s *CaplinSnapshots) closeWhatNotInList(l []string) { for _, fName := range l { protectFiles[fName] = struct{}{} } - toClose := make([]*snapshotsync.DirtySegment, 0) - s.dirty[snaptype.BeaconBlocks.Enum()].Walk(func(segments []*snapshotsync.DirtySegment) bool { - for _, sn := range segments { - if sn.Decompressor == nil { - continue - } - _, name := filepath.Split(sn.FilePath()) - if _, ok := protectFiles[name]; ok { - continue - } - toClose = append(toClose, sn) - } - return true - }) - for _, sn := range toClose { - sn.Close() - s.dirty[snaptype.BeaconBlocks.Enum()].Delete(sn) - } - - toClose = make([]*snapshotsync.DirtySegment, 0) - s.dirty[snaptype.BlobSidecars.Enum()].Walk(func(segments []*snapshotsync.DirtySegment) bool { - for _, sn := range segments { - if sn.Decompressor == nil { - continue - } - _, name := filepath.Split(sn.FilePath()) - if _, ok := protectFiles[name]; ok { - continue - } - toClose = append(toClose, sn) - } - return true - }) - for _, sn := range toClose { - sn.Close() - s.dirty[snaptype.BlobSidecars.Enum()].Delete(sn) - } + snapshotsync.CloseSegmentsNotInList(s.dirty[snaptype.BeaconBlocks.Enum()], protectFiles) + snapshotsync.CloseSegmentsNotInList(s.dirty[snaptype.BlobSidecars.Enum()], protectFiles) } type CaplinView struct { diff --git a/db/snapshotsync/snapshots.go b/db/snapshotsync/snapshots.go index 1287b469cbd..a3dd79f646e 100644 --- a/db/snapshotsync/snapshots.go +++ b/db/snapshotsync/snapshots.go @@ -1046,40 +1046,21 @@ func (s *RoSnapshots) openSegments(fileNames []string, open bool, optimistic boo continue } - var sn *DirtySegment - var exists bool - segtype.Walk(func(segs []*DirtySegment) bool { - for _, sn2 := range segs { - if sn2.Decompressor == nil { // it's ok if some segment was not able to open - continue - } - if fName == sn2.FileName() { - sn = sn2 - exists = true - return false - } - } - return true - }) - + sn, exists := FindOpenSegment(segtype, fName) if !exists { sn = &DirtySegment{segType: f.Type, version: f.Version, Range: Range{f.From, f.To}, frozen: s.snCfg.IsFrozen(f)} } if open { if err := sn.Open(s.dir); err != nil { - if errors.Is(err, os.ErrNotExist) { - if optimistic { - continue - } else { - break - } + stop, failErr := ClassifyOpenErr(err, optimistic) + if failErr != nil { + return failErr } - if optimistic { - continue - } else { - return err + if stop { + break } + continue } } @@ -1187,38 +1168,74 @@ func (s *RoSnapshots) closeWhatNotInList(l []string) { for _, f := range l { protectFiles[f] = struct{}{} } - toClose := make(map[snaptype.Enum][]*DirtySegment, 0) for _, t := range s.enums { - s.dirty[t].Walk(func(segs []*DirtySegment) bool { + CloseSegmentsNotInList(s.dirty[t], protectFiles) + } +} - for _, seg := range segs { - if _, ok := protectFiles[seg.FileName()]; ok { - continue - } - if _, ok := toClose[t]; !ok { - toClose[t] = make([]*DirtySegment, 0) - } - toClose[t] = append(toClose[t], seg) +// CloseSegmentsNotInList closes and drops tree segments whose file name is not +// in protectFiles, keeping any that a live reader still references. +func CloseSegmentsNotInList(tree *btree.BTreeG[*DirtySegment], protectFiles map[string]struct{}) { + closeAndDropNotProtected(tree, protectFiles, (*DirtySegment).FileName) +} + +func closeAndDropNotProtected(tree *btree.BTreeG[*DirtySegment], protectFiles map[string]struct{}, nameOf func(*DirtySegment) string) { + var toClose []*DirtySegment + tree.Walk(func(segs []*DirtySegment) bool { + for _, seg := range segs { + if _, ok := protectFiles[nameOf(seg)]; ok { + continue } + toClose = append(toClose, seg) + } + return true + }) - return true - }) + for _, delSeg := range toClose { + if delSeg.refcount.Load() > 0 { + // A live reader (View/RoTx) still holds this segment. Closing it + // now would nil its decompressor out from under that reader and + // turn the reader's later closeAndRemoveFiles into a crash. Leave + // it; it is reaped on a later pass once the reader releases it. + continue + } + delSeg.close() + tree.Delete(delSeg) } +} + +// FindOpenSegment returns tree's already-open segment named fName, if any. +func FindOpenSegment(tree *btree.BTreeG[*DirtySegment], fName string) (*DirtySegment, bool) { + return findOpenSegment(tree, func(sn *DirtySegment) bool { return sn.FileName() == fName }) +} - for segtype, delSegments := range toClose { - dirtyFiles := s.dirty[segtype] - for _, delSeg := range delSegments { - if delSeg.refcount.Load() > 0 { - // A live reader (View/RoTx) still holds this segment. Closing it - // now would nil its decompressor out from under that reader and - // turn the reader's later closeAndRemoveFiles into a crash. Leave - // it; it is reaped on a later pass once the reader releases it. +func findOpenSegment(tree *btree.BTreeG[*DirtySegment], match func(*DirtySegment) bool) (sn *DirtySegment, ok bool) { + tree.Walk(func(segs []*DirtySegment) bool { + for _, sn2 := range segs { + if sn2.Decompressor == nil { // it's ok if some segment was not able to open continue } - delSeg.close() - dirtyFiles.Delete(delSeg) + if match(sn2) { + sn, ok = sn2, true + return false + } } + return true + }) + return sn, ok +} + +// ClassifyOpenErr says how a snapshot-listing loop proceeds after a segment +// open error: stop the whole listing (stop), fail hard (failErr), or, when +// neither, skip just this file. +func ClassifyOpenErr(err error, optimistic bool) (stop bool, failErr error) { + if errors.Is(err, os.ErrNotExist) { + return !optimistic, nil + } + if optimistic { + return false, nil } + return false, err } func (s *RoSnapshots) RemoveOverlaps(onDelete func(l []string) error) error { diff --git a/db/snapshotsync/snapshots_test.go b/db/snapshotsync/snapshots_test.go index d6e5fc47df3..0cc073c6e54 100644 --- a/db/snapshotsync/snapshots_test.go +++ b/db/snapshotsync/snapshots_test.go @@ -24,6 +24,7 @@ import ( "testing/fstest" "github.com/stretchr/testify/require" + "github.com/tidwall/btree" dir2 "github.com/erigontech/erigon/common/dir" "github.com/erigontech/erigon/common/log/v3" @@ -1166,3 +1167,31 @@ func TestOverlapNoTruncation(t *testing.T) { require.Equal(uint64(1_500_000), visibleTxn[1].to) require.Equal(uint64(1_500_000-1), s.SegmentsMax()) } + +func TestCloseAndDropNotProtected(t *testing.T) { + tree := btree.NewBTreeGOptions[*DirtySegment](DirtySegmentLess, btree.Options{Degree: 4, NoLocks: false}) + protected := &DirtySegment{Range: Range{0, 1000}} + held := &DirtySegment{Range: Range{1000, 2000}} + held.refcount.Store(1) + stale := &DirtySegment{Range: Range{2000, 3000}} + tree.Set(protected) + tree.Set(held) + tree.Set(stale) + + closeAndDropNotProtected(tree, map[string]struct{}{"keep": {}}, func(sn *DirtySegment) string { + if sn == protected { + return "keep" + } + return "drop" + }) + + require.Equal(t, 2, tree.Len(), "protected and reader-held segments must survive") + var survivors []*DirtySegment + tree.Walk(func(segs []*DirtySegment) bool { + survivors = append(survivors, segs...) + return true + }) + require.Contains(t, survivors, protected) + require.Contains(t, survivors, held, "segment with live readers must not be dropped") + require.NotContains(t, survivors, stale) +}