[GitHub] [kafka] feyman2016 commented on a change in pull request #10593: KAFKA-10800 Validate the snapshot id when the state machine creates a snapshot

[GitBox]

feyman2016 commented on a change in pull request #10593:
URL: https://github.com/apache/kafka/pull/10593#discussion_r619795184



##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -2268,6 +2269,20 @@ private Long append(int epoch, List<T> records, boolean 
isAtomic) {
         );
     }
 
+    private void validateSnapshotId(OffsetAndEpoch snapshotId) {
+        Optional<LogOffsetMetadata> highWatermarkOpt = 
quorum().highWatermark();
+        if (!highWatermarkOpt.isPresent() || highWatermarkOpt.get().offset <= 
snapshotId.offset) {

Review comment:
       Conceptually, the `snapshotId.offset=highWatermarkOpt.get().offset` is 
ok, because the record at `snapshotId.offset` is not included in the snapshot, 
but I'm not sure if there are other restrictions because in the Jira 
description, it says: `The end offset and epoch of the snapshot is less than 
the high-watermark`, please kindly advice @jsancio 




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

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