[GitHub] [kafka] cmccabe commented on a change in pull request #10946: KAFKA-12997: Expose the append time for batches from raft

[GitBox]

cmccabe commented on a change in pull request #10946:
URL: https://github.com/apache/kafka/pull/10946#discussion_r662601954



##########
File path: raft/src/main/java/org/apache/kafka/raft/ReplicatedCounter.java
##########
@@ -106,7 +108,10 @@ public synchronized void handleCommit(BatchReader<Integer> 
reader) {
                     lastCommittedEpoch,
                     lastOffsetSnapshotted
                 );
-                Optional<SnapshotWriter<Integer>> snapshot = 
client.createSnapshot(lastCommittedOffset, lastCommittedEpoch, 0);
+                Optional<SnapshotWriter<Integer>> snapshot = 
client.createSnapshot(

Review comment:
       Do we need to assert that the `BatchReader` returned at least one 
record? If we don't, isn't it possible we could create a snapshot with 
lastCommittedOffset = -1, lastCommittedTimestamp = -1, etc. ?
   
   So I guess we could just add a check that lastCommittedOffset != -1.




-- 
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: jira-unsubscr...@kafka.apache.org

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