feat(eth/downloader): warn when a sync round stalls - #2494
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2dc4743 to
0b36e99
Compare
aef68c1 to
1b80127
Compare
1b80127 to
7dd4080
Compare
There was a problem hiding this comment.
Pull request overview
Adds diagnostics for downloader sync rounds that remain active without progress.
Changes:
- Tracks sync-round timing and progress, emitting rate-limited stall warnings.
- Adds tests for warning behavior and sync lifecycle cleanup.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
eth/downloader/downloader.go |
Implements stall detection and warning logic. |
eth/downloader/downloader_test.go |
Tests stall warnings and round cleanup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A sync round that never releases the synchronising flag makes every subsequent attempt fail the compare-and-swap and return errBusy, which Synchronise discards without logging. A node in that state stops syncing indefinitely and emits no diagnostics at all. Track when the in-flight round started and log a rate limited warning once it exceeds the threshold.
7dd4080 to
da790a5
Compare
| // Such a round silently rejects every subsequent attempt with errBusy, so | ||
| // without this the node can stop syncing indefinitely without emitting a | ||
| // single log line. The first probe past the threshold only captures a progress | ||
| // baseline, and an actual warning is emitted once a later probe shows the | ||
| // round has not advanced any further. |
| if d.stallProbed { | ||
| // Baseline already captured, only report when the round stopped | ||
| // advancing since then. Otherwise shift the baseline forward and | ||
| // stay quiet. | ||
| if progress > d.stallBaseline { |
|
This feature is not important, and the code is not very strong now. It is not worth spending too much time, so I choose to give up. |
Proposed changes
A sync round that never releases the synchronising flag makes every subsequent attempt fail the compare-and-swap and return errBusy, which Synchronise discards without logging. A node in that state stops syncing indefinitely and emits no diagnostics at all.
Track when the in-flight round started and log a rate limited warning once it exceeds the threshold.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that