jeffkbkim commented on code in PR #15587:
URL: https://github.com/apache/kafka/pull/15587#discussion_r1557990553


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -2415,6 +2415,22 @@ private CoordinatorResult<Void, Record> 
classicGroupJoinExistingMember(
         return EMPTY_RESULT;
     }
 
+    /**
+     * An overload of {@link 
GroupMetadataManager#completeClassicGroupJoin(ClassicGroup)} used as
+     * timeout operation. It additionally looks up the group by the id and 
checks the group type.
+     * completeClassicGroupJoin will only be called if the group is CLASSIC.
+     */
+    private CoordinatorResult<Void, Record> completeClassicGroupJoin(String 
groupId) {
+        ClassicGroup group;
+        try {
+            group = getOrMaybeCreateClassicGroup(groupId, false);
+        } catch (UnknownMemberIdException | GroupIdNotFoundException 
exception) {

Review Comment:
   The existing coordinator completes responses with UNKNOWN_MEMBER_ID instead 
of GROUP_ID_NOT_FOUND for join and sync. DeleteGroup and DeleteOffset requests 
are completed with GROUP_ID_NOT_FOUND if the group does not exist.
   
   So just returning null and handling them separately for each API might be 
correct. 



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