-
Notifications
You must be signed in to change notification settings - Fork 3.8k
CASSANDRA-20858 Add virtual table to expose last N uncaught exceptions #4355
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
4ec4336 to
4b4c8cd
Compare
3614cea to
bfc7631
Compare
patch by Stefan Miklosovic; reviewed by TBD for CASSANDRA-20858
bfc7631 to
06afbf9
Compare
06afbf9 to
16b5d63
Compare
| if (vtable == null) | ||
| { | ||
| foundVtable = getVirtualTable(SlowQueriesTable.class, tableName); | ||
| foundVtable = getVirtualTable(vtableClass, tableName); |
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.
this is actually a bug in the current code
| } | ||
| else | ||
| { | ||
| mergeInto.count += 1; |
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.
if I am not mistaken then a recent repeated exception info will be evicted if a list is full, even it happened just before an exception which makes the list full..
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.
yes, but that will happen when you get 1000 different exceptions and you go to insert 1001st which is different from everything else. Exceptions are "merged".
| synchronized (buffer) | ||
| { | ||
| ExceptionRow mergeInto = null; | ||
| for (ExceptionRow row : buffer) |
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.
such iteration can be costly for big buffer and/or when an exception happen frequently, so it may cause delays/CPU consumption in a processing path...
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 do not think this is a problem. Beware that this is de-duplicated. You would need to have 1000 different exception types thrown to iterate over it in some excessive fashion. If you have 1000 exceptions thrown you have way bigger problems than iterating over this "slowly".
Anyway, I have reworked it to use bounded map approach here
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.
For example, if we get FSError, that is one exception type. So all FSError exceptions will be deduplicated into one entry.
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.
yes, agree, I would expect just few exception types here, volumes are caused by instances not by types
|
closing in favor of #4356 |
patch by Stefan Miklosovic; reviewed by TBD for CASSANDRA-20858
Thanks for sending a pull request! Here are some tips if you're new here:
Commit messages should follow the following format:
The Cassandra Jira