Hi, Andrey,

For 2, we actually can know the next message size. In LogSegment.read(), we
first use the offset index to find the file position close to the requested
offset and then scan the log forward to find the message whose offset is at
or larger than the requested offset. By the time we find such a message, we
know exactly the size of the first message that we need to return in the
fetch response. No additional read is needed. We can probably just
propagate this information back to the caller. I added a comment related to
this in https://issues.apache.org/jira/browse/KAFKA-3810.

Thanks,

Jun

On Tue, Aug 16, 2016 at 4:58 AM, Andrey L. Neporada <
anepor...@yandex-team.ru> wrote:

> Hi Jun!
> Thanks for feedback.
>
> > On 15 Aug 2016, at 20:04, Jun Rao <j...@confluent.io> wrote:
> >
> > Hi, Andrey,
> >
> > Thanks for the update to the wiki. Just a few more minor comments.
> >
> > 1. "If *response_max_bytes* parameter is zero ("no limit"), the request
> is
> > processed *exactly* as before." Instead of using 0, it seems it's more
> > natural to use Int.MAX_INT to preserve the old behaviour.
> >
> OK, done.
>
> > 2. "For the first partition, server always fetches at least
> > *message.max.bytes."
> > *To be more precise, the server only fetches more bytes than
> > *response_max_bytes
> > *(and up *message.max.bytes*t) if there is a message whose size is larger
> > than *response_max_bytes.*
> >
>
> Unfortunately, there is no easy way to obtain the size of next message in
> ReplicaManager:fetchMessages() - you will need to issue extra small read
> from log to find it.
> So, unless I am missing something important, I would like to keep the
> proposal (and algorithm) as is.
>
> > 3. "The solution is to continue fetching from first empty partition in
> > round-robin fashion or to perform random shuffle of partitions." We
> > probably want to make it clearer by saying that this is for ordering the
> > partitions in the fetch request.
> >
> > 4. Just to make it clear. Could you include the new fetch request
> protocol
> > in the wiki (e.g.
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 4+-+Command+line+and+centralized+administrative+operations#KIP-4-
> Commandlineandcentralizedadministrativeoperations-
> MetadataRequest(version1))
> > and mark the new field?
> >
>
> OK, done.
>
> BTW, what is the target version of this KIP? Currently the inter-broker
> protocol version in KIP is set to 0.11.0-IV0. Do we want to target for 0.11
> or maybe somewhat earlier?
>
>
> > Jun
> >
>
> Thanks,
> Andrey.
>
>

Reply via email to