Re: [PR] CASSANDRA-20244: Avoid double loading in Cassandra side of journal [cassandra]

["belliottsmith (via GitHub)" <gi...@apache.org>]

belliottsmith commented on code in PR #3823:
URL: https://github.com/apache/cassandra/pull/3823#discussion_r1934230254


##########
src/java/org/apache/cassandra/service/accord/AccordJournalTable.java:
##########
@@ -492,15 +489,15 @@ public K key()
         }
 
         @Override
-        public void readAllForKey(K key, RecordConsumer<K> reader)
+        public void readAllForKey(K key, RecordConsumer<K> reader, boolean includeUpdates)

Review Comment:
   `includeUpdates` seems to imply normally we don't, whereas here we're _only_ reading updates right?
   
   Perhaps it would be clearer to accept a segment id or position we read from? 
   
   Are we also confident this is fine with compaction? Can anything weird happen if new data is compacted with old data?



-- 
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