chenyulin0719 commented on code in PR #18475:
URL: https://github.com/apache/kafka/pull/18475#discussion_r1910860848
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java:
##########
@@ -167,6 +169,8 @@ private PendingFetchCommittedRequest(final
Set<TopicPartition> requestedPartitio
@Override
public NetworkClientDelegate.PollResult poll(final long currentTimeMs) {
// Copy the outgoing request list and clear it.
+ maybeRetryListOffsetsRequests(false);
Review Comment:
Hi @lianetm Thanks for the detailed explanation. I think I did miss
something when writing the solution.
For the two aspects of the solution:
1. when to request metadata update
2. when to retry requests
The question driven me to only change 2 was "Do we really need to trigger
metadata update for retriable errors?".
My previous answer was "No", but I just realized that I was wrong.
Requesting a metadata update for "ReplicaNotAvailable" or "KAFKA_STORAGE_ERROR"
is meaningless, but it is necessary for other retriable errors
([Link](https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcherUtils.java#L132-L146))
(e.g. NOT_LEADER_OR_FOLLOWER, LEADER_NOT_AVAILABLE, and UNKNOWN_LEADER_EPOCH).
And you're right, I missed the backoff for fetchOffsets. I also missed to
trigger metadata update after StaleMetadataException.
I will revert the first commit and change this PR to address "when to
request metadata update".
Besides, I think we should call `metadata.requestUpdate` right after calling
`requestsToRetry.add(listOffsetsRequestState)`. So it will be blow, instead of
the one in completeExceptionally.
1.
https://github.com/apache/kafka/blob/cd061c8039d615f6584eab522c19b407317ba031/clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java#L579-L581
2.
https://github.com/apache/kafka/blob/cd061c8039d615f6584eab522c19b407317ba031/clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java#L523-L525
Please correct me if I'm wrong. I greatly appreciate your insightful
feedback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
