KAFKA-4708: Fix Transient Failure in BrokerApiVersionsCommandTest.che… #2489
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
| @@ -39,7 +40,17 @@ object BrokerApiVersionsCommand { | |||
| def execute(args: Array[String], out: PrintStream): Unit = { | |||
| val opts = new BrokerVersionCommandOptions(args) | |||
| val adminClient = createAdminClient(opts) | |||
| val brokerMap = adminClient.listAllBrokerVersionInfo() | |||
|
|
|||
| // Wait until AdminClient#findAllBrokers returns a non-empty list of brokers | |||
There was a problem hiding this comment.
Maybe we could move this to a separate method in AdminClient? Something like awaitBrokers?
There was a problem hiding this comment.
Just my 2 cents. I actually think this logic is waiting is better here unless we have more use-case for this awaitBrokers(). The idea is to focus AdminClient on the admin operation.
I am wondering if the code below may be slightly better:
var brokerMap = Map.empty[Node, Try[NodeApiVersions]]
do {
Thread.sleep(50)
brokerMap = adminClient.listAllBrokerVersionInfo()
} while (brokerMap.isEmpty)
There was a problem hiding this comment.
@lindong28 : Good idea. I think using a do...while loop is better.
@hachikuji: Yeah, let's have a method in AdminClient to do this... it seems simple enough.
| while (nodes.isEmpty) { | ||
| nodes = adminClient.findAllBrokers() | ||
| if (nodes.isEmpty) | ||
| Thread.sleep(1) |
There was a problem hiding this comment.
Seems a bit low. Perhaps 50ms would be more reasonable?
…ckBrokerApiVersionCommandOutput
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Thanks for the PR, LGTM, merging to trunk. A couple of notes:
|
…ckBrokerApiVersionCommandOutput Author: Colin P. Mccabe <cmccabe@confluent.io> Reviewers: Jason Gustafson <jason@confluent.io>, Dong Lin <lindong28@gmail.com>, Ismael Juma <ismael@juma.me.uk> Closes apache#2489 from cmccabe/KAFKA-4708
…ckBrokerApiVersionCommandOutput