jolshan commented on code in PR #15533: URL: https://github.com/apache/kafka/pull/15533#discussion_r1529486638
########## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ########## @@ -1190,10 +1190,11 @@ private CoordinatorResult<ConsumerGroupHeartbeatResponseData, Record> consumerGr .setHeartbeatIntervalMs(consumerGroupHeartbeatIntervalMs); // The assignment is only provided in the following cases: - // 1. The member reported its owned partitions; - // 2. The member just joined or rejoined to group (epoch equals to zero); - // 3. The member's assignment has been updated. - if (ownedTopicPartitions != null || memberEpoch == 0 || hasAssignedPartitionsChanged(member, updatedMember)) { + // 1. The member sent a full request. It does so when joining or rejoining the group; or + // on any errors (e.g. timeout). + // 2. The member's assignment has been updated. + boolean isFullRequest = memberEpoch == 0 || (rebalanceTimeoutMs != -1 && subscribedTopicNames != null && ownedTopicPartitions != null); Review Comment: I think I'm missing how ```(rebalanceTimeoutMs != -1 && subscribedTopicNames != null && ownedTopicPartitions != null)``` maps to ```// It does so when joining or rejoining the group; or on any errors (e.g. timeout).``` Are we instead saying that this particular case -- ie setting all these 3 fields indicates a "full" request? I think we should update the comment to make this a bit clearer. -- 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