Thanks, Harsha! If there are no other concerns, I will start the vote later today.
On Mon, Jan 14, 2019 at 6:04 PM Harsha <ka...@harsha.io> wrote: > +1 to include in 2.2. Thanks Rajini for sharing the details. > > On Mon, Jan 14, 2019, at 9:10 AM, Rajini Sivaram wrote: > > This is the result of the tests Gardner did before deploying the patch > > (thanks Gardner!): > > > > *We used the Trogdor `ConnectionStress` workload to test the lazy buffer > > allocation patch (which has already been merged to AK) and the bounded > > acceptor queue patch. We didn't see any performance difference between > the > > two patch sets in `connectsPerSec` and there was nothing outstanding in > the > > Kafka JMX metrics. We were able to get pre-patch AK to run OOM reliably > > though during the `ConnectionStress` test. When running the same > > configuration post-patch, we were not able to get Kafka to run OOM.* > > > > I won't have time to do any further performance tests before 2.2 KIP > > freeze. Are there any concerns about including this KIP in 2.2? If not, I > > will start voting later this week. > > > > Thanks, > > > > Rajini > > > > On Fri, Dec 14, 2018 at 12:13 PM Rajini Sivaram <rajinisiva...@gmail.com > > > > wrote: > > > > > Hi Harsha, > > > > > > I am not sure if we have numbers for connection bursts. But since we > have > > > the code, I can run some tests with and without the change and > provided the > > > results. > > > > > > Hi Edo, > > > > > > There is no reason why we can't make num.network.threads a listener > > > config that allows different listeners to use different number of > threads. > > > Have you run into any issues with the limitation of a single value for > the > > > broker? It will be good to get feedback from the community on whether > this > > > will be a useful change. Perhaps we could do it as a follow-on KIP if > > > required. > > > > > > On Fri, Dec 14, 2018 at 10:33 AM Edoardo Comar <eco...@uk.ibm.com> > wrote: > > > > > >> Hi Rajini > > >> > > >> thanks for the KIP! > > >> I noticed (from the KIP text) the new > > >> > Config option: Name: max.connections > > >> > The config may be prefixed with listener prefix to specify different > > >> limits for different listeners, enabling inter-broker connections to > be > > >> created even if there are a large number of client connections on a > > >> different listener. > > >> > > >> do you think it would make sense to also allow the > `num.network.threads` > > >> to have an optional per-listener prefix ? > > >> > > >> ciao, > > >> Edo > > >> -------------------------------------------------- > > >> > > >> Edoardo Comar > > >> > > >> IBM Event Streams > > >> IBM UK Ltd, Hursley Park, SO21 2JN > > >> > > >> > > >> Rajini Sivaram <rajinisiva...@gmail.com> wrote on 11/12/2018 > 18:22:03: > > >> > > >> > From: Rajini Sivaram <rajinisiva...@gmail.com> > > >> > To: dev <dev@kafka.apache.org> > > >> > Date: 11/12/2018 18:22 > > >> > Subject: Re: [DISCUSS] KIP-402: Improve fairness in SocketServer > > >> processors > > >> > > > >> > Hi Harsha, > > >> > > > >> > Thanks for reviewing the KIP. > > >> > > > >> > 1) Yes, agree that we also need a max.connections configuration > > >> per-broker. > > >> > I was thinking of doing that in a separate KIP, but I could add that > > >> here > > >> > as well. > > >> > 2) The number of connections processed in each iteration doesn't > feel > > >> like > > >> > an externalizable config.It is not a limit on connection rate, it is > > >> simply > > >> > ensuring that existing connections are processed by each Processor > after > > >> > atmost every 20 new connections. It will be hard to describe this > > >> > configuration for users to enable configuring this in a way that is > > >> > suitable for a connection flood since it would depend on the number > of > > >> > factors like existing connection count etc. It feels like we should > > >> come > > >> up > > >> > with a number that works well. We have been running with this code > for a > > >> > while and so far haven't run into any noticeable degradations with > 20. > > >> > > > >> > > > >> > > > >> > On Tue, Dec 11, 2018 at 6:03 PM Harsha <ka...@harsha.io> wrote: > > >> > > > >> > > Hi Rajini, > > >> > > Overall KIP looks good to me. Is it possible to > use > > >> > > max.connections config that we already have, althought its per IP. > > >> > > But broker level max.connections would also be good have to guard > > >> against > > >> > > DOS'ing a broker. > > >> > > Eitherway having constant like 20 without a configurable option > > >> doesn't > > >> > > sound right and as the KIP states that one can use > > >> num.network.threads > > >> to > > >> > > increase this capacity, it still not a viable option. Most of the > time > > >> > > users tend to keep network threads minimal and given this > > >> configuration > > >> > > will only need when a burst of requests comes through , allowing > > >> users > > >> to > > >> > > choose that ceiling would be beneficial. Can you add any details > on > > >> why 20 > > >> > > is sufficient , with default num.network.threads with 3 if one > broker > > >> is > > >> > > getting more than 60 simultaneous connections this would result > in > > >> > > perceived slower responses from client side right? > > >> > > > > >> > > Thanks, > > >> > > Harsha > > >> > > > > >> > > > > >> > > On Tue, Dec 11, 2018, at 2:48 AM, Rajini Sivaram wrote: > > >> > > > Hi all, > > >> > > > > > >> > > > I have submitted a KIP to improve fairness in channel > processing in > > >> > > > SocketServer to protect brokers from connection storms: > > >> > > > > > >> > > > - > > >> > > > > > >> > > > > > >> > > INVALID URI REMOVED > > >> > > > >> > > >> > u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D402-253A-2BImprove-2Bfairness-2Bin-2BSocketServer-2Bprocessors&d=DwIBaQ&c=jf_iaSHvJObTbx- > > >> > > > >> > > >> > siA1ZOg&r=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=948jiSDJojcN4XQb2LvdSgzKb4qIVwsFcJLf- > > >> > lTN5lo&s=exvoh8BxNf59LtbVmm1e0lzGzmjGS2UjoQMffB3Pc04&e= > > >> > > > > > >> > > > Feedback and suggestions welcome. > > >> > > > > > >> > > > Thank you, > > >> > > > > > >> > > > Rajini > > >> > > > > >> > > >> Unless stated otherwise above: > > >> IBM United Kingdom Limited - Registered in England and Wales with > number > > >> 741598. > > >> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire > PO6 3AU > > >> > > > >
