hachikuji commented on a change in pull request #10157:
URL: https://github.com/apache/kafka/pull/10157#discussion_r579707394
##########
File path: core/src/main/scala/kafka/raft/KafkaNetworkChannel.scala
##########
@@ -124,8 +124,14 @@ class KafkaNetworkChannel(
}
def onComplete(clientResponse: ClientResponse): Unit = {
- val response = if (clientResponse.authenticationException != null) {
- errorResponse(request.data, Errors.CLUSTER_AUTHORIZATION_FAILED)
+ val response = if (clientResponse.versionMismatch != null) {
+ errorResponse(request.data, Errors.UNSUPPORTED_VERSION)
+ } else if (clientResponse.authenticationException != null) {
+ // For now we treat authentication errors as retriable. We use the
+ // `NETWORK_EXCEPTION` error code for lack of a good alternative.
+ // Note that `BrokerToControllerChannelManager` will still log the
+ // authentication errors so that users have a chance to fix the
problem.
+ errorResponse(request.data, Errors.NETWORK_EXCEPTION)
Review comment:
I agree it is debatable. If we make it a fatal error, I think we would
do so under the expectation that the broker receiving the authentication
failure is the one misconfigured. However, that may or may not be true. If a
misconfigured broker tries to join the cluster, it could end up crashing all of
the other brokers through authentication failures. Perhaps what we want is for
authentication failures to be fatal during some initial window when the broker
is starting up and non-fatal afterwards. In any case, because we do not have a
good mechanism in place here to propagate fatal errors, it seemed simpler to
treat it as retriable.
----------------------------------------------------------------
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:
[email protected]
