[GitHub] [cassandra] beobal commented on a change in pull request #1160: CASSANDRA-16721 Move RepairedDataInfo to the execution controller rather than the ReadCommand to avoid unintended sharing

[GitBox <gi...@apache.org>]

beobal commented on a change in pull request #1160:
URL: https://github.com/apache/cassandra/pull/1160#discussion_r695998593



##########
File path: src/java/org/apache/cassandra/service/reads/AbstractReadExecutor.java
##########
@@ -155,7 +156,11 @@ private void makeRequests(ReadCommand readCommand, Iterable<Replica> replicas)
         if (hasLocalEndpoint)
         {
             logger.trace("reading {} locally", readCommand.isDigestQuery() ? "digest" : "data");
-            Stage.READ.maybeExecuteImmediately(new LocalReadRunnable(command, handler));
+
+            if (TEST_FORCE_ASYNC_LOCAL_READS)
+                new Thread(new LocalReadRunnable(readCommand, handler)).start();

Review comment:
       +1 to this suggestion

##########
File path: src/java/org/apache/cassandra/db/RepairedDataInfo.java
##########
@@ -34,12 +36,16 @@
 import org.apache.cassandra.tracing.Tracing;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
+@NotThreadSafe
 class RepairedDataInfo
 {
-    public static final RepairedDataInfo NULL_REPAIRED_DATA_INFO = new RepairedDataInfo(null)
+    public static final RepairedDataInfo NO_OP_REPAIRED_DATA_INFO = new RepairedDataInfo(null)
     {
-        boolean isConclusive(){ return true; }
-        ByteBuffer getDigest(){ return ByteBufferUtil.EMPTY_BYTE_BUFFER; }
+        @Override
+        public UnfilteredPartitionIterator extend(UnfilteredPartitionIterator partitions, DataLimits.Counter limit)
+        {
+           throw new UnsupportedOperationException("Attempted to extend with the no-op implementation!");

Review comment:
       It's a really trivial nit, but this feels kind of counter to the null object pattern being employed here. This could just return `partitions` unchanged to be safe. I get that there isn't a code path which could lead to `extend` being called on this instance, unlike `isConclusive/getDigest`, it's just a tiny bit inconsistent. 
   




-- 
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-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org