Hi Lucas, In KIP-103, we introduced a convention to define and look up the listeners. So it would be good if the later KIPs can follow the same convention.
>From what I understand, the advertised.listeners is actually designed for our purpose, i.e. providing a list of listeners that can be used in different cases. In KIP-103 it was used to separate internal traffic from the external traffic. It is not just for the user traffic or data only. So adding a controller listener is not repurposing the config. Also, ZK structure is only visible to brokers, the clients will still only see the listeners they are seeing today. For this KIP, we are essentially trying to separate the controller traffic from the inter-broker data traffic. So adding a new listener.name.for.controller config seems reasonable. The behavior would be: 1. If the listener.name.for.controller is set, the broker-controller communication will go through that listener. 2. Otherwise, the controller traffic falls back to use inter.broker.listener.name or inter.broker.security.protocol, which is the current behavior. Regarding updating the security protocol with one line change v.s two-lines change, I am a little confused, can you elaborate? Regarding the possibility of hurry and misreading. It is the system admin's responsibility to configure the right listener to ensure that different kinds of traffic are using the correct endpoints. So I think it is better that we always follow the same of convention instead of doing it in different ways. Thanks, Jiangjie (Becket) Qin On Fri, Aug 17, 2018 at 4:34 AM, Lucas Wang <lucasatu...@gmail.com> wrote: > Thanks for the review, Becket. > > (1) After comparing the two approaches, I still feel the current writeup is > a little better. > a. The current writeup asks for an explicit endpoint while reusing the > existing "inter.broker.listener.name" with the exactly same semantic, > and your proposed change asks for a new listener name for controller while > reusing the existing "advertised.listeners" config with a slight semantic > change since a new controller endpoint needs to be added to it. > Hence conceptually the current writeup requires one config change instead > of two. > Also with one listener name, e.g. INTERNAL, for inter broker traffic, > instead of two, e.g. "INTERNAL" and "CONTROLLER", > if an operator decides to switch from PLAINTEXT to SSL for internal > traffic, chances are that she wants to upgrade > both controller connections and data connections, she only needs to update > one line in > the "listener.security.protocol.map" config, and avoids possible mistakes. > > > b. When this KIP is picked up by an operator who is in a hurry without > reading the docs, if she sees a > new listener name for controller is required, and chances are there is > already a list of listeners, > it's possible for her to simply choose an existing listener name, without > explicitly creating > the new CONTROLLER listener and endpoints. If this is done, Kafka will be > run with the existing > behavior, defeating the purpose of this KIP. > In comparison, if she sees a separate endpoint is being asked, I feel it's > unlikely for her to > copy and paste an existing endpoint. > > Please let me know your comments. > > (2) Good catch, it's a typo, and it's been fixed. > > Thanks, > Lucas >
