Skip to content

downgrade Error log to Warning #46

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

Merged
merged 1 commit into from
May 6, 2019
Merged

downgrade Error log to Warning #46

merged 1 commit into from
May 6, 2019

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented May 5, 2019

It log spams in relays

@vyzo vyzo requested a review from Stebalien May 5, 2019 07:22
@ghost ghost assigned vyzo May 5, 2019
@ghost ghost added the status/in-progress In progress label May 5, 2019
@whyrusleeping
Copy link
Contributor

Wait... Why are we seeing this at all? This would imply a broken implementation

@vyzo
Copy link
Contributor Author

vyzo commented May 5, 2019

Ok, let's punt on downgrading and hunt for a bug instead.

@vyzo
Copy link
Contributor Author

vyzo commented May 6, 2019

Ok, something must be done about this log, regardless of bug. It filled the disk in the mplex relay.

It log spams in relays
@vyzo
Copy link
Contributor Author

vyzo commented May 6, 2019

I made it a Warning.

@vyzo vyzo changed the title downgrade Error log to Debug downgrade Error log to Warning May 6, 2019
@Stebalien
Copy link
Member

Stebalien commented May 6, 2019

Could be ipfs/kubo#6197 causing a concurrent write? Can't be, we lock on write.

@Stebalien Stebalien merged commit 94af77d into master May 6, 2019
@ghost ghost removed the status/in-progress In progress label May 6, 2019
@Stebalien Stebalien deleted the fix/log-spam branch May 6, 2019 18:19
@Stebalien
Copy link
Member

Could also be a bug in js?

@Stebalien
Copy link
Member

Ah, no. It's a cleanup race. We set closedRemote when closing the connection or resetting the stream.

@vyzo
Copy link
Contributor Author

vyzo commented May 6, 2019

Probably because of stream resets.

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