jmckenzie-dev commented on code in PR #3732:
URL: https://github.com/apache/cassandra/pull/3732#discussion_r1916956600
##########
src/java/org/apache/cassandra/db/ReadCommand.java:
##########
@@ -751,6 +753,20 @@ private class QueryCancellationChecker extends
StoppingTransformation<Unfiltered
{
long lastCheckedAt = 0;
+ @Override
+ protected void attachTo(BasePartitions partitions)
+ {
+ if (this.partitions == null)
Review Comment:
I'm concerned that this changes the semantics of attaching `BasePartitions`
to a `StoppingTransformation`. i.e. if someone passes in a different
`BasePartitions` instance here we'd just silently ignore it. What if we
augmented the call in `StoppingTransformation` with something like the
following:
```java
@Override
protected void attachTo(BasePartitions partitions)
{
Preconditions.checkArgument(this.partitions == null ||
this.partitions == partitions, "Attempted to attach 2nd different
BasePartitions in StoppingTransformation; this is a bug.");
this.partitions = partitions;
}
@Override
protected void attachTo(BaseRows rows)
{
Preconditions.checkArgument(this.rows == null || this.rows == rows,
"Attempted to attach 2nd different BaseRows in StoppingTransformation; this is
a bug.");
this.rows = rows;
}
```
That should allow us to gracefully ignore / make idempotent attampts to
attach a 2nd instance of the identical thing to Stop w/out changing the
semantics and potentially swallowing a bug on another iterator we thought we
would stop.
--
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]
