Fix stuck block detection #4198

pull rebroad wants to merge 1 commits into bitcoin:master from rebroad:FixStuckBlockDetection changing 1 files +5 −6

rebroad commented at 4:27 AM on May 20, 2014: contributor

Only update time of when a block is received only when a block is received (rather than when any message is received).
laanwj commented at 8:20 AM on June 23, 2014: member

I think this change makes sense, but needs rebase after 305ccaa2.
Fix stuck block detection 93f661fda5
rebroad commented at 10:48 AM on June 23, 2014: contributor

Rebased.
in src/main.cpp:None in 93f661fda5
```
3916 | @@ -3922,7 +3917,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
3917 |          }
3918 |  
3919 |          CValidationState state;
3920 | -        ProcessBlock(state, pfrom, &block);
3921 | +        if (ProcessBlock(state, pfrom, &block))
3922 | +            {
```
laanwj commented at 11:16 AM on June 23, 2014:

Indentation is strange here.

rebroad commented at 11:39 AM on June 23, 2014:

It's the same indentation that was used before, the code's just moved.

sipa commented at 11:47 AM on June 23, 2014:

You're turning a lone code block into an if, so you'll have to adapt the indentation to fit an if block...
sipa commented at 11:38 AM on June 23, 2014: member

This was actually how it originally worked, but I noticed large amounts of peers being unnecessarily disconnected during slow block processing, as their blocks were waiting.

That said, the current behaviour is probably not much better.

Perhaps we should associate timestamps with CNetMessage objects (set at the time they arrive), and use those for disconnection logic (as the network thread isn't stalled by long cs_main grabs).
BitcoinPullTester commented at 1:04 PM on June 23, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4198_93f661fda54ec95e00f4ad4692412ac32717f7c3/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj commented at 10:24 AM on June 25, 2014: member

OK in that case NACK, this is not a fix it just moves the problem to something different.
rebroad commented at 1:28 PM on June 27, 2014: contributor

Superceeded by #4431
rebroad closed this on Jun 27, 2014
rebroad deleted the branch on Jun 27, 2014
DrahtBot locked this on Sep 8, 2021

Contributors