On Wed, Apr 19, 2023, at 20:56, Philip Nee wrote: > Hey Colin, > > I still need to finish reading and understanding the KIP, but I have a > couple of comments despite being ignorant of most of the KRaft stuff. > (Sorry!) > > Firstly, does it make sense to create an extension of the current > AdminClient only to handle these specific KRaft use cases? It seems > cumbersome to have two sets of bootstrap configurations to make the > AdminClient generic enough to handle these specific cases, instead, maybe > it is more obvious (to me) to just extend the AdminClient. What I'm > thinking is KraftAdminClient which continuously uses *bootstrap.servers*, > but make this class only serves the Kraft controllers APIs.
Hi Philip, Thanks for taking a look. We would not want to create a new Admin client class in order to communicate directly with the controllers. The RPCs accepted by the controllers have the same format as the those accepted by the brokers. There is no difference in what is sent over the wire or the data structures that are used in the client. I know you mentioned you haven't had time to read all the KRaft stuff (and there is a lot, I understand). But this is one area that would probably be clarified if you were able to read at least KIP-500 and KIP-590. RPCs sent to the broker are forwarded to the controller (mostly) without modification. > > Secondly, if we want to continue with the design, I'm not yet sure why we > can't continue using the *bootstrap.servers*? I assume when the client gets > the metadata, it should know who it is talking to. I'm just reconsidering > your alternative again. > > A bad idea: Why don't we continue using *bootstrap.servers* but have a > separated config like *kraft.controller* = true/false. I feel like most > users might not know what is a controller and causes some mistakes down the > road. > Well, you called it a bad idea, and I can't help but agree! :) I think "the user might not know what a controller is" is a good sign that they shouldn't be interacting with the controller. Like many AdminClient APIs, this one is intended for use by administrators only. Most users indeed should not need to know what a controller is or interact with it directly. If they do interact it should be very clear that they are doing so. The --controller-bootstrap flag makes it very clear, as does the separate configuration. Let me give an example of the kind of problems that arise if you want to reuse bootstrap.servers for this purpose. If the user grasb a 3.4 Kafka AdminClient and set bootstrap.servers to a set of controller servers, and set direct.to.controller to true, the unknown (in 3.4) configuration will be ignored, and a normal metadata request will be sent without the direct to controller flag. In that case it will give back an error. Confusing, right? Using controller.servers clarifies this situation because the 3.4 client will not support that config, and will complain about the lack of bootstrap.servers. Actually, the situation could get even more confusing than what I described since some older preproduction versions of the KRaft controller did implement the METADATA RPC. So if you send them a METADATA request without any special information, it's not clear what you'll get. Indeed, it would be dependent on the client version and the controller version. The bottom line is that reusing the bootstrap.servers configuration here is not a good idea. best, Colin > Thanks, > P > > On Wed, Apr 19, 2023 at 2:18 PM Colin McCabe <cmcc...@apache.org> wrote: > >> Hi all, >> >> I wrote a short KIP about allowing AdminClient to talk directly with the >> KRaft controller quorum. Check it out here: >> >> https://cwiki.apache.org/confluence/x/Owo0Dw >> >> best, >> Colin >>
