KAFKA-19227: Piggybacked share fetch acknowledgements performance issue #19612
Conversation
github-actions
bot
added
triage
| @@ -2405,13 +2424,10 @@ && checkForStartOffsetWithinBatch(inFlightBatch.firstOffset(), inFlightBatch.las | |||
| lock.writeLock().unlock(); | |||
There was a problem hiding this comment.
should we have a maybeCompleteDelayedShareFetchRequest in this finally block as well?
There was a problem hiding this comment.
There exists code at line 2439 in this method itself for trigger. So for release, first cache is updated and persister call is just to record correct delivery count. So notification to purgatory is not tied with persister.
|
@apoorvmittal10 Thanks for making the change. The change makes sense to me. Can you post the metrics evidence that this is the part of the code causing the problem? |
You should look for ExpiresPerSec - DelayedShareFetch metric with and without produce. |
Sure, I ll update it as part of https://issues.apache.org/jira/browse/KAFKA-19245 |
…ue (apache#19612) The PR fixes the issue when ShareAcknowledgements are piggybacked on ShareFetch. The current default configuration in clients sets `batch size` and `max fetch records` as per the `max.poll.records` config, default 500. Which means all records in a single poll will be fetched and acknowledged. Also the default configuration for inflight records in a partition is 200. Which means prior fetch records has to be acknowledged prior fetching another batch from share partition. The piggybacked share fetch-acknowledgement calls from KafkaApis are async and later the response is combined. If respective share fetch starts waiting in purgatory because all inflight records are currently full, hence when startOffset is moved as part of acknowledgement, then a trigger should happen which should try completing any pending share fetch requests in purgatory. Else the share fetch requests wait in purgatory for timeout though records are available, which dips the share fetch performance. The regular fetch has a single criteria to land requests in purgatory, which is min bytes criteria, hence any produce in respective topic partition triggers to check any pending fetch requests. But share fetch can wait in purgatory because of multiple reasons: 1) Min bytes 2) Inflight records exhaustion 3) Share partition fetch lock competition. The trigger already happens for 1 and current PR fixes 2. We will investigate further if there should be any handling required for 3. Reviewers: Abhinav Dixit <[email protected]>, Andrew Schofield <[email protected]>
The PR fixes the issue when ShareAcknowledgements are piggybacked on
ShareFetch. The current default configuration in clients sets
batch sizeandmax fetch recordsas per themax.poll.recordsconfig,default 500. Which means all records in a single poll will be fetched
and acknowledged. Also the default configuration for inflight records in
a partition is 200. Which means prior fetch records has to be
acknowledged prior fetching another batch from share partition.
The piggybacked share fetch-acknowledgement calls from KafkaApis are
async and later the response is combined. If respective share fetch
starts waiting in purgatory because all inflight records are currently
full, hence when startOffset is moved as part of acknowledgement, then a
trigger should happen which should try completing any pending share
fetch requests in purgatory. Else the share fetch requests wait in
purgatory for timeout though records are available, which dips the share
fetch performance.
The regular fetch has a single criteria to land requests in purgatory,
which is min bytes criteria, hence any produce in respective topic
partition triggers to check any pending fetch requests. But share fetch
can wait in purgatory because of multiple reasons: 1) Min bytes 2)
Inflight records exhaustion 3) Share partition fetch lock competition.
The trigger already happens for 1 and current PR fixes 2. We will
investigate further if there should be any handling required for 3.
Reviewers: Abhinav Dixit [email protected], Andrew Schofield
[email protected]