netudima commented on code in PR #4509:
URL: https://github.com/apache/cassandra/pull/4509#discussion_r2590900056
##########
src/java/org/apache/cassandra/cql3/UpdateParameters.java:
##########
@@ -108,20 +105,8 @@ public <V> void newRow(Clustering<V> clustering) throws
InvalidRequestException
throw new InvalidRequestException("Invalid empty or null
value for column " + metadata.clusteringColumns().get(0).name);
}
}
-
- if (clustering == Clustering.STATIC_CLUSTERING)
- {
- if (staticBuilder == null)
- staticBuilder = BTreeRow.pooledUnsortedBuilder();
- builder = staticBuilder;
- }
- else
- {
- if (regularBuilder == null)
- regularBuilder = BTreeRow.pooledUnsortedBuilder();
- builder = regularBuilder;
Review Comment:
yes, I think you are right, it looks like the original logic (with
staticBuilder/regularBuilder) tried to reuse a builder if we have more than one
row updated by the same statement, then I've added the pooling logic for the
builder but kept the original builder reusing logic untouched. So for the cases
like you mentioned when the first row was ok we have the reference to the
builder object present in the pool as well in the local
regularBuilder/staticBuilder variable.
Regarding the issue, have you seen "assert this.clustering == null" failed
when we call BTreeRow.Builder#newRow?
Note: removal of reusing via regularBuilder/staticBuilder makes the logic
slightly less efficient for multi-row updates (because instead of taking a
builder from a usual field we have to do a thread local lookup) but I suppose
it is not so frequent case to try to optimize it..
--
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]
