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

kvserver: decrease verbosity of replicate queue trace logging #89042

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

AlexTalks
Copy link
Contributor

While logging of traces on replicate queue errors was recently added, logging these traces at the WARNING level appears to be too high, causing noisy logs. This change decreases the verbosity of these logs to the INFO level.

Release note: None

@AlexTalks AlexTalks requested a review from a team as a code owner September 29, 2022 23:08
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

This API feels unintuitive to me. So if what we're emitting is a warning, it'll show up in the main dev log, but if it's a regular info, it shows up elsewhere? What about warnings/errors (like this trace) that we do want to keep around, just not in the same log file?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@AlexTalks
Copy link
Contributor Author

I believe it's rather that if it's a warning, it's emitted in its own log as well as in the main log, while if it's below warning, it will only be in its own log. I think that's how I'm supposed to be reading this thing:

default:
channels:
INFO: [DEV, OPS]
WARNING: all except [DEV, OPS]

It is also explicitly set up this way (for specific channels) in the test logging config.

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

Ack. Is there a way to wholly segment even warnings to just the distribution channel? As an observer, that feels like what I’d want. Perhaps this is a question for @lidorcarmel or @knz. This PR LGTM, thanks!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@irfansharif
Copy link
Contributor

Perhaps worth eyeballing whether there are other possibly spammy warnings/errors that'd would benefit from this treatment. And filing an issue to revert them back to warnings/errors (easier to grep for in the dedicated log files), but properly segmented.

While logging of traces on replicate queue errors was recently added,
logging these traces at the `WARNING` level appears to be too high,
causing noisy logs. This change decreases the verbosity of these logs to
the `INFO` level.

Release note: None
@AlexTalks
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 30, 2022

Build succeeded:

@craig craig bot merged commit 8e6b81b into cockroachdb:master Sep 30, 2022
@AlexTalks AlexTalks deleted the rq_trace_at_info branch October 3, 2022 17:03
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