-
Notifications
You must be signed in to change notification settings - Fork 801
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
Capture CassandraLWT error and log/bump metrics for it. #4888
Conversation
@@ -76,6 +76,7 @@ type ( | |||
IsTimeoutError(error) bool | |||
IsNotFoundError(error) bool | |||
IsThrottlingError(error) bool | |||
IsCassandraLWTError(error) bool |
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.
Cassandra-specific func in the nosql interface doesn't look good. Can you give this a more generic name so that it looks less ugly for non-Cassandra nosql plugins? Perhaps something like IsDBUnavailableError
.
@@ -266,6 +266,12 @@ type ( | |||
Msg string | |||
} | |||
|
|||
// CassandraLWTError is returned when we get UNAVAILABLE error code from Cassandra with message containing | |||
// "Cannot perform LWT operation" | |||
CassandraLWTError struct { |
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 should keep this file DB-agnostic, so I'd prefer to use more generic names like DBUnavailableError which could be meaningful for plugins such as MongoDB.
Also see my similar comment in common/persistence/nosql/nosqlplugin/interfaces.go
.
I've renamed the places you mentioned. Couple of questions:
|
@ZackLK RE the metrics, I would, it'd be super confusing to have a metric called cassandra-error-XYZ and have it being emitted from some other random database. Particularly as we may query across clusters |
Capture DB Unavailable errors and log/bump metrics for it.
if errChecker.IsDBUnavailableError(err) { | ||
return &p.DBUnavailableError{ | ||
Msg: fmt.Sprintf("#{operation} operation failed. Error: #{err}"), | ||
} | ||
} | ||
|
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.
worth fixing
What changed?
Capturing a specific error from Cassandra by error code and substring, then publishing metrics & logging when we find it.
Why?
How did you test it?
Hasn't been tested yet - looking for feedback on how to unit test this that would provide value, or we can test in staging.
Potential risks
Release notes
Documentation Changes