alanwang67 commented on code in PR #4684:
URL: https://github.com/apache/cassandra/pull/4684#discussion_r2977097965
##########
src/java/org/apache/cassandra/cql3/statements/BatchStatement.java:
##########
@@ -541,7 +543,11 @@ private void executeWithoutConditions(List<? extends
IMutation> mutations, Consi
updatePartitionsPerBatchMetrics(mutations.size());
- boolean mutateAtomic = (isLogged() && mutations.size() > 1);
+ // We special case single token batch statements that span both Accord
and non-Accord
+ // tables to go through the batch log in order to preserve all or
nothing application
+ // see CASSANDRA-20588 for more details
+ boolean isSingleTokenBatchStatementSpanningAccordAndNonAccordTables =
(mutations.size() == 1) && (containsAccordMutation(ClusterMetadata.current(),
mutations.get(0)));
Review Comment:
I'm assuming you mean `StorageProxy.mutateWithTriggers`, as
`StorageProxy.mutateWithConditions` does not exist. So currently, there are two
methods that we branch to depending on whether or not we go through the batch
log path: `mutateAtomically` and `dispatchMutationsWithRetryOnDifferentSystem`.
These two methods use different `SplitConsumer`'s on the list of Accord
Mutations and the list of Normal Mutations after constructing them. This means
that in order to reuse the values that we created, we will need to perform the
creation of the `SplitConsumer` after we have determined which path we will go
down. Doing so is a bit awkward since the two `SplitConsumer`'s don't return
the same type. Therefore, we would need to perform this as a side effect.
However, this is a bit messy since `SplitConsumer` captures some state that is
only later created by `mutateAtomically` and also this means we would have to
allocate this state regardless of which branch we went down which seems a bit wa
steful.
Some other alternatives I've considered are just applying the
`SplitConsumer` lazily after we have passed in the two values. However, this
means that we will have to reiterate over the list of Mutations which doesn't
seem too great to do since we will have to pay this price for every Batch and
Modification Statement.
However from your above comments, I changed the check to do a single pass
and not allocate the filtered mutations and also inlined it so we only pay this
overhead in the batch statement for the single mutation case.
Also glad to take any suggestions if there's a better way to do things or if
the tradeoff is still worthwhile to do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
