diff --git a/eth/protocols/snap/syncv2.go b/eth/protocols/snap/syncv2.go index 4c7622858b..1a8fd36b9c 100644 --- a/eth/protocols/snap/syncv2.go +++ b/eth/protocols/snap/syncv2.go @@ -448,17 +448,8 @@ func newSyncerV2(db ethdb.Database, scheme string) *syncerV2 { if raw := rawdb.ReadSnapshotSyncStatus(db); len(raw) > 0 && raw[0] == syncProgressVersion { var progress syncProgressV2 if err := json.Unmarshal(raw[1:], &progress); err == nil { - // A completed journal whose pivot block was already committed is - // a dead leftover. Drop it before FrozenPivot hands the stale - // pivot back to the downloader. The flat state stays, on a - // full-sync node it is the live snapshot. - if progress.Phase == phaseComplete && progress.Pivot != nil && isPivotCommitted(db, progress.Pivot) { - log.Info("Dropping journal of a completed snap sync", "pivot", progress.Pivot.Number) - rawdb.DeleteSnapshotSyncStatus(db) - } else { - s.pivot = progress.Pivot - s.setPhase(progress.Phase) - } + s.pivot = progress.Pivot + s.setPhase(progress.Phase) } } return s @@ -564,11 +555,10 @@ func (s *syncerV2) Sync(target *types.Header, cancel chan struct{}) error { // If the pivot of a completed sync was already committed, full sync has // been mutating the flat state since. Nothing here can be resumed or // rolled forward, snap sync was re-enabled, so wipe and start fresh. - // newSyncerV2 already drops such a journal on load, so this branch only - // fires for an in-process re-enable, where the syncer was built before - // the sync completed so the load-time check never saw a committed journal. - // Same-pivot retries return through the skip above on purpose, a cycle - // that dies right after the commit just needs to recommit. + // FrozenPivot refuses to freeze such a pivot, so the downloader resumes + // against a fresh target and lands here to do the cleanup. Same-pivot + // retries return through the skip above on purpose, a cycle that dies + // right after the commit just needs to recommit. if prevPivot != nil && s.getPhase() == phaseComplete && isPivotCommitted(s.db, prevPivot) { log.Warn("Reenabled snap sync over a completed one, restarting from scratch", "oldpivot", prevPivot.Number, "target", target.Number) s.resetSyncState() @@ -701,6 +691,12 @@ func (s *syncerV2) FrozenPivot() *types.Header { } s.lock.RLock() defer s.lock.RUnlock() + // A committed pivot from a completed sync is a dead leftover, not a + // freeze. Handing it back would pin a re-enabled sync to the old pivot, + // so report it as unfrozen and let the next Sync wipe the journal. + if s.getPhase() == phaseComplete && s.pivot != nil && isPivotCommitted(s.db, s.pivot) { + return nil + } return s.pivot } diff --git a/eth/protocols/snap/syncv2_test.go b/eth/protocols/snap/syncv2_test.go index bc801e2518..5dee03b989 100644 --- a/eth/protocols/snap/syncv2_test.go +++ b/eth/protocols/snap/syncv2_test.go @@ -2432,40 +2432,42 @@ func testCommittedPivotRetrySkips(t *testing.T, scheme string) { } } -// TestReenableDropsCompletedJournal covers the constructor. A restart with -// a committed completed journal must drop it before FrozenPivot hands the -// stale pivot to the downloader. A journal from a crash before the commit -// stays. -func TestReenableDropsCompletedJournal(t *testing.T) { +// TestCommittedPivotNotFrozen covers FrozenPivot. A pivot from a crash +// before the commit must stay frozen so the downloader resumes against it. +// Once the pivot block is committed and the head has moved on, FrozenPivot +// must report it as unfrozen so a re-enabled sync is not pinned to it. The +// journal stays on disk either way, the next Sync cleans it. +func TestCommittedPivotNotFrozen(t *testing.T) { t.Parallel() - testReenableDropsCompletedJournal(t, rawdb.HashScheme) - testReenableDropsCompletedJournal(t, rawdb.PathScheme) + testCommittedPivotNotFrozen(t, rawdb.HashScheme) + testCommittedPivotNotFrozen(t, rawdb.PathScheme) } -func testReenableDropsCompletedJournal(t *testing.T, scheme string) { +func testCommittedPivotNotFrozen(t *testing.T, scheme string) { fix := seedCompletedSync(t, scheme) - // Crash before the commit, the journal survives and the pivot stays - // frozen. + // Crash before the commit, the head never reached the pivot, so it + // stays frozen for the resume. head50 := fix.mkHeader(50, common.HexToHash("0x50"), nil) rawdb.WriteHeadBlockHash(fix.db, head50.Hash()) syncer := newSyncerV2(fix.db, fix.nodeScheme) if frozen := syncer.FrozenPivot(); frozen == nil || frozen.Hash() != fix.pivotA.Hash() { - t.Fatal("expected journal to survive a restart before the pivot commit") + t.Fatal("expected the pivot to stay frozen before the commit") } - // Commit the pivot and advance the head, a restart now drops the - // journal. + // Commit the pivot and advance the head, the pivot is now a dead + // leftover and must not be reported as frozen. header103 := fix.mkHeader(103, common.HexToHash("0x67"), nil) rawdb.WriteHeadBlockHash(fix.db, header103.Hash()) syncer = newSyncerV2(fix.db, fix.nodeScheme) if frozen := syncer.FrozenPivot(); frozen != nil { - t.Fatalf("expected journal drop to release the pivot freeze, got %v", frozen.Number) + t.Fatalf("expected the committed pivot to be unfrozen, got %v", frozen.Number) } - if raw := rawdb.ReadSnapshotSyncStatus(fix.db); len(raw) != 0 { - t.Fatal("expected completed journal to be dropped from disk") + // The journal stays on disk until a re-enabled Sync wipes it, and the + // flat state is untouched, on a full-sync node it is the live snapshot. + if raw := rawdb.ReadSnapshotSyncStatus(fix.db); len(raw) == 0 { + t.Fatal("expected the journal to stay on disk") } - // The flat state stays, on a full-sync node it is the live snapshot. assertAccountBalance(t, fix.db, fix.hashY, 50) }