Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@arkpar
Copy link
Member

@arkpar arkpar commented Aug 7, 2019

Fixes #3269

  • Cache idle state in sync so that requests are not collected with every network poll.
  • Block import are printed in the console by default
  • Removed some additional console spam.

@arkpar arkpar added the A0-please_review Pull request needs code review. label Aug 7, 2019
@arkpar arkpar changed the title A logging Improve console output Aug 7, 2019
@tomaka tomaka requested a review from twittner August 7, 2019 19:36
@gavofyork gavofyork merged commit d5560b6 into master Aug 8, 2019
@gavofyork gavofyork deleted the a-logging branch August 8, 2019 07:34
Copy link
Contributor

@twittner twittner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is_idle is touched in too many places and is too fragile to use. If only one self.is_idle = false is missing, block_requests will not function properly. Rather than introducing this flag I would consider removing the line trace!(target: "sync", "No new block request for {}", id);.

Generally speaking I think we should make more use of the available log levels. I think it is okay for trace to produce a lot of output as it is meant to literally trace through the whole processing flow as it happens. We seem to be using info only rarely if at all so everything informational ends up in debug or trace. Maybe lifting most of debug to info and some of trace to debug would leave room for the truly noisy messages to be left on trace level.

@arkpar
Copy link
Member Author

arkpar commented Aug 8, 2019

info is printed to stderr by default. We are already using it for important messages that user should see. I agree that is_idle is somewhat fragile. Having the request diagnostic is fairly important though as it already helped finding a few issues in the past.

kianenigma pushed a commit that referenced this pull request Aug 9, 2019
* Cache idle state

* Display import error by default
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logging sync=trace is way too verbose

6 participants