UpdateTip: log only one line at most per block #7795

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_04_block_warning_logging_inline changing 1 files +14 −9

laanwj commented at 11:16 AM on April 3, 2016: member

Avoid logging two or more lines per block in UpdateTip by adding the (optional) warning into the UpdateTip log message.

Before:

2016-04-03 11:09:21 UpdateTip: new best=000000000000000004de55677b9b01d5bb105a6c93e76efca2fa40d6281614db height=405532 version=0x00000004 log2_work=84.416823 tx=119994369 date='2016-04-03 11:09:01' progress=1.000000 cache=1759.4MiB(1793826tx)
2016-04-03 11:09:21 UpdateTip: 7 of last 100 blocks have unexpected version

After:

2016-04-03 11:09:23 UpdateTip: new best=000000000000000004de55677b9b01d5bb105a6c93e76efca2fa40d6281614db height=405532 version=0x00000004 log2_work=84.416823 tx=119994369 date='2016-04-03 11:09:01' progress=1.000000 cache=25.5MiB(7394tx) warning='7 of last 100 blocks have unexpected version'

laanwj added the label Docs and Output on Apr 3, 2016
paveljanik commented at 6:54 AM on April 5, 2016: contributor

Does your fix mean that the debug log can miss some of the previously logged lines now, because only one warning is printed?

in src/main.cpp:None in 5553b1dc36 outdated

2561 | -      DateTimeStrFormat("%Y-%m-%d %H:%M:%S", chainActive.Tip()->GetBlockTime()),
2562 | -      Checkpoints::GuessVerificationProgress(chainParams.Checkpoints(), chainActive.Tip()), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize());
2563 | -
2564 |      cvBlockChange.notify_all();
2565 |  
2566 |      // Check the version of the last 100 blocks to see if we need to upgrade:

paveljanik commented at 7:10 AM on April 5, 2016:

When you are touching this part, can you please also move this comment about 100 blocks just over the for cycle checking these 100 blocks?

Edit: the questions is, how often such case can happen, if it all...

paveljanik commented at 7:11 AM on April 5, 2016: contributor

Concept ACK, less lines, more informations on line line, great!
jonasschnelli commented at 7:16 AM on April 5, 2016: contributor

utACK 5553b1dc3654b5aec6a6161d867f93b1509d3379 modulo @paveljanik's nit.
laanwj commented at 2:25 PM on April 5, 2016: member
Does your fix mean that the debug log can miss some of the previously logged lines now, because only one warning is printed?

Well there are two sources of warning:
- BIP9 version bits ("unknown new rules are about to activate (versionbit %i)")
- Old softfork mechanism ("%d of last 100 blocks have unexpected version")
I'm not sure it makes sense to ever have both, but I could accumulate the warnings instead of replacing them, sure.

Edit: OH wait, the first warning can trigger multiple times, potential for every bit. Yes I certainly need to do this.

UpdateTip: log only one line at most per block

Avoid logging two or more lines per block in UpdateTip by
adding the warning into the UpdateTip log message.

f20d42ed2b

laanwj force-pushed on Apr 5, 2016
laanwj commented at 2:40 PM on April 5, 2016: member

Ok, fixed both of @paveljanik 's nits. Warnings are now collected and moved the comment.
paveljanik commented at 5:04 PM on April 5, 2016: contributor

Thanks. (I haven't had the time to investigate myself but had the feeling, that it could be as you said).

ACK https://github.com/bitcoin/bitcoin/pull/7795/commits/f20d42ed2b1b8de9c41525f96c40b172ab82ec99
gmaxwell commented at 7:50 AM on May 17, 2016: contributor

utACK.

Are LogPrintF's that are composed across multiple lines instead of in a single logprint more likely to get torn by logprints from other threads? Should the field potentially be named "warnings" since it can contain more than one?
paveljanik commented at 9:15 AM on May 24, 2016: contributor

Can we move this forward?
sipa commented at 4:17 PM on May 25, 2016: member

utACK f20d42ed2b1b8de9c41525f96c40b172ab82ec99
sipa merged this on May 25, 2016
sipa closed this on May 25, 2016
sipa referenced this in commit 47a7cfb0aa on May 25, 2016
codablock referenced this in commit d1148b7c16 on Sep 16, 2017
codablock referenced this in commit 618777127b on Sep 19, 2017
codablock referenced this in commit 640bee1814 on Dec 21, 2017
MarkLTZ referenced this in commit 137ee6db0d on Apr 27, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

Labels