Hi folks, I looked at the previous numbers that Lucas provided (thanks!) but it's still not clear to me whether the performance benefits justify the added complexity. I'm looking for some intuition here (a graph would be great but not required): for a small/medium/large cluster, what are the expected percentage of control requests today that will benefit from the change? It's a bit hard to go through this level of detail without knowing the expected end-to-end benefit. The best folks to answer this might be ones running such clusters, and ideally should pitch in with some data.
Thanks Eno 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 > > >
