clolov commented on code in PR #24078:
URL: https://github.com/apache/kafka/pull/24078#discussion_r4455146952
##########
tools/src/main/java/org/apache/kafka/tools/DumpLogSegments.java:
##########
@@ -789,6 +790,25 @@ protected JsonNode valueAsJson(ApiMessage message, short
version) {
}
}
+ // Package private for testing
+ static class MirrorStateMessageParser extends
CoordinatorRecordMessageParser {
+ public MirrorStateMessageParser() {
+ super(new MirrorCoordinatorRecordSerde());
+ }
+
+ @Override
+ protected JsonNode keyAsJson(ApiMessage message) {
+ return
org.apache.kafka.coordinator.mirror.generated.CoordinatorRecordJsonConverters
+ .writeRecordKeyAsJson(message);
+ }
+
+ @Override
+ protected JsonNode valueAsJson(ApiMessage message, short version) {
Review Comment:
Could you print state and previousState as names instead of bytes? Right now
the output is `"state":8,"previousState":3`, so readers have to look the
numbers up. Even the test needs // FAILED and // MIRRORING comments to be
readable.
TransactionLogMessageParser.valueAsJson already does this for
transactionStatus. It rewrites the field with the state name and keeps the raw
number when the ID is unknown. tools already depends on server-common, so
MirrorPartitionState.fromValue(byte) is available. It throws
IllegalArgumentException for unknown values, so catching that and keeping the
number would match the transaction parser's behaviour. The expected JSON in
mirrorStateRecords() would need updating to match.
--
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]
