cmccabe commented on code in PR #13345:
URL: https://github.com/apache/kafka/pull/13345#discussion_r1146542326
##########
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:
This will break compatibility with older snapshot versions. Certainly it
breaks compatibility with the oldest raft snapshots, which had no control
records. I don't recall the exact version where we added the initial control
record. You need to find that version and then we can discuss whether breaking
compatibility here is acceptable. If it is, we need to have an appropriate
error message that talks about why the older version is not supported and which
versions are supported.
--
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]
