You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by da...@apache.org on 2021/07/15 12:46:06 UTC
[kafka] branch 3.0 updated: KAFKA-13062: Make
DeleteConsumerGroupsHandler unmap for COORDINATOR_NOT_AVAILABLE error
(#11021)
This is an automated email from the ASF dual-hosted git repository.
dajac pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/3.0 by this push:
new 15d596a KAFKA-13062: Make DeleteConsumerGroupsHandler unmap for COORDINATOR_NOT_AVAILABLE error (#11021)
15d596a is described below
commit 15d596a3f3db67f2146e400210302eb5566f3f5f
Author: Luke Chen <sh...@gmail.com>
AuthorDate: Thu Jul 15 20:40:21 2021 +0800
KAFKA-13062: Make DeleteConsumerGroupsHandler unmap for COORDINATOR_NOT_AVAILABLE error (#11021)
This patch improve the error handling in `DeleteConsumerGroupsHandler` and ensure that `COORDINATOR_NOT_AVAILABLE` is unmapped in order to look up the coordinator again.
Reviewers: David Jacot <dj...@confluent.io>
---
.../internals/DeleteConsumerGroupsHandler.java | 38 ++++++++++++--------
.../kafka/clients/admin/KafkaAdminClientTest.java | 40 +++++++++++++---------
.../internals/DeleteConsumerGroupsHandlerTest.java | 4 ++-
3 files changed, 49 insertions(+), 33 deletions(-)
diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandler.java b/clients/src/main/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandler.java
index c5d2205..2b42001 100644
--- a/clients/src/main/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandler.java
+++ b/clients/src/main/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandler.java
@@ -19,6 +19,7 @@ package org.apache.kafka.clients.admin.internals;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -86,47 +87,54 @@ public class DeleteConsumerGroupsHandler implements AdminApiHandler<CoordinatorK
Set<CoordinatorKey> groupIds,
AbstractResponse abstractResponse
) {
- DeleteGroupsResponse response = (DeleteGroupsResponse) abstractResponse;
- Map<CoordinatorKey, Void> completed = new HashMap<>();
- Map<CoordinatorKey, Throwable> failed = new HashMap<>();
- List<CoordinatorKey> unmapped = new ArrayList<>();
+ final DeleteGroupsResponse response = (DeleteGroupsResponse) abstractResponse;
+ final Map<CoordinatorKey, Void> completed = new HashMap<>();
+ final Map<CoordinatorKey, Throwable> failed = new HashMap<>();
+ final Set<CoordinatorKey> groupsToUnmap = new HashSet<>();
for (DeletableGroupResult deletedGroup : response.data().results()) {
CoordinatorKey groupIdKey = CoordinatorKey.byGroupId(deletedGroup.groupId());
Errors error = Errors.forCode(deletedGroup.errorCode());
if (error != Errors.NONE) {
- handleError(groupIdKey, error, failed, unmapped);
+ handleError(groupIdKey, error, failed, groupsToUnmap);
continue;
}
completed.put(groupIdKey, null);
}
- return new ApiResult<>(completed, failed, unmapped);
+
+ return new ApiResult<>(completed, failed, new ArrayList<>(groupsToUnmap));
}
private void handleError(
CoordinatorKey groupId,
Errors error,
Map<CoordinatorKey, Throwable> failed,
- List<CoordinatorKey> unmapped
+ Set<CoordinatorKey> groupsToUnmap
) {
switch (error) {
case GROUP_AUTHORIZATION_FAILED:
- log.error("Received authorization failure for group {} in `DeleteConsumerGroups` response", groupId,
- error.exception());
+ case INVALID_GROUP_ID:
+ case NON_EMPTY_GROUP:
+ case GROUP_ID_NOT_FOUND:
+ log.debug("`DeleteConsumerGroups` request for group id {} failed due to error {}", groupId.idValue, error);
failed.put(groupId, error.exception());
break;
case COORDINATOR_LOAD_IN_PROGRESS:
- case COORDINATOR_NOT_AVAILABLE:
+ // If the coordinator is in the middle of loading, then we just need to retry
+ log.debug("`DeleteConsumerGroups` request for group id {} failed because the coordinator " +
+ "is still in the process of loading state. Will retry", groupId.idValue);
break;
+ case COORDINATOR_NOT_AVAILABLE:
case NOT_COORDINATOR:
- log.debug("DeleteConsumerGroups request for group {} returned error {}. Will retry",
- groupId, error);
- unmapped.add(groupId);
+ // If the coordinator is unavailable or there was a coordinator change, then we unmap
+ // the key so that we retry the `FindCoordinator` request
+ log.debug("`DeleteConsumerGroups` request for group id {} returned error {}. " +
+ "Will attempt to find the coordinator again and retry", groupId.idValue, error);
+ groupsToUnmap.add(groupId);
break;
default:
- log.error("Received unexpected error for group {} in `DeleteConsumerGroups` response",
- groupId, error.exception());
+ log.error("`DeleteConsumerGroups` request for group id {} failed due to unexpected error {}", groupId.idValue, error);
failed.put(groupId, error.exception());
}
}
diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java b/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java
index ab11833..207b493 100644
--- a/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java
+++ b/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java
@@ -3182,41 +3182,47 @@ public class KafkaAdminClientTest {
env.kafkaClient().prepareResponse(
prepareOldFindCoordinatorResponse(Errors.GROUP_AUTHORIZATION_FAILED, Node.noNode()));
- final DeleteConsumerGroupsResult errorResult = env.adminClient().deleteConsumerGroups(groupIds);
+ DeleteConsumerGroupsResult errorResult = env.adminClient().deleteConsumerGroups(groupIds);
TestUtils.assertFutureError(errorResult.deletedGroups().get("groupId"), GroupAuthorizationException.class);
- //Retriable errors should be retried
+ // Retriable errors should be retried
env.kafkaClient().prepareResponse(
prepareOldFindCoordinatorResponse(Errors.NONE, env.cluster().controller()));
- final DeletableGroupResultCollection errorResponse1 = new DeletableGroupResultCollection();
- errorResponse1.add(new DeletableGroupResult()
- .setGroupId("groupId")
- .setErrorCode(Errors.COORDINATOR_NOT_AVAILABLE.code())
- );
- env.kafkaClient().prepareResponse(new DeleteGroupsResponse(
- new DeleteGroupsResponseData()
- .setResults(errorResponse1)));
-
- final DeletableGroupResultCollection errorResponse2 = new DeletableGroupResultCollection();
- errorResponse2.add(new DeletableGroupResult()
+ final DeletableGroupResultCollection errorResponse = new DeletableGroupResultCollection();
+ errorResponse.add(new DeletableGroupResult()
.setGroupId("groupId")
.setErrorCode(Errors.COORDINATOR_LOAD_IN_PROGRESS.code())
);
env.kafkaClient().prepareResponse(new DeleteGroupsResponse(
new DeleteGroupsResponseData()
- .setResults(errorResponse2)));
+ .setResults(errorResponse)));
/*
* We need to return two responses here, one for NOT_COORDINATOR call when calling delete a consumer group
* api using coordinator that has moved. This will retry whole operation. So we need to again respond with a
* FindCoordinatorResponse.
+ *
+ * And the same reason for the following COORDINATOR_NOT_AVAILABLE error response
*/
- final DeletableGroupResultCollection coordinatorMoved = new DeletableGroupResultCollection();
+
+ DeletableGroupResultCollection coordinatorMoved = new DeletableGroupResultCollection();
coordinatorMoved.add(new DeletableGroupResult()
.setGroupId("groupId")
.setErrorCode(Errors.NOT_COORDINATOR.code())
);
+
+ env.kafkaClient().prepareResponse(new DeleteGroupsResponse(
+ new DeleteGroupsResponseData()
+ .setResults(coordinatorMoved)));
+ env.kafkaClient().prepareResponse(prepareOldFindCoordinatorResponse(Errors.NONE, env.cluster().controller()));
+
+ coordinatorMoved = new DeletableGroupResultCollection();
+ coordinatorMoved.add(new DeletableGroupResult()
+ .setGroupId("groupId")
+ .setErrorCode(Errors.COORDINATOR_NOT_AVAILABLE.code())
+ );
+
env.kafkaClient().prepareResponse(new DeleteGroupsResponse(
new DeleteGroupsResponseData()
.setResults(coordinatorMoved)));
@@ -3226,9 +3232,9 @@ public class KafkaAdminClientTest {
new DeleteGroupsResponseData()
.setResults(validResponse)));
- final DeleteConsumerGroupsResult errorResult1 = env.adminClient().deleteConsumerGroups(groupIds);
+ errorResult = env.adminClient().deleteConsumerGroups(groupIds);
- final KafkaFuture<Void> errorResults = errorResult1.deletedGroups().get("groupId");
+ final KafkaFuture<Void> errorResults = errorResult.deletedGroups().get("groupId");
assertNull(errorResults.get());
}
}
diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandlerTest.java
index 30a65f4..8d3a237 100644
--- a/clients/src/test/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandlerTest.java
+++ b/clients/src/test/java/org/apache/kafka/clients/admin/internals/DeleteConsumerGroupsHandlerTest.java
@@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.kafka.common.Node;
import org.apache.kafka.common.errors.GroupAuthorizationException;
import org.apache.kafka.common.errors.GroupIdNotFoundException;
+import org.apache.kafka.common.errors.GroupNotEmptyException;
import org.apache.kafka.common.errors.InvalidGroupIdException;
import org.apache.kafka.common.message.DeleteGroupsResponseData;
import org.apache.kafka.common.message.DeleteGroupsResponseData.DeletableGroupResult;
@@ -57,12 +58,12 @@ public class DeleteConsumerGroupsHandlerTest {
@Test
public void testUnmappedHandleResponse() {
assertUnmapped(handleWithError(Errors.NOT_COORDINATOR));
+ assertUnmapped(handleWithError(Errors.COORDINATOR_NOT_AVAILABLE));
}
@Test
public void testRetriableHandleResponse() {
assertRetriable(handleWithError(Errors.COORDINATOR_LOAD_IN_PROGRESS));
- assertRetriable(handleWithError(Errors.COORDINATOR_NOT_AVAILABLE));
}
@Test
@@ -70,6 +71,7 @@ public class DeleteConsumerGroupsHandlerTest {
assertFailed(GroupAuthorizationException.class, handleWithError(Errors.GROUP_AUTHORIZATION_FAILED));
assertFailed(GroupIdNotFoundException.class, handleWithError(Errors.GROUP_ID_NOT_FOUND));
assertFailed(InvalidGroupIdException.class, handleWithError(Errors.INVALID_GROUP_ID));
+ assertFailed(GroupNotEmptyException.class, handleWithError(Errors.NON_EMPTY_GROUP));
}
private DeleteGroupsResponse buildResponse(Errors error) {