Commit 60e7e76
refactor: simplify snap+state sync dispatcher (#11102)
* refactor: replace snap+state sync dispatcher with SimpleDispatcher
- Introduce SimpleDispatcher that runs feeds sequentially (snap → state)
replacing the old parallel SyncDispatcher<T> for snap+state sync.
- Merge SyncMode.SnapSync into SyncMode.StateNodes; sequencing is now
handled by StateSyncRunner rather than the mode selector.
- Move isCloseToHead gate and DB tuning from mode selector / SyncDbTuner
into StateSyncRunner. DB tune reset wrapped in try/finally so it runs
on cancellation.
- StateSyncRunner early-returns when FindBestFullState != 0 (state
already present); Run wraps RunRound (reset pivot, run feed, verify
trie) plus the production-only waits.
- StateSyncFeed / SnapSyncFeed now implement ISimpleSyncFeed; return
null signals round finished, dispatcher drains in-flight requests
with CancellationToken.None so peer allocations are always freed.
- StateSyncFeed/Downloader/AllocationStrategyFactory registered via DI
and resolved by StateSyncRunner instead of being new'd per round.
- TreeSync.ValidatePrepareRequest collapsed to bool IsSyncRoundFinished;
the SyncMode parameter and _syncMode field are gone.
- Remove IsSnapGetRangesFinished from ISyncProgressResolver.
- Test harness SafeContext.RunFeed now calls IStateSyncRunner.RunRound;
feed-level tests resolve StateSyncFeed / StateSyncDownloader via DI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: yield state sync between rounds to MultiSyncModeSelector
Restore the old "pause when mode leaves StateNodes" behaviour at the
round boundary: StateSyncRunner.RunRound now awaits
syncModeSelector.WaitUntilMode(StateNodes) at the top of each iteration,
so beacon control / UpdatingPivot / fast-sync re-entry can claim peers
between rounds. Pivot changes mid-round are still handled by the feed
returning null via IsSyncRoundFinished.
Test infra substitutes ISyncModeSelector with one that reports
StateNodes immediately — the tests drive RunRound directly and shouldn't
depend on the real MultiSyncModeSelector settling.
Mid-round yielding (within a single RunFeed call) is not addressed here;
the feed's PrepareRequest doesn't consult the mode selector to avoid
complicating the test harness. Cost is bounded: pivot moves still end
the round naturally, and all currently-observed mode transitions that
matter (beacon reorg, pivot updates) also move the pivot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: make SimpleDispatcher<T> generic, bind feed/downloader at construction
Push T to the class level and move feed, downloader, allocation strategy
factory, and AllocationContexts to the constructor — each dispatcher is
now bound to the single feed it drives, and its entry point is
Run(CancellationToken) rather than a generic RunFeed<T>(...).
Register SimpleDispatcher<StateSyncBatch?> and SimpleDispatcher<SnapSyncBatch?>
as singletons via factory delegates in the SynchronizerModule (feed +
downloader + strategy factory are also DI-resolved). State/SnapSyncRunner
now inject the dispatcher instead of constructing it per call.
Also fix the stale class summary (no more WaitForMode) and the stale cref
on ISimpleSyncFeed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: register and resolve sync feed deps via interfaces
Register StateSyncFeed / SnapSyncFeed as ISimpleSyncFeed<T>, downloaders
as ISyncDownloader<T>, strategy factories as IPeerAllocationStrategyFactory<T>,
and resolve the same interfaces from the SimpleDispatcher factory. Tests
that directly touched the feed likewise resolve ISimpleSyncFeed<StateSyncBatch>
and ISyncDownloader<StateSyncBatch> instead of the concrete types.
Normalize the generic parameter to the non-nullable batch type
(StateSyncBatch / SnapSyncBatch) and add `where T : class` to
ISimpleSyncFeed<T> / SimpleDispatcher<T> so the interface and
implementation generics line up (previously ISimpleSyncFeed<StateSyncBatch?>
mismatched ISyncDownloader<StateSyncBatch>, forcing concrete resolution).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: remove unused StateSync using in StateSyncFeedTests
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address claude review findings on snap/state sync dispatcher
- SnapSyncFeed: catch OperationCanceledException before general Exception
so clean shutdown doesn't log ERROR
- SimpleDispatcher: remove token from Task.Run to prevent semaphore leak
if cancellation fires between WaitAsync and task scheduling
- StateSyncRunner: replace LINQ with foreach in WaitForCloseToHead
- SafeContext: add Feed property so tests use ctx.Feed instead of
container.Resolve<ISimpleSyncFeed<StateSyncBatch>>
* fix: swallow OCE on shutdown; retry transient SnapSyncFeed errors
- StateSyncRunner.Run wraps the body in try/catch (OperationCanceledException)
when token.IsCancellationRequested, so clean shutdown no longer
propagates OCE to Synchronizer.StartSnapAndStateSyncComponents'
ContinueWith handler and logs "State sync failed" at ERROR.
- SnapSyncFeed.PrepareRequest moves try/catch inside the while loop
with a dedicated catch (OperationCanceledException) returning null,
matching StateSyncFeed's shape. Transient exceptions from
_snapProvider.IsFinished() now retry on the next iteration instead of
permanently aborting snap sync by signalling done.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: rename RunRound -> RunStateSyncRounds; extract StateSyncPrecursorWait
- Rename `IStateSyncRunner.RunRound` to `RunStateSyncRounds` (multiple
rounds, plural).
- Combine the WaitUntilMode(StateNodes) and WaitForCloseToHead calls
into a single private method `StateSyncPrecursorWait` and use it both
at the top of `Run` and at the top of every `RunStateSyncRounds`
iteration. Yields between rounds when MSMS leaves StateNodes or the
node drifts away from head.
- Test harness: also substitute ISyncProgressResolver and
IBeaconSyncStrategy so the close-to-head check returns immediately
(default test peers have HeadNumber=0 which would never satisfy the
real predicate).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: drop unused ITreeSync interface and SyncCompleted event
The SyncCompleted event was fired by TreeSync but no longer had any
subscribers after VerifyStateOnStateSyncFinished was deleted and the
verify-trie call moved into StateSyncRunner.RunStateSyncRounds.
ITreeSync existed only to expose this event, so remove the whole
surface.
- Delete ITreeSync.cs and SyncCompletedEventArgs.
- Drop the event field and its invocation in TreeSync.VerifyPostSyncCleanUp.
- DI: AddSingleton<ITreeSync, TreeSync>() -> AddSingleton<TreeSync>().
- Drop the now-unused Substitute.For<ITreeSync>() in
SynchronizerModuleTests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(snap-sync): drive FlatSnapTrieFactory init/finalize from SnapSyncRunner.Run (#11472)
* test(snap-sync): add Windows-flake stress reproducer
Adds a 30-iteration stress reproducer for the Windows-only
E2ESyncTests.SnapSync flake observed in CI (run #25139586200,
job #73686059011 on PR #11422).
Each iteration is its own NUnit case so a single failure does not
terminate the rest, no [Retry] so every flake surfaces, and
[Explicit] so it does not run in normal CI. Scoped to DbMode.Flat
where the flake has been observed.
Local reproduction on Windows (~1m runtime, 60 iterations across the
two Flat fixtures): 9-19 failures with both observed CI patterns —
"Verification failed: ... N missing in flat" and
"TrieNodeException: Failed to load key X from root hash Y" during
post-snap-sync block processing.
Run: dotnet test --project Nethermind.Synchronization.Test \
-c release -- --filter "FullyQualifiedName~SnapSync_StressRepro"
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(snap-sync): drive FlatSnapTrieFactory init/finalize from SnapSyncRunner.Run
Replaces the lazy-init lock + `_initialized` flag in `FlatSnapTrieFactory`
(which raced against concurrent `CreateStateTree`/`CreateStorageTree` calls
and caused `Clear()` to wipe freshly-written accounts) with explicit
sequential lifecycle hooks driven by `SnapSyncRunner.Run`:
- `ISnapTrieFactory.EnsureInitialize()` (default no-op) — called once before
any tree is created. `FlatSnapTrieFactory` clears the flat DB here.
- `ISnapTrieFactory.FinalizeSync()` (default no-op) — called once after the
dispatcher has finished. `FlatSnapTrieFactory` flushes the persistence here.
- `SnapSyncRunner.Run` brackets `dispatcher.Run` with both calls.
Because both halves run from a single sequential entry point — and the new
`SimpleDispatcher.Run` only returns once every batch handler (and therefore
every `ISnapTree.Dispose`) has completed — the implementation needs no
internal locking and the `_initialized` flag goes away. This is the in-tree
equivalent of the master-branch fix in #11443; the runner-driven shape
suggested by @asdacap is cleaner because it doesn't depend on the factory
guessing when "the first call" happens.
`PatriciaSnapTrieFactory` and the test factories pick up the empty default
implementations of both new methods.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(snap-sync): extract RunSnapSyncOnce shared helper
`SnapSync` and `SnapSync_StressRepro` had verbatim-copied bodies; extract a
private `RunSnapSyncOnce(CancellationToken)` helper and have both call it.
Trim the multi-line stress-repro comment to a single line per coding-style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(snap-sync): SnapSyncRunner lifecycle ordering test + cancel/throw cases
Adds an internal-visible Func<CancellationToken, Task> ctor on SnapSyncRunner
so the lifecycle can be exercised without standing up a real SimpleDispatcher,
and three NSubstitute-based tests:
- happy path: EnsureInitialize → dispatcher → FinalizeSync
- dispatcher throws: FinalizeSync still runs
- dispatcher cancelled: FinalizeSync still runs
Trim the doc comments on the new ISnapTrieFactory hooks to a single line per
the repo's coding-style rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(snap-sync): make SnapSyncRunner Func ctor public
The Func<CancellationToken, Task> overload was internal so the test could
reach it via InternalsVisibleTo. Make it public — it's a legitimate way to
construct the runner, the surface is small, and DI still resolves the
SimpleDispatcher overload because Func<CancellationToken, Task> isn't a
registered service.
Promotes the public ctor pattern to use a primary constructor (matches
the rest of the codebase) and drops the assembly-level
InternalsVisibleTo that was added solely for this class.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(snap-sync): collapse SnapSyncRunner ordering tests into a single TestCase
Three near-identical tests (happy / throws / cancels) folded into one
parameterized method with a DispatcherOutcome enum.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(snap-sync): switch expression for SnapSyncRunner dispatcher outcome
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>1 parent 484537f commit 60e7e76
39 files changed
Lines changed: 664 additions & 502 deletions
File tree
- src/Nethermind
- Nethermind.Merge.Plugin.Test
- Nethermind.Network.Test
- Nethermind.State.Flat/Sync/Snap
- Nethermind.Synchronization.Test
- DbTuner
- FastSync
- SnapProtocolTests
- ParallelSync
- SnapSync
- SnapSyncFeed
- Nethermind.Synchronization
- DbTuner
- FastSync
- ParallelSync
- SnapSync
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
29 | 28 | | |
30 | 29 | | |
31 | 30 | | |
| |||
1052 | 1051 | | |
1053 | 1052 | | |
1054 | 1053 | | |
1055 | | - | |
1056 | | - | |
| 1054 | + | |
1057 | 1055 | | |
1058 | 1056 | | |
1059 | 1057 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| |||
Lines changed: 9 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
| 18 | + | |
17 | 19 | | |
18 | 20 | | |
19 | 21 | | |
20 | 22 | | |
21 | | - | |
22 | 23 | | |
23 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
24 | 31 | | |
25 | 32 | | |
26 | 33 | | |
27 | | - | |
28 | | - | |
29 | 34 | | |
30 | 35 | | |
31 | 36 | | |
32 | 37 | | |
33 | 38 | | |
34 | 39 | | |
35 | 40 | | |
36 | | - | |
37 | | - | |
38 | 41 | | |
39 | 42 | | |
40 | 43 | | |
41 | 44 | | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | 45 | | |
Lines changed: 2 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | 9 | | |
11 | 10 | | |
12 | 11 | | |
| |||
17 | 16 | | |
18 | 17 | | |
19 | 18 | | |
20 | | - | |
21 | 19 | | |
22 | 20 | | |
23 | | - | |
24 | | - | |
25 | 21 | | |
26 | 22 | | |
27 | 23 | | |
| |||
34 | 30 | | |
35 | 31 | | |
36 | 32 | | |
37 | | - | |
38 | 33 | | |
39 | 34 | | |
40 | | - | |
41 | | - | |
42 | 35 | | |
43 | 36 | | |
44 | 37 | | |
45 | 38 | | |
46 | 39 | | |
47 | | - | |
48 | 40 | | |
49 | 41 | | |
50 | | - | |
51 | | - | |
52 | 42 | | |
53 | 43 | | |
54 | 44 | | |
55 | 45 | | |
56 | 46 | | |
57 | | - | |
58 | | - | |
| 47 | + | |
59 | 48 | | |
60 | 49 | | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
| 50 | + | |
71 | 51 | | |
72 | 52 | | |
73 | 53 | | |
| |||
Lines changed: 21 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
408 | 408 | | |
409 | 409 | | |
410 | 410 | | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
411 | 425 | | |
| 426 | + | |
| 427 | + | |
412 | 428 | | |
413 | 429 | | |
414 | 430 | | |
415 | 431 | | |
416 | 432 | | |
417 | 433 | | |
418 | 434 | | |
419 | | - | |
| 435 | + | |
420 | 436 | | |
421 | 437 | | |
422 | 438 | | |
| |||
425 | 441 | | |
426 | 442 | | |
427 | 443 | | |
428 | | - | |
| 444 | + | |
429 | 445 | | |
430 | 446 | | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
431 | 450 | | |
432 | 451 | | |
433 | 452 | | |
| |||
Lines changed: 12 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
7 | | - | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
15 | | - | |
16 | 16 | | |
17 | | - | |
18 | | - | |
19 | | - | |
| 17 | + | |
20 | 18 | | |
21 | | - | |
| 19 | + | |
22 | 20 | | |
23 | 21 | | |
24 | 22 | | |
25 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
| 31 | + | |
| 32 | + | |
31 | 33 | | |
| 34 | + | |
| 35 | + | |
32 | 36 | | |
33 | 37 | | |
Lines changed: 1 addition & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
| |||
18 | 17 | | |
19 | 18 | | |
20 | 19 | | |
21 | | - | |
22 | 20 | | |
23 | 21 | | |
24 | 22 | | |
| |||
52 | 50 | | |
53 | 51 | | |
54 | 52 | | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | 53 | | |
59 | | - | |
| 54 | + | |
60 | 55 | | |
61 | 56 | | |
62 | 57 | | |
| |||
0 commit comments