jsancio commented on code in PR #14141:
URL: https://github.com/apache/kafka/pull/14141#discussion_r1287805598
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -984,11 +991,16 @@ private CompletableFuture<FetchResponseData>
handleFetchRequest(
Throwable cause = exception instanceof ExecutionException ?
exception.getCause() : exception;
- // If the fetch timed out in purgatory, it means no new data
is available,
- // and we will complete the fetch successfully. Otherwise, if
there was
- // any other error, we need to return it.
Errors error = Errors.forException(cause);
- if (error != Errors.REQUEST_TIMED_OUT) {
+ if (error == Errors.REQUEST_TIMED_OUT) {
+ // Note that for this case the calling thread is the
expiration service thread and not the
+ // polling thread.
+ //
+ // If the fetch request timed out in purgatory, it means
no new data is available,
+ // just return the original fetch response.
+ return response;
Review Comment:
Yes. That is what I would like to do in the long-term. In the long-term I
plan to have a more generic event executor that would allow us to keep the
current concurrency model, remove the expiration service thread, and keep all
our deterministic unittest and simulation tests.
It is a bigger change and I didn't want to block this fix on that.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
