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

Better logging for notifications #5500

Closed
wants to merge 4 commits into from

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Apr 2, 2020

Related to #5481

Two changes:

The first change should obviously go in.
The second one is more a proposal, and I'd be ok if people are against doing this.

@tomaka tomaka added the A0-please_review Pull request needs code review. label Apr 2, 2020
@tomaka tomaka requested review from mxinden and rphmeier April 2, 2020 09:15
Copy link
Contributor

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

The second one is more a proposal, and I'd be ok if people are against doing this.

What about testing this patch out on one of the validators to make sure this is a temporary fix?

log::warn!(
target: "sub-libp2p",
"📞 Failed to push message to queue, dropped it"
"📞 Queue of notifications with {} is full, dropped message (protocol: {:?})",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"📞 Queue of notifications with {} is full, dropped message (protocol: {:?})",
"📞 Notification queue with peer {} is full, dropped message (protocol: {:?})",

This sounds better to me. Not a native speaker though.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 2, 2020

I reverted the queue size change back to 256 and addressed review.
This should be ready for merging.

@tomaka tomaka changed the title Better logging for notifications and buffer size increase Better logging for notifications Apr 3, 2020
@tomaka
Copy link
Contributor Author

tomaka commented Apr 3, 2020

I'm closing this one and opening #5512 instead

@tomaka tomaka closed this Apr 3, 2020
@tomaka tomaka deleted the notifs-queue-change branch April 3, 2020 09:36
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.

3 participants