dajac commented on code in PR #15921:
URL: https://github.com/apache/kafka/pull/15921#discussion_r1598728051
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroupMember.java:
##########
@@ -569,6 +566,26 @@ private static String lookupTopicNameById(
}
}
+ /**
+ * Converts the JoinGroupRequestProtocolCollection to a list of
ClassicProtocol.
+ *
+ * @param protocols The JoinGroupRequestProtocolCollection.
+ * @return The converted list of ClassicProtocol.
+ */
+ public static List<ConsumerGroupMemberMetadataValue.ClassicProtocol>
classicProtocolListFromJoinRequestProtocolCollection(
Review Comment:
Should we keep a unit test for this one?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroupMember.java:
##########
@@ -88,7 +88,7 @@ public Builder(ConsumerGroupMember member) {
this.state = member.state;
this.assignedPartitions = member.assignedPartitions;
this.partitionsPendingRevocation =
member.partitionsPendingRevocation;
- this.classicMemberMetadata = member.classicMemberMetadata;
+ this.classicMemberMetadata = member.classicMemberMetadata == null
? null : member.classicMemberMetadata.duplicate();
Review Comment:
nit: Do we still need this? I suppose that we don't.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroupMember.java:
##########
@@ -495,6 +481,17 @@ public
JoinGroupRequestData.JoinGroupRequestProtocolCollection supportedJoinGrou
return protocols;
}
+ /**
+ * @return The session timeout if the member uses the classic protocol.
+ */
+ public Optional<Integer> classicProtocolSessionTimeout() {
+ if (classicMemberMetadata != null) {
Review Comment:
nit: We could use `useClassicProtocol()`.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/classic/ClassicGroup.java:
##########
@@ -1400,7 +1400,7 @@ public static ClassicGroup fromConsumerGroup(
Optional.ofNullable(member.instanceId()),
member.clientId(),
member.clientHost(),
- member.rebalanceTimeoutMs(),
+ member.classicProtocolSessionTimeout().get(),
Review Comment:
This is incorrect, right? It should be the following line. I also wonder if
we should use get or default to `consumerGroupSessionTimeoutMs`.
--
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
