Regarding the fetch.partition.max.bytes, there was some discussion on the Jira around removing this setting completely. It’s probably not the easiest thing for user’s to set, so there is certainly an argument for removing it. This would have the side effect that a catching up broker would fill responses from a single partition at a time, but as we’re ensuring fairness across requests I’m still left wondering if we need a partition level limit at all?
One middle ground would keep it configurable but with a default of Int.MaxValue. B > On 10 Aug 2016, at 11:56, Andrey L. Neporada <anepor...@yandex-team.ru> wrote: > > 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. >
