maedhroz commented on code in PR #3575:
URL: https://github.com/apache/cassandra/pull/3575#discussion_r1800139541
##########
src/java/org/apache/cassandra/service/reads/DataResolver.java:
##########
@@ -278,6 +277,10 @@ private PartitionIterator
resolveWithReplicaFilteringProtection(E replicas, Repa
private UnaryOperator<PartitionIterator>
preCountFilterForReplicaFilteringProtection()
{
+ // Key columns are immutable and should never need to participate in
replica filtering
+ if (!command.rowFilter().hasNonKeyExpressions())
+ return results -> results;
Review Comment:
Early port of CASSANDRA-19938...washes out next trunk rebase...
##########
src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java:
##########
@@ -1266,6 +1272,32 @@ public boolean isRangeRequest()
return false;
}
+ /*
+ * The execution method does not need to perform reconciliation so the
read command
+ * should execute in a mannager suited to not needing reconciliation. Such
as when
+ * executing transactionally at a single replica and doing an index scan
where the index
+ * scan should not return extra rows and expect post filtering at the
coordinator.
+ */
+ public SinglePartitionReadCommand withoutReconciliation()
+ {
+ if (indexQueryPlan() == null)
+ return this;
Review Comment:
When
[CASSANDRA-19007](https://issues.apache.org/jira/browse/CASSANDRA-19007) gets
done, I think we could have a filtering queries that will be controlled by the
row filter's reconciliation flag as well. We may want to simply check
`rowFilter().isEmpty()` here. Normal partition and clustering restrictions
won't be in the row filter, IIRC
##########
test/distributed/org/apache/cassandra/fuzz/sai/AccordMultiNodeSAITest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.fuzz.sai;
+
+import org.junit.BeforeClass;
+
+public class AccordMultiNodeSAITest extends MultiNodeSAITestBase
Review Comment:
Same thing if you run `FullAccordCQLTest`
##########
src/java/org/apache/cassandra/index/sai/plan/QueryController.java:
##########
@@ -168,6 +167,22 @@ public UnfilteredRowIterator queryStorage(PrimaryKey key,
ReadExecutionControlle
return partition.queryMemtableAndDisk(cfs, executionController);
}
+ private static Runnable getIndexReleaser(Set<SSTableIndex>
referencedIndexes)
+ {
+ return new Runnable()
+ {
+ boolean closed;
+ @Override
+ public void run()
+ {
+ if (closed)
+ return;
+ closed = true;
+ referencedIndexes.forEach(SSTableIndex::releaseQuietly);
+ }
+ };
+ }
Review Comment:
I seem to remember us trying to explicitly make sure we only released the
SSTable indexes once in CASSANDRA-19428. Did we miss a case, or is there
something specific to this patch that made this check necessary?
##########
src/java/org/apache/cassandra/service/accord/txn/TxnNamedRead.java:
##########
@@ -129,6 +129,8 @@ public AsyncChain<Data> read(Timestamp executeAt)
// this simply looks like the transaction witnessed TTL'd data and the
data then expired
// immediately after the transaction executed, and this simplifies
things a great deal
int nowInSeconds = (int)
TimeUnit.MICROSECONDS.toSeconds(executeAt.hlc());
+ if (consistencyLevel == null || consistencyLevel ==
ConsistencyLevel.ONE)
Review Comment:
`NODE_LOCAL` isn't possible here?
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
