-
Notifications
You must be signed in to change notification settings - Fork 212
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
Downgrade log message with high spam rate #1507
base: main
Are you sure you want to change the base?
Conversation
b78cc4f
to
b771739
Compare
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.
Please switch the exception type emitted in cfc to be detected as canceled and conditionally downgraded
b771739
to
8d8d854
Compare
@@ -291,6 +291,12 @@ private boolean errorResponse(Throwable t) { | |||
requestMetadata.getToolInvocationId(), | |||
requestMetadata.getActionId(), | |||
name)); | |||
} else if ("write cancelled".equals(t.getMessage())) { |
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.
Prefer that we get a WriteException that specializes to a WriteCancelledException, rather than relying on correlated string messages.
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.
We were also getting the same error. This pr has fixed the issue for us #1526.
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.
Can you please add a test that exceptions indicating cancellations are not logged here?
These log messages have a very high rate and are expected during cache expirations. Downgrade these to FINE to avoid spamming the logs.