soarez commented on code in PR #14290: URL: https://github.com/apache/kafka/pull/14290#discussion_r1358318177
########## clients/src/main/resources/common/message/BrokerHeartbeatRequest.json: ########## @@ -18,7 +18,7 @@ "type": "request", "listeners": ["controller"], "name": "BrokerHeartbeatRequest", - "validVersions": "0", + "validVersions": "0-1", Review Comment: I guess this is more confusing now after the discussion around whether to burn the tag for Replicas or not. So this is a good question. [KIP-482](https://cwiki.apache.org/confluence/display/KAFKA/KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields) does say part of the motivation for tagged fields is to add them without bumping the record version: > Finally, sometimes, we want to add extra information to a message without requiring a version bump for everyone who has to read that message. This is particularly important for metadata like consumer offsets. I'd rather we don't have two different definitions for same record version. That can lead to unnecessary confusion. It also feels weird to bump the version for PartitionRecord but not for PartitionChangeRecord. I also don't see any downside with bumping the version. I wonder what @cmccabe 's take is on this. -- 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
