dajac commented on code in PR #16825:
URL: https://github.com/apache/kafka/pull/16825#discussion_r1707020093
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorService.java:
##########
@@ -1176,4 +1176,34 @@ private <IN, OUT> OUT handleOperationException(
return handler.apply(apiError.error(), apiError.message());
}
}
+
+ /**
+ * This is the handler used by offset fetch operations to convert errors
to coordinator errors.
+ *
+ * @param request The OffsetFetchRequestGroup request.
+ * @param exception The exception to handle.
+ * @return The OffsetFetchRequestGroup response.
+ */
+ private OffsetFetchResponseData.OffsetFetchResponseGroup
handleOffsetFetchException(
+ OffsetFetchRequestData.OffsetFetchRequestGroup request,
+ Throwable exception
+ ) {
+ ApiError apiError = ApiError.fromThrowable(exception);
+
+ switch (apiError.error()) {
Review Comment:
I think that we should actually handle all the errors handled by
`handleOperationException` too. A write operation could actually return some of
those at any time. What do you think?
Handling `UNKNOWN_SERVER_ERROR` while we are here will also be a nice
addition.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorService.java:
##########
@@ -1176,4 +1176,34 @@ private <IN, OUT> OUT handleOperationException(
return handler.apply(apiError.error(), apiError.message());
}
}
+
+ /**
+ * This is the handler used by offset fetch operations to convert errors
to coordinator errors.
+ *
+ * @param request The OffsetFetchRequestGroup request.
+ * @param exception The exception to handle.
+ * @return The OffsetFetchRequestGroup response.
+ */
+ private OffsetFetchResponseData.OffsetFetchResponseGroup
handleOffsetFetchException(
+ OffsetFetchRequestData.OffsetFetchRequestGroup request,
+ Throwable exception
+ ) {
+ ApiError apiError = ApiError.fromThrowable(exception);
+
+ switch (apiError.error()) {
+ case REQUEST_TIMED_OUT:
+ // Remap REQUEST_TIMED_OUT to NOT_COORDINATOR, otherwise older
consumers will not
Review Comment:
nit: Should we add the version too? We could say consumers on version prior
to 3.9. I say 3.9 because I think that we should cherry-pick your other PR to
3.9 branch.
--
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]
