artemlivshits commented on code in PR #15559:
URL: https://github.com/apache/kafka/pull/15559#discussion_r1534632389
##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -921,7 +921,10 @@ private[group] class GroupCoordinator(
): Unit = {
val (error, verificationGuard) = errorAndGuard
if (error != Errors.NONE) {
- val finalError =
GroupMetadataManager.maybeConvertOffsetCommitError(error)
+ val finalError = error match {
+ case Errors.NETWORK_EXCEPTION =>
Errors.COORDINATOR_LOAD_IN_PROGRESS
Review Comment:
Should we add a comment why we're translating to this error and not the
other?
##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -831,6 +831,7 @@ class ReplicaManager(val config: KafkaConfig,
error match {
case Errors.INVALID_TXN_STATE => Some(error.exception("Partition
was not added to the transaction"))
case Errors.CONCURRENT_TRANSACTIONS |
+ Errors.NETWORK_EXCEPTION |
Review Comment:
Should we add a comment why we're translating to NotEnoughReplicasException
error and not something else? I think there is a bit of thinking that went
into this logic and the code doesn't reflect it.
##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -921,7 +921,10 @@ private[group] class GroupCoordinator(
): Unit = {
val (error, verificationGuard) = errorAndGuard
if (error != Errors.NONE) {
- val finalError =
GroupMetadataManager.maybeConvertOffsetCommitError(error)
+ val finalError = error match {
+ case Errors.NETWORK_EXCEPTION =>
Errors.COORDINATOR_LOAD_IN_PROGRESS
Review Comment:
I'm curious why we singled out this error explicitly rather than putting it
into the function tasked specifically with error translation?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
