Conversation
…ailure Patch by Yifan Cai; Reviewed by TBD for CASSANALYTICS-196
| return false; | ||
| } | ||
|
|
||
| if (updateCount > 1) |
There was a problem hiding this comment.
Can't we check this before we need to extract the tableId and the metadata? (just at line 638)
| { | ||
| JVMStabilityInspector.inspectThrowable(t); | ||
|
|
||
| if (failedMutationHasNoCdcTable(inputBuffer, size)) |
There was a problem hiding this comment.
It is scary that this new behavior doesn't break a pre-existing test :-(
| CassandraBridgeImplementation.setup(); | ||
| TableId unknownTableId = TableId.fromUUID(UUID.randomUUID()); |
There was a problem hiding this comment.
nit: Maybe move these two to a helper getUnregisteredTable method?
| * when the table's metadata can't be found, or even the first {@link TableId} can't be read | ||
| */ | ||
| @VisibleForTesting | ||
| static boolean failedMutationHasNoCdcTable(byte[] inputBuffer, int size) |
There was a problem hiding this comment.
Maybe it is just me, but I tend to not like the negative on method names. HasNo + false is what we are looking for here -> that double negative always messes my head :-P
What do you think on having a failedMutationHasCdcTable method instead that returns true if it needs to be handled, and false if it needs to be ignored?
Obviously not a blocker.
jyothsnakonisa
left a comment
jyothsnakonisa
left a comment
There was a problem hiding this comment.
Looks good to me!
3 participants
Patch by Yifan Cai; Reviewed by TBD for CASSANALYTICS-196