BN, add timeout for sync workers which are waiting in queue.

[cheatfate]

Add timeout of 1.seconds for sync workers which are waiting in Q state. It means that first worker waiting in queue will have timeout of 12.seconds, second worker will have 24.seconds and so on.

[github-actions]

Unit Test Results

         9 files  ±0    1 115 suites  ±0   29m 51s ⏱️ -21s
  4 246 tests +2    3 899 ✔️ +2  347 💤 ±0  0 ❌ ±0 
16 932 runs  +6  16 534 ✔️ +6  398 💤 ±0  0 ❌ ±0 

Results for commit d02376a. ± Comparison against base commit 9ad8ea0.

♻️ This comment has been updated with latest results.

[cheatfate]

This should fix #5794

[etan-status]

The fix indeed should fix the low-peer scenario, and is relatively tiny.

It's probably okay to push this after the Deneb Mainnet release, just in case a regression gets introduced, as there is no downgrade possibility from the Deneb release back to a Capella release (they cannot connect to Deneb networks).

[etan-status]

1s intuitively feels a bit short, I could see false positives if a block is stuck in P for a while (for example, the occasional multi-second state replay). The underlying problem only occurs in low-peer scenario with bad peers failing to provide data, I think it's okay if it takes a bit longer to recover in that edge case, if it means that the happy case is a bit more reliable.

[etan-status]

and, also, if r11 fails (incomplete download, failed validation and so on), and p1 goes away due to the corresponding descore for failing to provide correct data, a different peer will eventually pick up r11.

otherwise, r12/r13/r14 would just get stuck again and again (as before).

[cheatfate]

Yep in old version r12, r13 and r14 will stuck waiting for peer to appear which should provide r11 again.

[etan-status]

this is the value if every single request takes the maximum time to complete a single time.

• reality could be longer, e.g., when there are retries of earlier sync requests
• reality could be faster, e.g., if the time is computed while there are many prior requests, but then those prior requests complete quickly

I wonder if a simpler mechanism with a static, e.g., 30sec timeout, could also mitigate the risk of getting stuck. It would take a bit longer to unstuck than the current solution, but is simpler to reason about.

Alternatively, to get it fully correct may involve having to re-schedule the timeouts whenever a prior request completes.

[etan-status]

On Goerli, a similar situation actually comes up even though the number of available peers is very high.

Sync manager only considers peers viable that report a higher slot progress than the local head. However, because goerli is partitioned into split views and proposals are infrequent, there are long stretches where the local head may be higher than the peer's branches. This leads to a situation where < 10 peers are actually viable for sync manager at a time, and I have observed the situation where the Q status workers temporarily could not proceed for minutes because all the other workers were stuck in U/R stage.

Regarding timings, the D/P stages can take quite long, and are the only way to eventually unstuck Q workers. If we just want to specify a single timeout number, I think Q should wait for at least 30-90 seconds based on manual observations of sync progress. Alternatively, a shorter timeout may be suitable if it only is applied while no other worker is in D/P stage, e.g., 5-15 seconds. While other workers are in D/P, I don't think a timeout is needed, as the situation will resolve itself eventually, but if a single timeout value makes the implementation easier, 30-90 seconds should not do too much harm.