kamalcph commented on code in PR #16257:
URL: https://github.com/apache/kafka/pull/16257#discussion_r1686572470


##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadata.java:
##########
@@ -241,7 +255,7 @@ public RemoteLogSegmentMetadata 
createWithUpdates(RemoteLogSegmentMetadataUpdate
 
         return new RemoteLogSegmentMetadata(remoteLogSegmentId, startOffset,
                 endOffset, maxTimestampMs, rlsmUpdate.brokerId(), 
rlsmUpdate.eventTimestampMs(),
-                segmentSizeInBytes, rlsmUpdate.customMetadata(), 
rlsmUpdate.state(), segmentLeaderEpochs);
+                segmentSizeInBytes, rlsmUpdate.customMetadata(), 
rlsmUpdate.state(), segmentLeaderEpochs, tieredEpoch);

Review Comment:
   should we also update `hashcode` and `equals` with tieredEpoch field?



##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogSegmentMetadata.java:
##########
@@ -104,10 +110,11 @@ public RemoteLogSegmentMetadata(RemoteLogSegmentId 
remoteLogSegmentId,
                                     int segmentSizeInBytes,
                                     Optional<CustomMetadata> customMetadata,
                                     RemoteLogSegmentState state,
-                                    Map<Integer, Long> segmentLeaderEpochs) {
+                                    Map<Integer, Long> segmentLeaderEpochs, 
int tieredEpoch) {

Review Comment:
   nit: newline



##########
storage/src/main/java/org/apache/kafka/server/log/remote/metadata/storage/serialization/RemoteLogSegmentMetadataTransform.java:
##########
@@ -83,7 +83,7 @@ public RemoteLogSegmentMetadata 
fromApiMessageAndVersion(ApiMessageAndVersion ap
                 new RemoteLogSegmentMetadata(remoteLogSegmentId, 
record.startOffset(), record.endOffset(),
                                              record.maxTimestampMs(), 
record.brokerId(),
                                              record.eventTimestampMs(), 
record.segmentSizeInBytes(),
-                                             segmentLeaderEpochs);
+                                             segmentLeaderEpochs, 0);

Review Comment:
   Why we didn't made similar change in `toApiMessageAndVersion` method?



##########
metadata/src/main/resources/common/metadata/TopicRecord.json:
##########
@@ -17,12 +17,16 @@
   "apiKey": 2,
   "type": "metadata",
   "name": "TopicRecord",
-  "validVersions": "0",
+  "validVersions": "0-1",

Review Comment:
   For my understanding:
   
   If the `__cluster_metadata` topic contains the `TopicRecord` message with 
both the versions 0 and 1. Will the brokers with old binary able to deserialize 
them?



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