Thanks Becket. Following the convention of KIP-103 makes sense. I've updated the KIP with your proposed changes. Please take another look.
Lucas On Mon, Aug 20, 2018 at 7:29 AM Becket Qin <becket....@gmail.com> wrote: > 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 > > >
