-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Improve logging for locked file acquisition #6777
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
Conversation
err | ||
); | ||
} else { | ||
debug!("Released lock at `{}`", self.0.path().display()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be trace!
like the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe since it comes after all the other output.
) | ||
})?; | ||
|
||
debug!("Acquired lock for `{resource}`"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be trace!
like the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so — the acquired lock message above is debug level. The trace logs are for the acquire lock error message and the check message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still feels like a lot for --verbose
IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine with me I think, but we'll want to change both.
Consistency etc. when debugging a deadlock