kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jun Rao <...@confluent.io>
Subject Re: [VOTE] KIP-72 - Allow putting a bound on memory consumed by Incoming requests
Date Wed, 09 Nov 2016 17:28:31 GMT
Hi, Radai,

Thanks for the KIP. Some comments below.

1. The KIP says "to facilitate faster implementation (as a safety net) the
pool will be implemented in such a way that memory that was not release()ed
(but still garbage collected) would be detected and "reclaimed". this is to
prevent "leaks" in case of code paths that fail to release() properly.".
What are the cases that could cause memory leaks? If we are concerned about
bugs, it seems that it's better to just do more testing to make sure the
usage of the simple implementation (SimpleMemoryPool) is solid instead of
adding more complicated logic (GarbageCollectedMemoryPool) to hide the
potential bugs.

2. I am wondering how much this KIP covers the SSL channel implementation.
2.1 SslTransportLayer maintains netReadBuffer, netWriteBuffer,
appReadBuffer per socket. Should those memory be accounted for in memory
pool?
2.2 One tricky thing with SSL is that during a KafkaChannel.read(), it's
possible for multiple NetworkReceives to be returned since multiple
requests' data could be encrypted together by SSL. To deal with this, we
stash those NetworkReceives in Selector.stagedReceives and give it back to
the poll() call one NetworkReceive at a time. What this means is that, if
we stop reading from KafkaChannel in the middle because memory pool is
full, this channel's key may never get selected for reads (even after the
read interest is turned on), but there are still pending data for the
channel, which will never get processed.

3. The code has the following two methods in MemoryPool, which are not
described in the KIP. Could you explain how they are used in the wiki?
isLowOnMemory()
isOutOfMemory()

4. Could you also describe in the KIP at the high level, how the read
interest bit for the socket is turned on/off with respect to MemoryPool?

5. Should queued.max.bytes defaults to -1 or Long.MAX_VALUE?

Thanks,

Jun

On Mon, Nov 7, 2016 at 1:08 PM, radai <radai.rosenblatt@gmail.com> wrote:

> Hi,
>
> I would like to initiate a vote on KIP-72:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-72%3A+
> Allow+putting+a+bound+on+memory+consumed+by+Incoming+requests
>
> The kip allows specifying a limit on the amount of memory allocated for
> reading incoming requests into. This is useful for "sizing" a broker and
> avoiding OOMEs under heavy load (as actually happens occasionally at
> linkedin).
>
> I believe I've addressed most (all?) concerns brought up during the
> discussion.
>
> To the best of my understanding this vote is about the goal and
> public-facing changes related to the new proposed behavior, but as for
> implementation, i have the code up here:
>
> https://github.com/radai-rosenblatt/kafka/tree/broker-memory
> -pool-with-muting
>
> and I've stress-tested it to work properly (meaning it chugs along and
> throttles under loads that would DOS 10.0.1.0 code).
>
> I also believe that the primitives and "pattern"s introduced in this KIP
> (namely the notion of a buffer pool and retrieving from / releasing to said
> pool instead of allocating memory) are generally useful beyond the scope of
> this KIP for both performance issues (allocating lots of short-lived large
> buffers is a performance bottleneck) and other areas where memory limits
> are a problem (KIP-81)
>
> Thank you,
>
> Radai.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message