dajac commented on code in PR #18406:
URL: https://github.com/apache/kafka/pull/18406#discussion_r1914532356


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -1030,6 +1034,8 @@ private void convertToClassicGroup(
         if (joiningMember == null) {
             prepareRebalance(classicGroup, String.format("Downgrade group %s 
from consumer to classic.", classicGroup.groupId()));
         }
+
+        log.info("Converted consumer group {} to classic group.", 
consumerGroup.groupId());

Review Comment:
   ditto about the format.



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -671,7 +671,11 @@ ConsumerGroup getOrMaybeCreateConsumerGroup(
             throw new GroupIdNotFoundException(String.format("Consumer group 
%s not found.", groupId));
         }
 
-        if (group == null || (createIfNotExists && 
maybeDeleteEmptyClassicGroup(group, records))) {
+        if (group == null) {
+            return new ConsumerGroup(snapshotRegistry, groupId, metrics);
+        } else if (createIfNotExists && maybeDeleteEmptyClassicGroup(group, 
records)) {
+            log.info("Got an empty classic group {} when getting consumer 
group. " +
+                "Removed the old classic group and use an empty consumer group 
instead.", groupId);

Review Comment:
   Should we use the format of the other log? `[GroupId {}] Converted the empty 
classic group to a consumer group.`?



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -1110,6 +1116,8 @@ ConsumerGroup convertToConsumerGroup(ClassicGroup 
classicGroup, List<Coordinator
             scheduleConsumerGroupSessionTimeout(consumerGroup.groupId(), 
memberId, member.classicProtocolSessionTimeout().get())
         );
 
+        log.info("Converted classic group {} to consumer group.", 
classicGroup.groupId());

Review Comment:
   ditto.



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -4361,7 +4369,10 @@ CoordinatorResult<Void, CoordinatorRecord> 
classicGroupJoinToClassicGroup(
         // Group is created if it does not exist and the member id is UNKNOWN. 
if member
         // is specified but group does not exist, request is rejected with 
GROUP_ID_NOT_FOUND
         ClassicGroup group;
-        maybeDeleteEmptyConsumerGroup(groupId, records);
+        if (maybeDeleteEmptyConsumerGroup(groupId, records)) {
+            log.info("Got an empty consumer group {} when a classic consumer 
joins. " +
+                "Removed the old consumer group and use an empty consumer 
group instead.", groupId);

Review Comment:
   ditto.



-- 
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

Reply via email to