Hi! > On 09 Aug 2016, at 20:46, Jun Rao <j...@confluent.io> wrote: > > Hi, Andrey, > > Thanks for the proposal. It looks good overall. Some minor comments. > > 1. It seems that it's bit weird that fetch.partition.max.bytes is a broker > level configuration while fetch.limit.bytes is a client side configuration. > Intuitively, it seems both should be set by the client? If we do that, one > benefit is that we can validate that fetch.limit.bytes >= > fetch.partition.max.bytes on the client side. >
Yes, such cooperative configuration for fetch request may look a bit weird. But I don’t see other options if we want to remove partition limits from fetch request. In this case we need some server-side configuration for partition limits. > 2. Naming wise. fetch.response.max.bytes and replica.fetch.response.max.bytes > seem to be more consistent with our current convention than > fetch.limit.bytes and replica.fetch.limit.bytes. Agree, will rename. > > 3. When you say "This way we can ensure that response size is less than ( > *limit_bytes* + *message.max.bytes*).", it should be "less than > max(limit_bytes, message.max.bytes)", right? > No, I mean that actual response side can be bigger than limit_bytes, but less than limit_bytes + message.max.bytes. This behaviour is a result of algorithm proposed in KIP (and in PR). > Finally, KIP-73 (replication quota) is proposing a similar change to fetch > request protocol. We can probably just combine the two changes into one, > instead of bumping the fetch request version twice. Fine with that. > > Thanks, > > Jun > Thanks, Andrey.
