FrankYang0529 commented on code in PR #19346:
URL: https://github.com/apache/kafka/pull/19346#discussion_r2051678587


##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsIterator.java:
##########
@@ -346,29 +343,12 @@ private T decodeDataRecord(Optional<ByteBuffer> key, 
Optional<ByteBuffer> value)
     }
 
     private static ControlRecord decodeControlRecord(Optional<ByteBuffer> key, 
Optional<ByteBuffer> value) {
-        if (key.isEmpty()) {
-            throw new IllegalArgumentException("Missing key in the record when 
a key was expected");
-        } else if (key.get().remaining() == 0) {
+        if (key.get().remaining() == 0) {

Review Comment:
   Should we check whether key optional is empty before using get function?



##########
raft/src/main/java/org/apache/kafka/raft/ControlRecord.java:
##########
@@ -23,54 +23,51 @@
 import org.apache.kafka.common.message.VotersRecord;
 import org.apache.kafka.common.protocol.ApiMessage;
 import org.apache.kafka.common.record.ControlRecordType;
+import org.apache.kafka.common.record.ControlRecordUtils;
 
+import java.nio.ByteBuffer;
 import java.util.Objects;
+import java.util.Optional;
+
+import static org.apache.kafka.common.record.ControlRecordType.LEADER_CHANGE;
 
 public final class ControlRecord {
     private final ControlRecordType recordType;
     private final ApiMessage message;
 
-    private static void throwIllegalArgument(ControlRecordType recordType, 
ApiMessage message) {
-        throw new IllegalArgumentException(
-            String.format(
-                "Record type %s doesn't match message class %s",
-                recordType,
-                message.getClass()
-            )
-        );
+    public static ControlRecord of(Optional<ByteBuffer> key, 
Optional<ByteBuffer> value) {

Review Comment:
   After this PR, the only reference of this function is 
`RecordsIterator#decodeControlRecord`. If we can check Optional there, we can 
use ByteBuffer directly in this function.



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