[GitHub] [kafka] jsancio commented on a diff in pull request #13345: KAFKA-13020; Implement reading Snapshot log append timestamp

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

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


##########
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:
   Yes. I got confused with the version numbers. Snapshot header and footer was added in 3.0.0:
   ```bash
   $ git show --summary  d3ec9f940cc4402270cba3ad159ec64a9676a385
   commit d3ec9f940cc4402270cba3ad159ec64a9676a385
   Author: Niket <ni...@users.noreply.github.com>
   Date:   Tue Jun 29 09:37:20 2021 -0700
   
       KAFKA-12952 Add header and footer records for raft snapshots (#10899)
   
       Add header and footer records for raft snapshots. This helps identify when the snapshot
       starts and ends. The header also contains a time.  The time field is currently set to 0.
       KAFKA-12997 will add in the necessary wiring to use the correct timestamp.
   
       Reviewers: Jose Sancio <js...@gmail.com>, Colin P. McCabe <cm...@apache.org>
   
    create mode 100644 clients/src/main/resources/common/message/SnapshotFooterRecord.json
    create mode 100644 clients/src/main/resources/common/message/SnapshotHeaderRecord.json
   ```
   ```bash
   jsancio@jerry:~/work/kafka$ git tag --contains d3ec9f940cc4402270cba3ad159ec64a9676a385
   3.0.0
   3.0.0-rc0
   3.0.0-rc1
   3.0.0-rc2
   3.0.1
   3.0.1-rc0
   3.0.2
   3.0.2-rc0
   3.1.0
   3.1.0-rc0
   3.1.0-rc1
   3.1.1
   3.1.1-rc0
   3.1.1-rc1
   3.1.2
   3.1.2-rc0
   3.2.0
   3.2.0-rc0
   3.2.0-rc1
   3.2.1
   3.2.1-rc2
   3.2.1-rc3
   3.2.2
   3.2.2-rc0
   3.2.3
   3.2.3-rc0
   3.3.0
   3.3.0-rc1
   3.3.0-rc2
   3.3.1
   3.3.1-rc0
   3.3.2
   3.3.2-rc1
   3.4.0
   3.4.0-rc2
   ```



-- 
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-unsubscribe@kafka.apache.org

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