ijuma commented on a change in pull request #10157:
URL: https://github.com/apache/kafka/pull/10157#discussion_r579694847
##########
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:
Why do we treat authentication problems as retriable?
##########
File path: core/src/main/scala/kafka/server/AlterIsrManager.scala
##########
@@ -165,9 +165,19 @@ class DefaultAlterIsrManager(
new ControllerRequestCompletionHandler {
override def onComplete(response: ClientResponse): Unit = {
debug(s"Received AlterIsr response $response")
- val body = response.responseBody().asInstanceOf[AlterIsrResponse]
val error = try {
- handleAlterIsrResponse(body, message.brokerEpoch,
inflightAlterIsrItems)
+ if (response.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.
+ Errors.NETWORK_EXCEPTION
+ } else if (response.versionMismatch != null) {
Review comment:
Seems like it's pretty common t check for these two fields. Could we
have an enum to represent the two errors and then have a function from the enum
the Error? Seems a bit easier to use and less error prone.
----------------------------------------------------------------
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:
us...@infra.apache.org
