Quieter initial block download #1337

rebroad commented at 7:49 PM on May 17, 2012: contributor

This pull is a subset of pull #1311, which many said was too big, and did too many things. It wasn't easy to break it up into smaller pieces as so many are interdependent. Anyway, here goes. Comments welcome.

This pull adds a command line argument "-quietinitial", which is intended to make the "traffic" in debug.log less so that the block download progress can be more easily seen. It's off by default, and without using it, things look the same as usual.

I realise that some of what this does can be achieved with 3rd party log file filterers, but some of it can't, and it's mostly developers that use such 3rd party tools, and I'd like to help enable bitcoin to be useful to less technical people too.

This sub-pull, IMHO, isn't as useful unless combined with some of the other pull requests I've raised (and am still raising, due to hacking #1311 into smaller pieces).

I suspect the others will need rebasing depending on what order the pulls are effected. ho hum..!

Heh... #1337 is a cool pull number :) Not sure this one deserves such a number though!

Anyway, comments welcomed - I need to get a better idea of what makes a good pull or not.

in src/main.cpp:None in b051a5dc71 outdated

 924 | @@ -925,6 +925,13 @@ int GetNumBlocksOfPeers()
 925 |      return std::max(cPeerBlockCounts.median(), Checkpoints::GetTotalBlocksEstimate());
 926 |  }
 927 |  
 928 | +bool CaughtUp()
 929 | +{
 930 | +    int BlocksOfPeers = GetNumBlocksOfPeers();
 931 | +    //printf("CaughtUp: nBestHeight=%d, GetNumBlocksOfPeers()=%d\n", nBestHeight, BlocksOfPeers);

Diapolo commented at 8:02 PM on May 17, 2012:

I dislike commented out code and you have re-introduced "int BlocksOfPeers" here.

rebroad commented at 8:16 PM on May 17, 2012:

Thanks.. These lines now removed. I obviously didn't update something somewhere in git. :-s

Diapolo commented at 8:07 PM on May 17, 2012: none

As "Add build directory to .gitignore, so that it's not tracked." was merged to master, you should rebase this to not include that commit again.

And I'm sure (as you change more stuff and not only add "-quietinitial") you should split things up and explain more what your changes are doing.

in src/addrman.cpp:None in 1fe2e94067 outdated

 302 | @@ -303,7 +303,8 @@ void CAddrMan::Good_(const CService &addr, int64 nTime)
 303 |      // TODO: maybe re-add the node, but for now, just bail out
 304 |      if (nUBucket == -1) return;
 305 |  
 306 | -    printf("Moving %s to tried\n", addr.ToString().c_str());
 307 | +    if (CaughtUp() || !fQuietInitial)

luke-jr commented at 7:39 PM on May 18, 2012:

Minor(?) nitpick: please check fQuietinitial (simple) before CaughtUp() (a function call)

Diapolo commented at 9:17 PM on May 18, 2012:

I like this suggestion, as it's a nice coding style. Perhaps someone should look all over the code and check if we can optimize control-flow statements!?

rebroad commented at 10:35 AM on May 21, 2012:

@luke-jr good point. I shall change these. Thanks. Now changed. Also, the fLogPeers was a rebase error, which belongs in a separate commit. I've finally gotten the hang of rebasing now I think!

luke-jr commented at 2:04 AM on May 19, 2012: member

src/main.cpp: In function ‘bool ProcessMessage(CNode*, std::string, CDataStream&)’:
src/main.cpp:2592:46: error: ‘fLogPeers’ was not declared in this scope

rebroad commented at 10:08 AM on May 31, 2012: contributor

I've refactored this, making it simpler, and also separating the AskFor into two functions now, one for txs and one for blocks. This will make it easier to merge in #1404.

Add command line option -quietinitial.

This makes debugg.log quieter during block chain catch up, so that it is easier to see the block download process.

d4cdd87e2d

sipa commented at 7:44 PM on June 12, 2012: member

Why does this add a function AskForBlock? It seems to change more than just disabling some debug output.

Diapolo commented at 8:12 PM on June 12, 2012: none

I think rebroad is currently learning the tricks and handling of Github, this took me a while, too :).

rebroad commented at 11:07 PM on June 14, 2012: contributor

@sipa The addition of AskForBlock is explained above. It's due to #1404.

jgarzik commented at 10:23 PM on July 5, 2012: contributor

Sorry... in general this is just a huge amount of changes for little value, during a special condition (IBD). These are events that might be interesting (if they fail or have problems or simply do not appear, when they should), even during initial block download.

NAK from me.... though if the other devs want to override the NAK, I won't complain.

Leaving pull request open due to this "weak NAK."

jgarzik commented at 3:48 PM on August 1, 2012: contributor

Consensus does not seem to want this, closing.

jgarzik closed this on Aug 1, 2012

lateminer referenced this in commit b9fc1603ef on Jan 22, 2019

lateminer referenced this in commit c5fc9cc7a3 on May 6, 2020

DrahtBot locked this on Sep 8, 2021