+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 > >> > >
