Ok thanks, if you guys are seeing this at LinkedIn then the motivation makes more sense.
Eno On Tue, Aug 21, 2018 at 5:39 PM, Becket Qin <becket....@gmail.com> wrote: > Hi Eno, > > Thanks for the comments. This KIP is not really about improving the > performance in general. It is about ensuring the cluster state can still be > updated quickly even if the brokers are under heavy load. > > We have seen quite often that it took dozens of seconds for a broker to > process the requests sent by the controller when the cluster is under heavy > load. This leads to the issues Lucas mentioned in the motivation part. > > Thanks, > > Jiangjie (Becket) Qin > > > On Aug 20, 2018, at 11:33 PM, Eno Thereska <eno.there...@gmail.com> > wrote: > > > > 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 > >>> > >> > >
