Skip to content
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

Fix spammy reconnection logs #459

Merged

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Feb 5, 2025

We periodically try to reconnect to known persisted peers, but so far every 10 seconds, which resulted in very spammy reconnection logs. Here, we bump the reconnection interval to once a minute, which is still pretty low.

We also drop redundant reconnection logging as we already log connection attempts in do_connect_peer, so no need to
additionally log them just to note it's a reconnection.

tnull added 2 commits February 5, 2025 12:39
We periodically try to reconnect to known persisted peers, but so far
every 10 seconds, which resulted in very spammy reconnection logs.
Here, we bump the reconnection interval to once a minute, which is still
pretty low.
We already log connection attempts in `do_connect_peer`, so no need to
additionally log them just to note it's a reconnection.
@tnull tnull requested a review from arik-so February 5, 2025 11:54
@tnull tnull added this to the 0.5 milestone Feb 5, 2025
It can be very useful to see which tasks end up delaying shutdown.
Therefore, we here bump their logging from `TRACE` to `DEBUG`.
Copy link

Choose a reason for hiding this comment

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

does this have the potential to make it more difficult for people to understand why a reconnection is occurring?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it shouldn't change the status quo in any direction I think, it just gets rid of redundantly logged lines that didn't hold any additional information really.

@arik-so
Copy link

arik-so commented Feb 6, 2025

CI seems to have been cancelled, but not due to test failure?

@tnull tnull merged commit 5f0ab5d into lightningdevkit:main Feb 6, 2025
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants