[GitHub] [kafka] hachikuji commented on pull request #10794: KAFKA-12677: parse envelope response to check if not_controller error existed

[GitBox <gi...@apache.org>]

hachikuji commented on pull request #10794:
URL: https://github.com/apache/kafka/pull/10794#issuecomment-864268173


   @showuon Thanks for the patch. One detail in the description does not seem right to me.
   
   > 1. "NOT_CONTROLLER" exception won't be correctly send back, instead, `UNKNOWN_SERVER_ERROR` will be returned. The reason is the `NotControllerException` is wrapped by a `CompletionException` when the `Future` completeExceptionally. And the `CompletionException` will not match any Errors we defined, so the `UNKNOWN_SERVER_ERROR` will be returned.
   
   The client is not necessarily trying to reach the controller. It is sending a request to a broker and the fact that we forward it to a controller is an implementation detail. So the NOT_CONTROLLER error does not make sense for the client. That is why we do not return it. We decided instead that if the controller could not be reached before expiration of the timeout, then we would return REQUEST_TIMED_OUT instead. 
   
   That aside, it is not very clear to me why we need to parse the envelope response. The intent was to return NOT_CONTROLLER as the error code in the EnvelopeResponse itself. Perhaps there is a race condition in the controller handling where this is not working? Probably that is what we should fix.
   
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org