lianetm commented on code in PR #16449:
URL: https://github.com/apache/kafka/pull/16449#discussion_r1667045593


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1476,21 +1478,19 @@ public void unsubscribe() {
         acquireAndEnsureOpen();
         try {
             fetchBuffer.retainAll(Collections.emptySet());
-            if (groupMetadata.get().isPresent()) {
-                Timer timer = time.timer(Long.MAX_VALUE);
-                UnsubscribeEvent unsubscribeEvent = new 
UnsubscribeEvent(calculateDeadlineMs(timer));
-                applicationEventHandler.add(unsubscribeEvent);
-                log.info("Unsubscribing all topics or patterns and assigned 
partitions {}",
+            Timer timer = time.timer(Long.MAX_VALUE);
+            UnsubscribeEvent unsubscribeEvent = new 
UnsubscribeEvent(calculateDeadlineMs(timer));
+            applicationEventHandler.add(unsubscribeEvent);
+            log.info("Unsubscribing all topics or patterns and assigned 
partitions {}",
                     subscriptions.assignedPartitions());
 
-                try {
-                    processBackgroundEvents(unsubscribeEvent.future(), timer);
-                    log.info("Unsubscribed all topics or patterns and assigned 
partitions");
-                } catch (TimeoutException e) {
-                    log.error("Failed while waiting for the unsubscribe event 
to complete");
-                }
-                resetGroupMetadata();
+            try {
+                processBackgroundEvents(unsubscribeEvent.future(), timer);
+                log.info("Unsubscribed all topics or patterns and assigned 
partitions");
+            } catch (TimeoutException e) {
+                log.error("Failed while waiting for the unsubscribe event to 
complete");
             }
+            if (groupMetadata.get().isPresent()) resetGroupMetadata();

Review Comment:
   nit: this `resetGroupMetadata` is only used here I believe. We could simply 
have a "maybeResetGroupMetadata();" call here, and move the check for 
groupMetadata.get().isPresent() into it? seems cleaner to me but totally up to 
you.



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