chickenchickenlove commented on code in PR #23856:
URL: https://github.com/apache/kafka/pull/23856#discussion_r4336772690
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -6942,7 +6942,15 @@ public void onLoaded() {
log.info("Loaded consumer group {} with {} members.",
groupId, consumerGroup.members().size());
consumerGroup.members().forEach((memberId, member) -> {
log.debug("Loaded member {} in consumer group {}.",
memberId, groupId);
- scheduleConsumerGroupSessionTimeout(groupId, memberId);
+
+ if (member.useClassicProtocol() &&
member.classicProtocolSessionTimeout().isPresent()) {
+ scheduleConsumerGroupSessionTimeout(
+ groupId,
+ memberId,
+ member.classicProtocolSessionTimeout().get());
Review Comment:
@dajac @liuliuOD thanks for taking the time to look into this.
Initially, I thought that using an explicit branch would make the intent
clearer. However, I realized that the JavaDoc for
`classicProtocolSessionTimeout()` already defines its contract as "The session
timeout if the member uses the classic protocol."
Given this contract, I agree that your suggested approach is better, as it
improves readability and avoids redundant method calls! I've updated the code
accordingly.
--
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]
