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


##########
tests/kafkatest/tests/client/consumer_test.py:
##########
@@ -346,11 +356,20 @@ def test_fencing_static_consumer(self, 
num_conflict_consumers, fencing_stage, me
                            timeout_sec=60,
                            err_msg="Timed out waiting for the consumer to 
shutdown")
                 # Wait until the group becomes empty to ensure the instance ID 
is released.
-                # We use the 50-second timeout because the consumer session 
timeout is 45 seconds.
+                self.logger.debug("Stopped consumers waiting for group %s to 
be removed from the group", self.group_id)

Review Comment:
   I don't quite get what we want to say here?



##########
tests/kafkatest/tests/client/consumer_test.py:
##########
@@ -346,11 +356,20 @@ def test_fencing_static_consumer(self, 
num_conflict_consumers, fencing_stage, me
                            timeout_sec=60,
                            err_msg="Timed out waiting for the consumer to 
shutdown")
                 # Wait until the group becomes empty to ensure the instance ID 
is released.
-                # We use the 50-second timeout because the consumer session 
timeout is 45 seconds.
+                self.logger.debug("Stopped consumers waiting for group %s to 
be removed from the group", self.group_id)
+
+                if len(consumer.joined_nodes()) != 0:
+                    self.logger.debug("Not all stopped consumers removed %s. 
Describe output is %s", self.group_id, " 
".join(self.kafka.describe_consumer_group_members(self.group_id)))
+
+                # We use the 60-second timeout because the consumer session 
timeout is 45 seconds adding some time for latency.
                 wait_until(lambda: self.group_id in 
self.kafka.list_consumer_groups(state="empty"),
-                           timeout_sec=50,
+                           timeout_sec=60,
                            err_msg="Timed out waiting for the consumers to be 
removed from the group.")
                 conflict_consumer.start()
+
+                if len(consumer.joined_nodes()) != num_conflict_consumers:
+                    self.logger.debug("All conflict members not in group %s. 
Describe output is %s", self.group_id, " 
".join(self.kafka.describe_consumer_group_members(self.group_id)))

Review Comment:
   similar with this, should this log be in the except after trying to 
`await_members(conflict_consumer, ..)` and not succeeding? logging before 
waiting would lead to false negatives I expect?



##########
tests/kafkatest/tests/client/consumer_test.py:
##########
@@ -346,11 +356,20 @@ def test_fencing_static_consumer(self, 
num_conflict_consumers, fencing_stage, me
                            timeout_sec=60,
                            err_msg="Timed out waiting for the consumer to 
shutdown")
                 # Wait until the group becomes empty to ensure the instance ID 
is released.
-                # We use the 50-second timeout because the consumer session 
timeout is 45 seconds.
+                self.logger.debug("Stopped consumers waiting for group %s to 
be removed from the group", self.group_id)
+
+                if len(consumer.joined_nodes()) != 0:

Review Comment:
   can this condition be true at this point? (I would imagine that if ever 
there were still `joined_nodes`, the condition above on `wait_until(lambda: 
len(consumer.dead_nodes()) == len(consumer.nodes)` would fail before hitting 
this line, but I could be missing something).
   
   If what I imagine is the case, then I guess we should wait in a try/except 
to be able to log what we want in the catch maybe?



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

Reply via email to