KAFKA-4716: Fix case when controller cannot be reached #2526
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): |
|
|
||
| final ClientRequest clientRequest = kafkaClient.newClientRequest( | ||
| getBrokerId(), | ||
| getAnyReadyBrokerId(), |
There was a problem hiding this comment.
im not 100% sure, but i think you want this to also hit the controller. i hit the controller in my personal client, and @cmccabe is doing it here https://github.com/apache/kafka/pull/2472/files#diff-7378a806dbf302c1e7a9098c4780d2a8R283
There was a problem hiding this comment.
I don't think so. The metadata can be received from any broker. That's how one bootstraps, e.g., with a bootstrap list, which might not include the controller. cc @ijuma
There was a problem hiding this comment.
The case where you should fetch the metadata from the controller is if you expect the metadata response to include the topic that was just created. For example, if you create a topic, it returns a success and then you ask any broker for the metadata, it may be missing the topic that was just created (since metadata updates are asynchronous). I haven't checked the code to understand if this matters or not.
There was a problem hiding this comment.
This is the code before creating topics. This code just gets the metadata first, which then allows us to figure out which one is the controller.
There was a problem hiding this comment.
Sounds fine then (what else could you do if you don't even know who the controller is :)).
There was a problem hiding this comment.
In fact the current logic in prepareTopic loops until each one of the topic metadata has been propagated to at least one broker. So the above is correct.
There was a problem hiding this comment.
it also depends on what you want to check. asking the controller will tell you what state brokers should eventually be in, checking any random broker will tell you what state that particular one is in now. i.e. if you want to make sure its fully propagated you'd have to check all the brokers.
There was a problem hiding this comment.
I'm doing a limited exploration that addresses the blocker. I completely agree that we need to write down the exact invariants in some of the code, as well as add a ton of unit tests.
There was a problem hiding this comment.
I'm hoping KIP-117 will do that eventually.
There was a problem hiding this comment.
I personally don't think it makes sense to wait for the metadata to propagate to all brokers. A broker may be partitioned away from the controller, but not the clients, for example. I would prefer to make the consumer and producer smarter when they get stale metadata from a random broker.
|
🥇 |
Author: Eno Thereska <eno@confluent.io> Reviewers: Dan Norwood, Ismael Juma, Guozhang Wang Closes #2526 from enothereska/0.10.2-KAFKA-4716
No description provided.