New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from May 25, 2016

Conversation

Projects
None yet
5 participants
@laanwj
Member

laanwj commented Apr 3, 2016

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'
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 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?

Contributor

paveljanik commented Apr 5, 2016

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

@paveljanik

View changes

Show outdated Hide outdated src/main.cpp
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 5, 2016

Contributor

Concept ACK, less lines, more informations on line line, great!

Contributor

paveljanik commented Apr 5, 2016

Concept ACK, less lines, more informations on line line, great!

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Apr 5, 2016

Member

utACK 5553b1dc3654b5aec6a6161d867f93b1509d3379 modulo @paveljanik's nit.

Member

jonasschnelli commented Apr 5, 2016

utACK 5553b1dc3654b5aec6a6161d867f93b1509d3379 modulo @paveljanik's nit.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 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.

Member

laanwj commented Apr 5, 2016

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.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 5, 2016

Member

Ok, fixed both of @paveljanik 's nits. Warnings are now collected and moved the comment.

Member

laanwj commented Apr 5, 2016

Ok, fixed both of @paveljanik 's nits. Warnings are now collected and moved the comment.

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 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 f20d42e

Contributor

paveljanik commented Apr 5, 2016

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

ACK f20d42e

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell May 17, 2016

Member

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?

Member

gmaxwell commented May 17, 2016

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

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik May 24, 2016

Contributor

Can we move this forward?

Contributor

paveljanik commented May 24, 2016

Can we move this forward?

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa May 25, 2016

Member

utACK f20d42e

Member

sipa commented May 25, 2016

utACK f20d42e

@sipa sipa merged commit f20d42e into bitcoin:master May 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

sipa added a commit that referenced this pull request May 25, 2016

Merge #7795: UpdateTip: log only one line at most per block
f20d42e UpdateTip: log only one line at most per block (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7795: UpdateTip: log only one line at most per block
f20d42e UpdateTip: log only one line at most per block (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7795: UpdateTip: log only one line at most per block
f20d42e UpdateTip: log only one line at most per block (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 21, 2017

Merge #7795: UpdateTip: log only one line at most per block
f20d42e UpdateTip: log only one line at most per block (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment