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


##########
raft/src/main/java/org/apache/kafka/snapshot/RecordsSnapshotReader.java:
##########
@@ -121,9 +122,22 @@ private Optional<Batch<T>> nextBatch() {
             Batch<T> batch = iterator.next();
 
             if (!lastContainedLogTimestamp.isPresent()) {
-                // The Batch type doesn't support returning control batches. 
For now lets just use
-                // the append time of the first batch
-                lastContainedLogTimestamp = 
OptionalLong.of(batch.appendTimestamp());
+                // This must be the first batch which is expected to be a 
control batch with one record for
+                // the snapshot header.
+                if (batch.controlRecords().isEmpty()) {
+                    throw new IllegalStateException("First batch is not a 
control batch with at least one record");

Review Comment:
   It does break backward compatibility for Kafka KRaft versions less than 
3.3.0. Support for header and footer in snapshot was added in Kafka 3.3.0: 
https://github.com/apache/kafka/pull/10899.
   
   We agree to break backwards compatibility for KRaft cluster in version 
3.3.0. It is not possible for a KRaft cluster less than 3.3.0 to upgrade to a 
version later than 3.3.0. So it is safe to assume that every cluster running 
this version has a footer and header in the snapshot file.
   
   Snapshot header and footer are not the only example of where we broke 
backwards compatibility in KRaft 3.3.0.



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

Reply via email to