dajac commented on code in PR #15921:
URL: https://github.com/apache/kafka/pull/15921#discussion_r1598001232
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroupMember.java:
##########
@@ -209,8 +209,19 @@ public Builder
setSupportedClassicProtocols(JoinGroupRequestData.JoinGroupReques
.setMetadata(protocol.metadata())
)
);
- this.classicMemberMetadata = new
ConsumerGroupMemberMetadataValue.ClassicMemberMetadata()
- .setSupportedProtocols(newSupportedProtocols);
+
+ if (this.classicMemberMetadata == null) {
+ this.classicMemberMetadata = new
ConsumerGroupMemberMetadataValue.ClassicMemberMetadata();
+ }
+
this.classicMemberMetadata.setSupportedProtocols(newSupportedProtocols);
+ return this;
+ }
+
+ public Builder setSessionTimeoutMs(int sessionTimeoutMs) {
Review Comment:
`setSessionTimeoutMs` is a bit misleading here in my opinion because we
don't know whether it applies to the consumer of the classic protocol. I
actually wonder if we should remove `setSupportedClassicProtocols` and
`setSessionTimeoutMs` and use `setClassicMemberMetadata` to set both. The
rational is that we always set both or none. What do you think?
##########
group-coordinator/src/main/resources/common/message/ConsumerGroupMemberMetadataValue.json:
##########
@@ -40,6 +40,8 @@
"default": null, "taggedVersions": "0+", "tag": 0,
"about": "The classic member metadata which includes the supported
protocols of the consumer if it uses the classic protocol. The metadata is null
if the consumer uses the consumer protocol.",
"fields": [
+ { "name": "SessionTimeoutMs", "type": "int32", "versions": "0+",
"default": -1,
Review Comment:
nit: Do we really need to default value?
--
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]
