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


##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1783,12 +1782,14 @@ public void 
testDescribeTopicsWithDescribeTopicPartitionsApiErrorHandling() thro
         }
     }
 
-    @Flaky("KAFKA-18441")
     @Test
     public void testAdminClientApisAuthenticationFailure() {
         Cluster cluster = mockBootstrapCluster();
         try (final AdminClientUnitTestEnv env = new 
AdminClientUnitTestEnv(Time.SYSTEM, cluster,
-                newStrMap(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, 
"1000"))) {
+                newStrMap(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, "1000",
+                    // The default "retry.backoff.ms" is 100. if following 
assertion can't finish in 100L,
+                    // the test will fail. Set it to 5000 to make sure the 
test can finish in time.
+                    AdminClientConfig.RETRY_BACKOFF_MS_CONFIG, "5000"))) {
             env.kafkaClient().setNodeApiVersions(NodeApiVersions.create());
             
env.kafkaClient().createPendingAuthenticationError(cluster.nodes().get(0),
                     TimeUnit.DAYS.toMillis(1));

Review Comment:
   Agree that there is something about the `MockClient` we're not getting here 
(or is off and we might be hiding it). 
   
   This `TimeUnit.DAYS.toMillis(1)` was supposed to achieve exactly what we're 
having to do by setting a higher `RETRY_BACKOFF_MS_CONFIG`. The 
`createPendingAuthenticationError` takes the backoff param and sets it as the 
backoff for the node
   
https://github.com/apache/kafka/blob/97a228070e7711f7547e245d82771f9b81dda96d/clients/src/test/java/org/apache/kafka/clients/MockClient.java#L170
   
   So the MockClient shouldn't generate a new fetchMetadata for 1 day I expect.
   
   
https://github.com/apache/kafka/blob/97a228070e7711f7547e245d82771f9b81dda96d/clients/src/test/java/org/apache/kafka/clients/MockClient.java#L606
   
   That being said, agree with @AndrewJSchofield 's suggestion, we could add 
the config to stabilize the very noisy test, and have a follow-up jira to check 
why the MockClient is requiring an explicit backoff config in this case.



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