niket-goel commented on a change in pull request #10899:
URL: https://github.com/apache/kafka/pull/10899#discussion_r655511645



##########
File path: 
clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java
##########
@@ -664,4 +666,55 @@ private static void writeLeaderChangeMessage(ByteBuffer 
buffer,
         builder.close();
     }
 
+    public static MemoryRecords withSnapshotHeaderRecord(
+            long initialOffset,
+            long timestamp,
+            int leaderEpoch,
+            ByteBuffer buffer,
+            MetadataSnapshotHeaderRecord snapshotHeaderRecord) {

Review comment:
       Thanks. I was trying to find some info around styling. Will follow this 
across the PR.

##########
File path: 
raft/src/main/java/org/apache/kafka/raft/internals/BatchAccumulator.java
##########
@@ -233,6 +235,68 @@ public void appendLeaderChangeMessage(LeaderChangeMessage 
leaderChangeMessage, l
         }
     }
 
+
+    public void appendSnapshotHeaderMessage(MetadataSnapshotHeaderRecord 
snapshotHeaderRecord) {
+        appendLock.lock();
+        try {
+            // Ideally there should be nothing in the batch here.
+            // TODO verify this

Review comment:
       I am concerned about improper usage in the future which might lead to 
stamping the header after other records in the file. Is asserting on the fact 
that the current batch has no records as this time valid? It should be an error 
if it isn't.
   
   What do you think?




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


Reply via email to