chia7712 commented on code in PR #19346:
URL: https://github.com/apache/kafka/pull/19346#discussion_r2047432004
##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsIterator.java:
##########
@@ -358,17 +355,6 @@ private static ControlRecord
decodeControlRecord(Optional<ByteBuffer> key, Optio
throw new IllegalArgumentException("Got an unexpected empty value
in the record");
}
- ControlRecordType type = ControlRecordType.parse(key.get());
-
- final ApiMessage message = switch (type) {
- case LEADER_CHANGE ->
ControlRecordUtils.deserializeLeaderChangeMessage(value.get());
- case SNAPSHOT_HEADER ->
ControlRecordUtils.deserializeSnapshotHeaderRecord(value.get());
- case SNAPSHOT_FOOTER ->
ControlRecordUtils.deserializeSnapshotFooterRecord(value.get());
- case KRAFT_VERSION ->
ControlRecordUtils.deserializeKRaftVersionRecord(value.get());
- case KRAFT_VOTERS ->
ControlRecordUtils.deserializeVotersRecord(value.get());
- default -> throw new
IllegalArgumentException(String.format("Unknown control record type %s", type));
- };
-
- return new ControlRecord(type, message);
+ return ControlRecord.of(key, value);
Review Comment:
either key or value is impossible to be empty, right?
##########
raft/src/main/java/org/apache/kafka/raft/ControlRecord.java:
##########
@@ -70,7 +87,10 @@ public ControlRecord(ControlRecordType recordType,
ApiMessage message) {
default:
throw new IllegalArgumentException(String.format("Unknown
control record type %s", recordType));
}
+ return new ControlRecord(recordType, message);
Review Comment:
Could you please consider streamlining the method with message only?
```java
public static ControlRecord of(ApiMessage message) {
ControlRecordType recordType;
if (message instanceof LeaderChangeMessage)
recordType = ControlRecordType.LEADER_CHANGE;
else if (message instanceof SnapshotHeaderRecord)
recordType = ControlRecordType.SNAPSHOT_HEADER;
else if (message instanceof SnapshotFooterRecord)
recordType = ControlRecordType.SNAPSHOT_FOOTER;
else if (message instanceof KRaftVersionRecord)
recordType = ControlRecordType.KRAFT_VERSION;
else if (message instanceof VotersRecord)
recordType = ControlRecordType.KRAFT_VOTERS;
else
throw new IllegalArgumentException(String.format("Unknown
control record type %s", message.getClass()));
return new ControlRecord(recordType, message);
}
```
--
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
