Repository navigation
[Bug] Periodic block synchronization stalls accompanied by simultaneous peer disconnections #7027
Description
Software Versions
- OS: Linux
- JVM: Oracle Corporation 1.8.0_161 amd64
- Git Commit: 5c449a1
- Version: GreatVoyage-v4.8.2.3
Expected Behavior
The node should continuously synchronize blocks without periodically stopping progress and disconnecting multiple healthy peers.
Actual Behavior
During block synchronization, the node periodically stops progressing for approximately 32–33 seconds, followed by simultaneous disconnections of multiple peers. Synchronization resumes after reconnection.
Three stalls were observed at 02:26:54, 02:39:36, and 02:49:32. During each occurrence, eight peers were disconnected within the same second.
These stalls are separate from consistently slow block processing and account for approximately 5% of additional synchronization time.
Frequency
Approximately once every 10–13 minutes during the observed synchronization period, with each stall lasting about half a minute.
Activity
halibobo1205 commentedon Oct 13, 2026
Thanks for the report. The three stalls in Actual Behavior (02:26:54, 02:39:36, 02:49:32) have no logs attached yet. Could you add the following and note which date the log is from?
grep -nE 'maxPendingBlockSize|syncFetchBatchNum|minConnections|maxConnections' config.confxxo1shine commentedon Oct 13, 2026
@halibobo1205, the logs from that run are no longer available, so I cannot provide the records for those three stalls. The code investigation and a regression test now verify the missing fetch retrigger after window capacity is released, but they do not replace the original logs from that run.
lxcmyf commentedon Oct 14, 2026
The missing retrigger is a plausible explanation, but the current flag protocol has a second race that a simple
fetchFlag = trueinhandleSyncBlock()would not fully fix. The fetch task readsfetchFlagand clears it in two separate volatile operations. If the block handler frees capacity and sets the flag between those operations, the fetch task can overwrite that notification withfalse, leaving the queue idle until another unrelated event occurs.Please consider an atomic consume such as
AtomicBoolean#getAndSet(false)or a single-threaded wake-up mechanism, and add a deterministic concurrency test for this interleaving. It would also be useful to verify that every capacity-release path—including successful processing and removal of obsolete or invalid blocks—causes fetch capacity to be reevaluated.317787106 commentedon Oct 14, 2026
This report is incomplete in its current form. Although it includes version information and stall timestamps, it lacks supporting logs, relevant configuration, and reproducible steps to verify the reported behavior.
The code investigation and regression test in #7029 may demonstrate a fetch-scheduling defect, but they do not establish that it caused the three reported stalls or the estimated 5% synchronization overhead.
Since the original logs are unavailable, please capture a new occurrence with logs covering the stall and peer disconnections, including the date, timezone, block heights, and disconnect reasons. Please also provide the relevant synchronization settings and reproduction steps.
Until that evidence is available, the cause of the reported incidents remains unconfirmed.
xxo1shine commentedon Oct 14, 2026
@lxcmyf, This needs to be considered together with the execution order after the flag is cleared. Once the fetch task reads fetchFlag as true, it clears the flag and immediately calls startFetchSyncBlock(), which recalculates the current window capacity.
In the interleaving you described, if the block handler releases capacity and sets the flag between that read and clear, the flag is subsequently cleared, but the fetch pass immediately following it still reevaluates the released capacity. If capacity is released after the flag is cleared, the newly set true remains available to trigger the next pass. Therefore, this interleaving alone does not establish that the queue would remain idle until another unrelated event occurs. fetchFlag indicates that a reevaluation is needed; it does not require a separate execution for every notification.
The current change also records capacity release when buffered blocks belonging to disconnected peers are removed, when already-solidified blocks are removed, and when blocks are taken out for processing, then triggers a fetch reevaluation at the end of the handling pass. I would prefer to retain the current implementation without introducing AtomicBoolean or changing the scheduling mechanism at this point. If there is another concrete interleaving in which capacity has been released, the current pass fails to act on it, and the subsequent trigger is also lost, we can examine that sequence further.
xxo1shine commentedon Oct 14, 2026
@317787106, The original logs from that run are no longer available, as I explained in my earlier reply. The three stalls and approximately 5% additional synchronization time in the report were observations from that run, not conclusions measured by the regression test.
The missing fetch retrigger identified during the investigation is supported by code analysis and a regression test, and #7029 addresses that specific defect. The absence of the original logs limits our ability to confirm each historical incident individually, but it should not require waiting for another occurrence before proceeding with a fix that already has supporting evidence. If similar stalls still occur after the fix, we can investigate further using new logs and configuration details.
waynercheung commentedon Oct 14, 2026
Two points that may help settle
FixesvsRelated to:PushBlockgaps followed by same-second disconnects show up only without the patch, the incidents are attributed andFixesholds. There is no need to wait for another occurrence.TIME_OUTdisconnect.not sync for a long time(30s without progress, checked every 2s) fits a scheduling stall.get sync message timeout(a request older than 5s) orget avd message timeout(20s) would point to a different cause.lxcmyf commentedon Oct 15, 2026
You are right about this interleaving. If capacity is released before the flag is cleared, the immediately following
startFetchSyncBlock()reevaluates the updated capacity; if it is released afterward, the newtrueremains for the next pass. With the addedcapacityReleasedhandling covering the removal paths, theAtomicBooleanchange is not necessary for this case, so I withdraw that concern.One minor test improvement would be to emulate the actual scheduler order by clearing the flag before invoking
startFetchSyncBlock(), but I do not consider that a blocker.xxo1shine commentedon Oct 15, 2026
@waynercheung, Distinguishing the specific timeout log preceding a disconnect is useful for diagnosis. However, using the same snapshot, block range, and configuration does not fix peer response timing or thread scheduling. A single A/B sync run that does not reproduce the stall therefore cannot rule out this issue; even if it occurs only on the unpatched side, the logs still need to establish the cause.
The regression test in #7029 already verifies the specific triggering conditions: fetching stops when the window is full, and after processing buffered blocks releases capacity, requests resume without another inbound message. This directly verifies the scheduling defect addressed by the fix.
The limitation caused by the missing original logs has already been explained, and I am not presenting the regression test as confirmation of each of the three historical stalls. I would prefer to retain the current fix and the Fixes association, without making an additional full-range A/B synchronization experiment a prerequisite for this fix.
xxo1shine commentedon Oct 15, 2026
@lxcmyf, We now agree on this execution order. I will retain the existing flag mechanism and the fetch retrigger after capacity is released, without introducing AtomicBoolean. Clearing the flag before fetching in the test would make it more closely reflect the actual scheduler order; for now, I would prefer to keep the existing tests without further adjustments in this PR.
waynercheung commentedon Oct 15, 2026
@xxo1shine Agreed. A single A/B run wouldn't be conclusive, and I didn't intend it as a prerequisite for the fix. Now that #7029 is merged, checking the specific timeout message preceding each disconnect would be a useful first step if a stall recurs on a build containing the fix.