cmccabe commented on PR #12403: URL: https://github.com/apache/kafka/pull/12403#issuecomment-1188236978
So, we definitely need to get rid of the calls to *.get* and other cases of needless blocking. There are definitely some cases where we aren't properly handling errors as well (this is related I guess) in how we're chaining futures. The problem with returning CompletableFuture from everything is that there are things we do in the request handler threads that are blocking, such as anything the Authorizer does. I think it's kind of awkward to have half of the stuff blocking, and half non-blocking. I also see cases in this PR where we're returning a completed future, but the response hasn't been sent yet. Overall I'd prefer not to return futures from everything unless we can push this down to functions like `sendResponseMaybeThrottle`, which are currently callback-based rather than future-based. And the authorizer, of course, which is just blocking. I think mixing the two (three?) styles will lead to weird issues so we probably should just remove the calls to get and the error handling for now... -- 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]
