KAFKA-20765: Fix OffsetFetch stale-epoch retry spinning in dedup loop#22747
KAFKA-20765: Fix OffsetFetch stale-epoch retry spinning in dedup loop#22747aliehsaeedii wants to merge 1 commit into
Conversation
When an in-flight OffsetFetch fails with STALE_MEMBER_EPOCH and the member already knows a newer epoch, CommitRequestManager retries the request. If a duplicated fetch for the same partitions was chained onto the failed request, the retry could be deduplicated against the already-completed request: PendingRequests.addOffsetFetchRequest matched duplicates only by partitions and did not check whether the matched request was still pending. A completed request may still appear in the buffers while its completion callbacks run, because buffer removal is itself a completion callback and CompletableFuture dependents run LIFO. Chaining onto a completed future fails the retry immediately and synchronously, triggering another retry, another dedup, and so on: a tight loop that never sends an OffsetFetch with the new epoch, never completes the application-level future, and ends in a StackOverflowError. Exclude requests whose future is already done from the duplicate search so the retry is enqueued and re-sent with the current epoch. Also fix the misleading stale-epoch ERROR log (no longer claims the request cannot be retried when a retry follows) and downgrade the per-chained -caller "duplicated, inflight" WARN to an accurate debug message. Add a test that deterministically reproduces the loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| log.debug("Completed request not found in the in-flight buffer (it was " + | ||
| "deduplicated and chained onto an existing request): {}", fetchRequest); |
There was a problem hiding this comment.
dedup/chained it's certainly one of the reasons for this, but I'm not sure it's the only one. E.g, requests that remain unsent (not added to inflight) if we don't have a coordinator?. So maybe we can consider letting the log with what we're sure of (what it had before, dup found but not inflight), and we mention some of the potential reasons in a comment? wdyt?
| " or failed). The request cannot be retried and will fail.", responseError); | ||
| if (memberInfo.memberEpoch.isPresent()) { | ||
| log.debug("OffsetFetch failed with {}. The member has a newer epoch, so the " + | ||
| "request will be retried with it.", responseError); |
There was a problem hiding this comment.
will be retried...if there's time left (worth clarifying, we cannot guarantee it will be retried at this point. The retry logic is elsewhere and based on expiration)
| // The retried request should be sent after the backoff, with the latest member ID and epoch. | ||
| time.sleep(retryBackoffMs); |
There was a problem hiding this comment.
uhm I think there is no backoff here (by the time it fails the new epoch is known already, so we just send it on the next poll). I expect the test should pass without the sleep and that we can remove the comment too.
Fixes KAFKA-20765.
The issue
In the async consumer (
AsyncKafkaConsumer), when an in-flightOffsetFetch fails with
STALE_MEMBER_EPOCHand a refreshed member epochis already known,
CommitRequestManagercorrectly classifies the erroras retriable — but the retry can enter a tight, non-progressing loop
instead of being re-sent:
second request is deduplicated by
PendingRequests.addOffsetFetchRequestand chained onto the first(in-flight) one.
STALE_MEMBER_EPOCH(e.g. anothermember joined and the coordinator bumped the group epoch while the
request was in flight — routine under the KIP-1071 streams protocol
during staggered startup).
request is removed from the in-flight buffer (CompletableFuture
dependents run LIFO, and buffer removal is itself a completion
callback). Its retry is handed back to
addOffsetFetchRequest, whichfinds the original request still in the buffer —
sameRequest()compares only the requested partitions — and chains the retry onto it.
stale-epoch error), so chaining completes the retry's future immediately
and synchronously. The error handler sees
STALE_MEMBER_EPOCHagain,retries again, chains again… a synchronous spin that only ends when the
stack blows up.
Consequences (all observed in a system test run and reproduced in the
unit test):
WARN "A duplicated, inflight, request was identified, but unable to find it in the outbound buffer"repeated hundreds of times withinmilliseconds,
java.lang.StackOverflowErrorsurfacing in SLF4J'stoString()ofOffsetFetchRequestState,(the epoch is stamped at send time — the one stage the loop never
reaches),
In Kafka Streams with the streams rebalance protocol (KIP-1071) this
blocks the stream thread in committed-offset initialization for the full
default.api.timeout.ms, exceedsmax.poll.interval.ms, and gets themember evicted (observed in
streams_broker_down_resilience_testwithstaggered startup). Plain
group.protocol=consumerapps are exposed tothe same spin; KIP-1071 just triggers the preconditions far more easily.
The fix
In
PendingRequests.addOffsetFetchRequest, requests whose future isalready completed are no longer considered duplicates. A completed
request cannot deliver a result to anyone; it only appears in the
buffers transiently while its completion callbacks run. With the check
in place, the retry falls through to being enqueued and re-sent, picking
up the current member epoch at send time.
Also included:
the fix with the exact production signatures) and verifies the retry is
sent with the new epoch and completes both callers' futures.
"cannot be retried" when a retry is about to happen (only logged when
the member has no epoch anymore), and the per-chained-caller
"duplicated, inflight" WARN is now an accurate debug message (it fires
for every deduplicated request, success or failure — expected, not an
anomaly).
Testing
./gradlew :clients:test --tests "org.apache.kafka.clients.consumer.internals.CommitRequestManagerTest"— all pass, including the new
testDuplicatedOffsetFetchFailsWithStaleEpochAndRetriesWithNewEpoch.🤖 Generated with Claude Code
Reviewers: Lianet Magrans lmagrans@confluent.io