mjsax commented on code in PR #19491:
URL: https://github.com/apache/kafka/pull/19491#discussion_r2047459449
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsGroupHeartbeatRequestManager.java:
##########
@@ -237,6 +237,7 @@ private static
List<StreamsGroupHeartbeatRequestData.TopicInfo> getRepartitionTo
repartitionTopicInfo.topicConfigs().add(new
StreamsGroupHeartbeatRequestData.KeyValue().setKey(k).setValue(v))
);
repartitionTopicsInfo.add(repartitionTopicInfo);
+
repartitionTopicInfo.topicConfigs().sort(Comparator.comparing(StreamsGroupHeartbeatRequestData.KeyValue::key).thenComparing(StreamsGroupHeartbeatRequestData.KeyValue::value));
Review Comment:
Why do we need `thanComparing`? Should the `key` not be unique? I see that
`topicConfigs()` returns `List<KeyValue>` (so maybe we don't have strict
guarantee, and I am sure there is reasons why this is not a `Map<String,
Object>` ?).
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsGroupHeartbeatRequestManager.java:
##########
@@ -251,6 +252,7 @@ private static
List<StreamsGroupHeartbeatRequestData.TopicInfo> getChangelogTopi
changelogTopic.getValue().topicConfigs().forEach((k, v) ->
changelogTopicInfo.topicConfigs().add(new
StreamsGroupHeartbeatRequestData.KeyValue().setKey(k).setValue(v))
);
+
changelogTopicInfo.topicConfigs().sort(Comparator.comparing(StreamsGroupHeartbeatRequestData.KeyValue::key).thenComparing(StreamsGroupHeartbeatRequestData.KeyValue::value));
Review Comment:
same question/comment.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsGroupHeartbeatRequestManager.java:
##########
@@ -490,8 +492,12 @@ private NetworkClientDelegate.UnsentRequest
makeHeartbeatRequestAndHandleRespons
}
private NetworkClientDelegate.UnsentRequest makeHeartbeatRequest(final
long currentTimeMs) {
+ StreamsGroupHeartbeatRequestData data =
this.heartbeatState.buildRequestData();
+ if (membershipManager.state() == MemberState.JOINING) {
+ logger.info("Joining group with request {}", data);
Review Comment:
No objections to this change, just unclear why we add it now?
--
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]
