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

connmgr: reduce log level for closing connections #2165

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

richard-ramos
Copy link
Contributor

This PR changes the log level for closing connections in connmgr.go from INFO to DEBUG, since as reported in status-im/status-go#3249 these logs add too much noise.

@jakubgs
Copy link

jakubgs commented Mar 2, 2023

I'd argue that the closing conn one should probably be a trace level log.

@@ -173,7 +173,7 @@ func (cm *BasicConnMgr) memoryEmergency() {

// Trim connections without paying attention to the silence period.
for _, c := range cm.getConnsToCloseEmergency(target) {
log.Infow("low on memory. closing conn", "peer", c.RemotePeer())
log.Debugw("low on memory. closing conn", "peer", c.RemotePeer())
Copy link
Contributor

Choose a reason for hiding this comment

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

This definitely shouldn't be hidden as debug. Running low on memory is a pretty serious condition.

@@ -375,7 +375,7 @@ func (cm *BasicConnMgr) doTrim() {
func (cm *BasicConnMgr) trim() {
// do the actual trim.
for _, c := range cm.getConnsToClose() {
log.Infow("closing conn", "peer", c.RemotePeer())
log.Debugw("closing conn", "peer", c.RemotePeer())
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is fine.

Copy link

Choose a reason for hiding this comment

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

Are you sure it's debug? Look at this:

admin@node-01.sm-eu-zur1.spiff.app-test:~ % ls -lh /var/log/docker/spiff-workflow-wakunode/docker.log
-rw-r--r-- 1 syslog syslog 13G Mar  2 13:13 /var/log/docker/spiff-workflow-wakunode/docker.log

admin@node-01.sm-eu-zur1.spiff.app-test:~ % grep 'closing conn' /var/log/docker/spiff-workflow-wakunode/docker.log | wc -l
57729782

This is from 1.5 day of not really that active of a node. If you have a trace level it should be trace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have a trace log level. Or if we do, we don't use it.

Copy link

Choose a reason for hiding this comment

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

Fair enough.

@marten-seemann marten-seemann changed the title fix: reduce level for connection closing logs connmgr: reduce log level for closing connections Mar 2, 2023
@marten-seemann marten-seemann merged commit 98837aa into libp2p:master Mar 3, 2023
@jakubgs jakubgs deleted the chore/reduce-log-level branch April 3, 2023 13:22
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.

3 participants