Quieter initial block download #1337
Conversation
| bool CaughtUp() | ||
| { | ||
| int BlocksOfPeers = GetNumBlocksOfPeers(); | ||
| //printf("CaughtUp: nBestHeight=%d, GetNumBlocksOfPeers()=%d\n", nBestHeight, BlocksOfPeers); |
There was a problem hiding this comment.
I dislike commented out code and you have re-introduced "int BlocksOfPeers" here.
There was a problem hiding this comment.
Thanks.. These lines now removed. I obviously didn't update something somewhere in git. :-s
|
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. |
| @@ -303,7 +303,8 @@ void CAddrMan::Good_(const CService &addr, int64 nTime) | |||
| // TODO: maybe re-add the node, but for now, just bail out | |||
| if (nUBucket == -1) return; | |||
|
|
|||
| printf("Moving %s to tried\n", addr.ToString().c_str()); | |||
| if (CaughtUp() || !fQuietInitial) | |||
There was a problem hiding this comment.
Minor(?) nitpick: please check fQuietinitial (simple) before CaughtUp() (a function call)
There was a problem hiding this comment.
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!?
There was a problem hiding this comment.
@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!
|
|
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. |
This makes debugg.log quieter during block chain catch up, so that it is easier to see the block download process.
|
Why does this add a function AskForBlock? It seems to change more than just disabling some debug output. |
|
I think rebroad is currently learning the tricks and handling of Github, this took me a while, too :). |
|
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." |
|
Consensus does not seem to want this, closing. |
changes to aid in testing and debugging
7e493df Using WAIT_LOCK macro instead of WaitableLock. (furszy) a0d0e33 [Refactor] Complete move from CCriticalSection to identical RecursiveMutex (both are AnnotatedMixin<std::recursive_mutex>). (furszy) 6608757 doc: Add comment to cs_main and mempool::cs (furszy) Pull request description: This was done on top of bitcoin#1335 and bitcoin#1336 (Starting in bb575f8). Effort to bring us up up-to-date with upstream's sync.h/cpp sources. This PR contains: * A complete move from CCriticalSection to identical RecursiveMutex (both are AnnotatedMixin<std::recursive_mutex>). * Using WAIT_LOCK macro instead of WaitableLock. ACKs for top commit: random-zebra: utACK 7e493df Fuzzbawls: utACK 7e493df Tree-SHA512: 539fe93566f90246409606acb0aaeb3a5f839110cb96af7868654738685a07b9e1332f8362a04d328825291007d24c80d0b34b1318edc4afe84a8ac8e5affe61
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.