jsancio commented on code in PR #20707:
URL: https://github.com/apache/kafka/pull/20707#discussion_r2738170485


##########
raft/src/main/java/org/apache/kafka/snapshot/SnapshotReader.java:
##########
@@ -57,6 +57,12 @@ public interface SnapshotReader<T> extends AutoCloseable, 
Iterator<Batch<T>> {
      */
     long lastContainedLogTimestamp();
 
+    /**
+     * Returns true if the snapshot has been committed.
+     * Uncommitted bootstrap snapshots return false.
+     */
+    boolean isCommittedSnapshot();

Review Comment:
   Thanks. I think that returning uncommitted snapshot through 
`RaftListener.handleLoadSnapshot` has a high probability of the 
user/application handling it wrong. Instead of changing the semantic of 
`handleLoadSnapshot` to return uncommitted snapshots. I think that we should 
add another callback to `RaftListener`, for example 
`handleLoadBootstrap(SnapshotReader<T>)`. This make it clearer to the user the 
semantic difference between the two snapshots.
   
   If we don't do this, all KRaft application need to remember to check if the 
returned snapshot is uncommitted or committed. Which is not clear from the 
`RaftListener` interface and the documentation of that interface.



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

Reply via email to