From 707d6647ee37be17c02895142c530a7aadc6f4eb Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 9 Apr 2018 17:10:42 +0100 Subject: [PATCH 01/22] KAFKA-5692: Change PreferredReplicaLeaderElectionCommand to use AdminClient --- .../kafka/clients/admin/AdminClient.java | 53 +++ .../admin/ElectPreferredLeadersOptions.java | 31 ++ .../admin/ElectPreferredLeadersResult.java | 130 +++++++ .../kafka/clients/admin/KafkaAdminClient.java | 30 ++ .../apache/kafka/common/protocol/ApiKeys.java | 6 +- .../common/requests/AbstractRequest.java | 2 + .../common/requests/AbstractResponse.java | 2 + .../ElectPreferredLeadersRequest.java | 160 ++++++++ .../ElectPreferredLeadersResponse.java | 126 +++++++ .../kafka/common/utils/CollectionUtils.java | 2 +- .../clients/admin/KafkaAdminClientTest.java | 38 ++ .../kafka/clients/admin/MockAdminClient.java | 4 + .../common/requests/RequestResponseTest.java | 22 ++ ...referredReplicaLeaderElectionCommand.scala | 202 ++++++++++- .../main/scala/kafka/cluster/Partition.scala | 1 + .../kafka/controller/KafkaController.scala | 91 +++-- .../controller/PartitionStateMachine.scala | 44 ++- .../server/DelayedElectPreferredLeader.scala | 90 +++++ .../main/scala/kafka/server/KafkaApis.scala | 26 ++ .../scala/kafka/server/ReplicaManager.scala | 44 +++ .../api/AdminClientIntegrationTest.scala | 191 +++++++++- .../kafka/api/AuthorizerIntegrationTest.scala | 19 +- ...rredReplicaLeaderElectionCommandTest.scala | 341 ++++++++++++++++++ .../AbstractCoordinatorConcurrencyTest.scala | 2 +- .../kafka/server/ReplicaManagerTest.scala | 8 +- .../unit/kafka/server/RequestQuotaTest.scala | 4 + 26 files changed, 1611 insertions(+), 58 deletions(-) create mode 100644 clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersOptions.java create mode 100644 clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java create mode 100644 clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java create mode 100644 clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java create mode 100644 core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala create mode 100644 core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java index bdd7cc36169c4..8504cba6fad88 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java @@ -792,8 +792,61 @@ public DeleteConsumerGroupsResult deleteConsumerGroups(Collection groupI return deleteConsumerGroups(groupIds, new DeleteConsumerGroupsOptions()); } + /** + * Elect the preferred broker of the given {@code partitions} as leader, or + * elect the preferred broker for all partitions as leader if the argument to {@code partitions} is null. + * + * This is a convenience method for {@link #electPreferredLeaders(Collection, ElectPreferredLeadersOptions)} + * with default options. + * See the overload for more details. + * + * @param partitions The partitions for which the the preferred leader should be elected. + * @return The ElectPreferredLeadersResult. + */ + public ElectPreferredLeadersResult electPreferredLeaders(Collection partitions) { + return electPreferredLeaders(partitions, new ElectPreferredLeadersOptions()); + } + + /** + * Elect the preferred broker of the given {@code partitions} as leader, or + * elect the preferred broker for all partitions as leader if the argument to {@code partitions} is null. + * + * This operation is not transactional so it may succeed for some partitions while fail for others. + * + * It may take several seconds after this method returns + * success for all the brokers in the cluster to become aware that the partitions have new leaders. + * During this time, {@link AdminClient#describeTopics(Collection)} + * may not return information about the partitions' new leaders. + * + * This operation is supported by brokers with version 1.1.0 or higher. + * + *

The following exceptions can be anticipated when calling {@code get()} on the futures obtained from + * the returned {@code ElectPreferredLeadersResult}:

+ *
    + *
  • {@link org.apache.kafka.common.errors.ClusterAuthorizationException} + * if the authenticated user didn't have alter access to the cluster.
    • + *
  • {@link org.apache.kafka.common.errors.UnknownTopicOrPartitionException} + * if the topic or partition did not exist within the cluster.
    • + *
  • {@link org.apache.kafka.common.errors.InvalidTopicException} + * if the topic was already queued for deletion.
    • + *
  • {@link org.apache.kafka.common.errors.NotControllerException} + * if the request was sent to a broker that was not the controller for the cluster.
    • + *
  • {@link org.apache.kafka.common.errors.TimeoutException} + * if the request timed out before the election was complete.
    • + *
  • {@link org.apache.kafka.common.errors.LeaderNotAvailableException} + * if the preferred leader was not alive or not in the ISR.
    • + *
+ * + * @param partitions The partitions for which the the preferred leader should be elected. + * @param options The options to use when electing the preferred leaders. + * @return The ElectPreferredLeadersResult. + */ + public abstract ElectPreferredLeadersResult electPreferredLeaders(Collection partitions, + ElectPreferredLeadersOptions options); + /** * Get the metrics kept by the adminClient */ public abstract Map metrics(); + } diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersOptions.java b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersOptions.java new file mode 100644 index 0000000000000..80b00975f7da9 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersOptions.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.clients.admin; + +import org.apache.kafka.common.annotation.InterfaceStability; + +import java.util.Collection; + +/** + * Options for {@link AdminClient#electPreferredLeaders(Collection, ElectPreferredLeadersOptions)}. + * + * The API of this class is evolving, see {@link AdminClient} for details. + */ +@InterfaceStability.Evolving +public class ElectPreferredLeadersOptions extends AbstractOptions { +} diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java new file mode 100644 index 0000000000000..14ebfb01df98e --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java @@ -0,0 +1,130 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.clients.admin; + +import org.apache.kafka.common.KafkaFuture; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.annotation.InterfaceStability; +import org.apache.kafka.common.errors.ApiException; +import org.apache.kafka.common.errors.UnknownTopicOrPartitionException; +import org.apache.kafka.common.internals.KafkaFutureImpl; +import org.apache.kafka.common.requests.ApiError; + +import java.util.Collection; +import java.util.Map; +import java.util.Set; + +/** + * The result of {@link AdminClient#electPreferredLeaders(Collection, ElectPreferredLeadersOptions)} + * + * The API of this class is evolving, see {@link AdminClient} for details. + */ +@InterfaceStability.Evolving +public class ElectPreferredLeadersResult { + + private final KafkaFutureImpl> electionFuture; + private final Set partitions; + + ElectPreferredLeadersResult(KafkaFutureImpl> electionFuture, Set partitions) { + this.electionFuture = electionFuture; + this.partitions = partitions; + } + + /** + * Get the result of the election for the given {@code partition}. + * If there was not an election triggered for the given {@code partition}, the + * returned future will complete with an error. + */ + public KafkaFuture partitionResult(final TopicPartition partition) { + final KafkaFutureImpl result = new KafkaFutureImpl<>(); + electionFuture.whenComplete(new KafkaFuture.BiConsumer, Throwable>() { + @Override + public void accept(Map topicPartitions, Throwable throwable) { + if (throwable != null) { + result.completeExceptionally(throwable); + } else if (!topicPartitions.containsKey(partition)) { + result.completeExceptionally(new UnknownTopicOrPartitionException( + "Preferred leader election for partition \"" + partition + + "\" was not attempted")); + } else { + ApiException exception = topicPartitions.get(partition).exception(); + if (exception == null) { + result.complete(null); + } else { + result.completeExceptionally(exception); + } + } + } + }); + return result; + } + + /** + *

Get a future for the topic partitions for which a leader election + * was attempted. A partition will be present in this result if + * an election was attempted even if the election was not successful.

+ * + *

This method is provided to discover the partitions attempted when + * {@link AdminClient#electPreferredLeaders(Collection)} is called + * with a null {@code partitions} argument.

+ */ + public KafkaFuture> partitions() { + if (partitions != null) { + return KafkaFutureImpl.completedFuture(this.partitions); + } else { + final KafkaFutureImpl> result = new KafkaFutureImpl<>(); + electionFuture.whenComplete(new KafkaFuture.BiConsumer, Throwable>() { + @Override + public void accept(Map topicPartitions, Throwable throwable) { + if (throwable != null) { + result.completeExceptionally(throwable); + } else { + for (ApiError apiError : topicPartitions.values()) { + if (apiError.isFailure()) { + result.completeExceptionally(apiError.exception()); + } + } + result.complete(topicPartitions.keySet()); + } + } + }); + return result; + } + } + + /** + * Return a future which succeeds if all the topic elections succeed. + */ + public KafkaFuture all() { + final KafkaFutureImpl result = new KafkaFutureImpl<>(); + electionFuture.thenApply(new KafkaFuture.Function, Void>() { + @Override + public Void apply(Map topicPartitions) { + for (ApiError apiError : topicPartitions.values()) { + if (apiError.isFailure()) { + result.completeExceptionally(apiError.exception()); + return null; + } + } + result.complete(null); + return null; + } + }); + return result; + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java index 2ba3cf225b406..9198dd995408a 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java @@ -111,6 +111,8 @@ import org.apache.kafka.common.requests.FindCoordinatorResponse; import org.apache.kafka.common.requests.ListGroupsRequest; import org.apache.kafka.common.requests.ListGroupsResponse; +import org.apache.kafka.common.requests.ElectPreferredLeadersRequest; +import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.MetadataRequest; import org.apache.kafka.common.requests.MetadataResponse; import org.apache.kafka.common.requests.OffsetFetchRequest; @@ -2777,4 +2779,32 @@ void handleFailure(Throwable throwable) { public Map metrics() { return Collections.unmodifiableMap(this.metrics.metrics()); } + + @Override + public ElectPreferredLeadersResult electPreferredLeaders(final Collection partitions, + ElectPreferredLeadersOptions options) { + final Set partitionSet = partitions != null ? new HashSet<>(partitions) : null; + final KafkaFutureImpl> electionFuture = new KafkaFutureImpl<>(); + final long now = time.milliseconds(); + runnable.call(new Call("electPreferredLeaders", calcDeadlineMs(now, options.timeoutMs()), + new ControllerNodeProvider()) { + + @Override + public AbstractRequest.Builder createRequest(int timeoutMs) { + return new ElectPreferredLeadersRequest.Builder(partitions, timeoutMs); + } + + @Override + public void handleResponse(AbstractResponse abstractResponse) { + ElectPreferredLeadersResponse response = (ElectPreferredLeadersResponse) abstractResponse; + electionFuture.complete(response.errors()); + } + + @Override + void handleFailure(Throwable throwable) { + electionFuture.completeExceptionally(throwable); + } + }, now); + return new ElectPreferredLeadersResult(electionFuture, partitionSet); + } } diff --git a/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java b/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java index 3d7710025ba48..ca6146bb79429 100644 --- a/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java +++ b/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java @@ -59,6 +59,8 @@ import org.apache.kafka.common.requests.DescribeLogDirsResponse; import org.apache.kafka.common.requests.DescribeDelegationTokenRequest; import org.apache.kafka.common.requests.DescribeDelegationTokenResponse; +import org.apache.kafka.common.requests.ElectPreferredLeadersRequest; +import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.EndTxnRequest; import org.apache.kafka.common.requests.EndTxnResponse; import org.apache.kafka.common.requests.ExpireDelegationTokenRequest; @@ -186,7 +188,9 @@ public Struct parseResponse(short version, ByteBuffer buffer) { RENEW_DELEGATION_TOKEN(39, "RenewDelegationToken", RenewDelegationTokenRequest.schemaVersions(), RenewDelegationTokenResponse.schemaVersions()), EXPIRE_DELEGATION_TOKEN(40, "ExpireDelegationToken", ExpireDelegationTokenRequest.schemaVersions(), ExpireDelegationTokenResponse.schemaVersions()), DESCRIBE_DELEGATION_TOKEN(41, "DescribeDelegationToken", DescribeDelegationTokenRequest.schemaVersions(), DescribeDelegationTokenResponse.schemaVersions()), - DELETE_GROUPS(42, "DeleteGroups", DeleteGroupsRequest.schemaVersions(), DeleteGroupsResponse.schemaVersions()); + DELETE_GROUPS(42, "DeleteGroups", DeleteGroupsRequest.schemaVersions(), DeleteGroupsResponse.schemaVersions()), + ELECT_PREFERRED_LEADERS(43, "ElectPreferredLeaders", ElectPreferredLeadersRequest.schemaVersions(), + ElectPreferredLeadersResponse.schemaVersions()); private static final ApiKeys[] ID_TO_TYPE; private static final int MIN_API_KEY = 0; diff --git a/clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java index d16e60f84db5d..239024f86327c 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java @@ -227,6 +227,8 @@ public static AbstractRequest parseRequest(ApiKeys apiKey, short apiVersion, Str return new DescribeDelegationTokenRequest(struct, apiVersion); case DELETE_GROUPS: return new DeleteGroupsRequest(struct, apiVersion); + case ELECT_PREFERRED_LEADERS: + return new ElectPreferredLeadersRequest(struct, apiVersion); default: throw new AssertionError(String.format("ApiKey %s is not currently handled in `parseRequest`, the " + "code should be updated to do so.", apiKey)); diff --git a/clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java b/clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java index c0ebef1d967c6..9980b205afda3 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java @@ -156,6 +156,8 @@ public static AbstractResponse parseResponse(ApiKeys apiKey, Struct struct) { return new DescribeDelegationTokenResponse(struct); case DELETE_GROUPS: return new DeleteGroupsResponse(struct); + case ELECT_PREFERRED_LEADERS: + return new ElectPreferredLeadersResponse(struct); default: throw new AssertionError(String.format("ApiKey %s is not currently handled in `parseResponse`, the " + "code should be updated to do so.", apiKey)); diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java new file mode 100644 index 0000000000000..fc42943877445 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.common.requests; + +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.protocol.ApiKeys; +import org.apache.kafka.common.protocol.types.ArrayOf; +import org.apache.kafka.common.protocol.types.Field; +import org.apache.kafka.common.protocol.types.Schema; +import org.apache.kafka.common.protocol.types.Struct; +import org.apache.kafka.common.utils.CollectionUtils; + +import java.nio.ByteBuffer; +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; + +import static org.apache.kafka.common.protocol.CommonFields.TOPIC_NAME; +import static org.apache.kafka.common.protocol.types.Type.INT32; + +public class ElectPreferredLeadersRequest extends AbstractRequest { + + private static final String TIMEOUT_KEY_NAME = "timeout"; + private static final String TOPIC_PARTITIONS_KEY_NAME = "topic_partitions"; + private static final String PARTITIONS_KEY_NAME = "partitions"; + + public static final Schema ELECT_PREFERRED_LEADERS_REQUEST_V0 = new Schema( + new Field(TOPIC_PARTITIONS_KEY_NAME, ArrayOf.nullable( + new Schema( + TOPIC_NAME, + new Field(PARTITIONS_KEY_NAME, + new ArrayOf(INT32), + "The partitions of this topic whose preferred leader should be elected")))), + new Field(TIMEOUT_KEY_NAME, INT32, "The time in ms to wait for the elections to be completed.") + ); + + public static Schema[] schemaVersions() { + return new Schema[]{ELECT_PREFERRED_LEADERS_REQUEST_V0}; + } + + public static class Builder extends AbstractRequest.Builder { + + private final Set topicPartitions; + private final int timeout; + + public Builder(Collection topicPartitions, int timeout) { + super(ApiKeys.ELECT_PREFERRED_LEADERS); + this.topicPartitions = topicPartitions != null ? new HashSet<>(topicPartitions) : null; + this.timeout = timeout; + } + + @Override + public ElectPreferredLeadersRequest build(short version) { + return new ElectPreferredLeadersRequest(version, topicPartitions, timeout); + } + } + + private final Set topicPartitions; + private final int timeout; + + public ElectPreferredLeadersRequest(short version, Set topicPartitions, int timeout) { + super(ApiKeys.ELECT_PREFERRED_LEADERS, version); + this.topicPartitions = topicPartitions; + this.timeout = timeout; + } + + public ElectPreferredLeadersRequest(Struct struct, short version) { + super(ApiKeys.ELECT_PREFERRED_LEADERS, version); + Object[] topicPartitionsArray = struct.getArray(TOPIC_PARTITIONS_KEY_NAME); + if (topicPartitionsArray != null) { + topicPartitions = new HashSet<>(topicPartitionsArray.length); + for (Object topicPartitionObj : topicPartitionsArray) { + Struct topicPartitionStruct = (Struct) topicPartitionObj; + String topicName = topicPartitionStruct.get(TOPIC_NAME); + Object[] partitionsArray = topicPartitionStruct.getArray(PARTITIONS_KEY_NAME); + if (partitionsArray != null) { + for (Object partitionObj : partitionsArray) { + Integer partition = (Integer) partitionObj; + TopicPartition topicPartition = new TopicPartition(topicName, partition); + topicPartitions.add(topicPartition); + } + } + } + } else { + topicPartitions = null; + } + timeout = struct.getInt(TIMEOUT_KEY_NAME); + } + + public Set topicPartitions() { + return topicPartitions; + } + + public int timeout() { + return timeout; + } + + @Override + protected Struct toStruct() { + Struct struct = new Struct(ApiKeys.ELECT_PREFERRED_LEADERS.requestSchema(version())); + Struct[] topicPartitionsArray; + if (topicPartitions != null) { + Map> map = CollectionUtils.groupPartitionsByTopic(topicPartitions); + List topicPartitionsList = new ArrayList<>(topicPartitions.size()); + for (Map.Entry> entry : map.entrySet()) { + Struct partitionStruct = struct.instance(TOPIC_PARTITIONS_KEY_NAME); + partitionStruct.set(TOPIC_NAME, entry.getKey()); + partitionStruct.set(PARTITIONS_KEY_NAME, entry.getValue().toArray()); + topicPartitionsList.add(partitionStruct); + } + topicPartitionsArray = topicPartitionsList.toArray(new Struct[topicPartitionsList.size()]); + } else { + topicPartitionsArray = null; + } + struct.set(TOPIC_PARTITIONS_KEY_NAME, topicPartitionsArray); + struct.set(TIMEOUT_KEY_NAME, timeout); + return struct; + } + + @Override + public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) { + short version = version(); + switch (version) { + case 0: + ApiError error = ApiError.fromThrowable(e); + Map errors = new HashMap<>(topicPartitions.size()); + for (TopicPartition partition : topicPartitions) + errors.put(partition, error); + return new ElectPreferredLeadersResponse(throttleTimeMs, errors); + default: + throw new IllegalArgumentException(String.format( + "Version %d is not valid. Valid versions for %s are 0 to %d", + version, this.getClass().getSimpleName(), ApiKeys.ELECT_PREFERRED_LEADERS.latestVersion())); + } + } + + public static ElectPreferredLeadersRequest parse(ByteBuffer buffer, short version) { + return new ElectPreferredLeadersRequest(ApiKeys.ELECT_PREFERRED_LEADERS.parseRequest(version, buffer), version); + } + +} diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java new file mode 100644 index 0000000000000..d8833ba92efcd --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java @@ -0,0 +1,126 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.common.requests; + +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.protocol.ApiKeys; +import org.apache.kafka.common.protocol.Errors; +import org.apache.kafka.common.protocol.types.ArrayOf; +import org.apache.kafka.common.protocol.types.Field; +import org.apache.kafka.common.protocol.types.Schema; +import org.apache.kafka.common.protocol.types.Struct; +import org.apache.kafka.common.utils.CollectionUtils; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.apache.kafka.common.protocol.CommonFields.ERROR_CODE; +import static org.apache.kafka.common.protocol.CommonFields.ERROR_MESSAGE; +import static org.apache.kafka.common.protocol.CommonFields.PARTITION_ID; +import static org.apache.kafka.common.protocol.CommonFields.THROTTLE_TIME_MS; +import static org.apache.kafka.common.protocol.CommonFields.TOPIC_NAME; + +public class ElectPreferredLeadersResponse extends AbstractResponse { + + private static final String REPLICA_ELECTION_RESULT_KEY_NAME = "replica_election_result"; + private static final String PARTITION_RESULTS_KEY_NAME = "partition_results"; + + public static final Schema ELECT_PREFERRED_LEADERS_RESPONSE_V0 = new Schema( + THROTTLE_TIME_MS, + new Field(REPLICA_ELECTION_RESULT_KEY_NAME, new ArrayOf(new Schema( + TOPIC_NAME, + new Field(PARTITION_RESULTS_KEY_NAME, new ArrayOf( + new Schema( + PARTITION_ID, + ERROR_CODE, + ERROR_MESSAGE)), + "The results for each partition"))))); + + public static Schema[] schemaVersions() { + return new Schema[]{ELECT_PREFERRED_LEADERS_RESPONSE_V0}; + } + + private final int throttleTimeMs; + private final Map errors; + + public ElectPreferredLeadersResponse(int throttleTimeMs, Map errors) { + this.throttleTimeMs = throttleTimeMs; + this.errors = errors; + } + + public ElectPreferredLeadersResponse(Struct struct) { + throttleTimeMs = struct.get(THROTTLE_TIME_MS); + Object[] resourcesArray = struct.getArray(REPLICA_ELECTION_RESULT_KEY_NAME); + errors = new HashMap<>(resourcesArray.length); + for (Object partitionObj : resourcesArray) { + Struct topicStruct = (Struct) partitionObj; + String topicName = topicStruct.get(TOPIC_NAME); + Object[] partitionResults = topicStruct.getArray(PARTITION_RESULTS_KEY_NAME); + for (Object partitionResult : partitionResults) { + Struct partitionStruct = (Struct) partitionResult; + int partitionId = partitionStruct.get(PARTITION_ID); + ApiError error = new ApiError(partitionStruct); + errors.put(new TopicPartition(topicName, partitionId), error); + } + } + } + + public Map errors() { + return errors; + } + + public int throttleTimeMs() { + return throttleTimeMs; + } + + @Override + public Map errorCounts() { + return apiErrorCounts(errors); + } + + @Override + protected Struct toStruct(short version) { + Struct struct = new Struct(ApiKeys.ELECT_PREFERRED_LEADERS.responseSchema(version)); + struct.set(THROTTLE_TIME_MS, throttleTimeMs); + Map> groupedErrors = CollectionUtils.groupPartitionDataByTopic(errors); + List replicaElectionResultList = new ArrayList<>(errors.size()); + for (Map.Entry> topicToErrors : groupedErrors.entrySet()) { + Struct topicStruct = struct.instance(REPLICA_ELECTION_RESULT_KEY_NAME); + topicStruct.set(TOPIC_NAME, topicToErrors.getKey()); + List partitionResultList = new ArrayList<>(topicToErrors.getValue().size()); + for (Map.Entry partitionToError : topicToErrors.getValue().entrySet()) { + Struct partitionResultStruct = topicStruct.instance(PARTITION_RESULTS_KEY_NAME); + partitionResultStruct.set(PARTITION_ID, partitionToError.getKey()); + partitionToError.getValue().write(partitionResultStruct); + partitionResultList.add(partitionResultStruct); + } + topicStruct.set(PARTITION_RESULTS_KEY_NAME, partitionResultList.toArray()); + replicaElectionResultList.add(topicStruct); + } + struct.set(REPLICA_ELECTION_RESULT_KEY_NAME, replicaElectionResultList.toArray(new Struct[0])); + return struct; + } + + public static ElectPreferredLeadersResponse parse(ByteBuffer buffer, short version) { + return new ElectPreferredLeadersResponse(ApiKeys.ELECT_PREFERRED_LEADERS.parseResponse(version, buffer)); + } + +} diff --git a/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java b/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java index f3a7dbd52bf08..a1df4276103fc 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java @@ -19,10 +19,10 @@ import org.apache.kafka.common.TopicPartition; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Collection; import java.util.stream.Collectors; public final class CollectionUtils { 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 82c5b1d81da01..210c7e20d128c 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 @@ -40,6 +40,7 @@ import org.apache.kafka.common.errors.AuthenticationException; import org.apache.kafka.common.errors.GroupAuthorizationException; import org.apache.kafka.common.errors.InvalidRequestException; +import org.apache.kafka.common.errors.ClusterAuthorizationException; import org.apache.kafka.common.errors.InvalidTopicException; import org.apache.kafka.common.errors.LeaderNotAvailableException; import org.apache.kafka.common.errors.NotLeaderForPartitionException; @@ -70,6 +71,7 @@ import org.apache.kafka.common.requests.FindCoordinatorResponse; import org.apache.kafka.common.requests.ListGroupsResponse; import org.apache.kafka.common.requests.MetadataRequest; +import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.MetadataResponse; import org.apache.kafka.common.requests.OffsetFetchResponse; import org.apache.kafka.common.resource.PatternType; @@ -634,6 +636,42 @@ public void testDeleteAcls() throws Exception { } } + @Test + public void testElectPreferredLeaders() throws Exception { + TopicPartition topic1 = new TopicPartition("topic", 0); + TopicPartition topic2 = new TopicPartition("topic", 2); + try (AdminClientUnitTestEnv env = mockClientEnv()) { + env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); + //env.kafkaClient().prepareMetadataUpdate(env.cluster(), Collections.emptySet()); + + // Test a call where one partition has an error. + HashMap map = new HashMap<>(); + map.put(topic1, ApiError.NONE); + map.put(topic2, ApiError.fromThrowable(new ClusterAuthorizationException(null))); + env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(0, + map)); + ElectPreferredLeadersResult results = env.adminClient().electPreferredLeaders(asList(topic1, topic2)); + results.partitionResult(topic1).get(); + TestUtils.assertFutureError(results.partitionResult(topic2), ClusterAuthorizationException.class); + TestUtils.assertFutureError(results.all(), ClusterAuthorizationException.class); + + // Test a call where there are no errors. + map.put(topic1, ApiError.NONE); + map.put(topic2, ApiError.NONE); + env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(0, + map)); + + results = env.adminClient().electPreferredLeaders(asList(topic1, topic2)); + results.partitionResult(topic1).get(); + results.partitionResult(topic2).get(); + + // Now try a timeout + results = env.adminClient().electPreferredLeaders(asList(topic1, topic2), new ElectPreferredLeadersOptions().timeoutMs(2000)); + TestUtils.assertFutureError(results.partitionResult(topic1), TimeoutException.class); + TestUtils.assertFutureError(results.partitionResult(topic2), TimeoutException.class); + } + } + /** * Test handling timeouts. */ diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java b/clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java index aa2e68351373d..d721245be854e 100644 --- a/clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java +++ b/clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java @@ -327,6 +327,10 @@ public DeleteConsumerGroupsResult deleteConsumerGroups(Collection groupI throw new UnsupportedOperationException("Not implemented yet"); } + public ElectPreferredLeadersResult electPreferredLeaders(Collection partitions, ElectPreferredLeadersOptions options) { + throw new UnsupportedOperationException("Not implemented yet"); + } + @Override public CreateAclsResult createAcls(Collection acls, CreateAclsOptions options) { throw new UnsupportedOperationException("Not implemented yet"); diff --git a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java index aa980311fc9f2..5cb8335d5f5ff 100644 --- a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java +++ b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java @@ -304,6 +304,10 @@ public void testSerialization() throws Exception { checkRequest(createRenewTokenRequest()); checkErrorResponse(createRenewTokenRequest(), new UnknownServerException()); checkResponse(createRenewTokenResponse(), 0); + checkRequest(createElectPreferredLeadersRequest()); + checkRequest(createElectPreferredLeadersRequestNullPartitions()); + checkErrorResponse(createElectPreferredLeadersRequest(), new UnknownServerException()); + checkResponse(createElectPreferredLeadersResponse(), 0); } @Test @@ -1322,4 +1326,22 @@ private DescribeDelegationTokenResponse createDescribeTokenResponse() { return new DescribeDelegationTokenResponse(20, Errors.NONE, tokenList); } + + private ElectPreferredLeadersRequest createElectPreferredLeadersRequestNullPartitions() { + return new ElectPreferredLeadersRequest.Builder(null, 100).build((short) 0); + } + + private ElectPreferredLeadersRequest createElectPreferredLeadersRequest() { + return new ElectPreferredLeadersRequest.Builder(asList( + new TopicPartition("my_topic", 1), + new TopicPartition("my_topic", 2)), 100).build((short) 0); + } + + private ElectPreferredLeadersResponse createElectPreferredLeadersResponse() { + Map errors = new HashMap<>(); + errors.put(new TopicPartition("my_topic", 0), ApiError.NONE); + errors.put(new TopicPartition("my_topic", 1), ApiError.fromThrowable(Errors.UNKNOWN_TOPIC_OR_PARTITION.exception("blah"))); + return new ElectPreferredLeadersResponse(200, errors); + } + } diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 7bfecde335d03..86924d49e9388 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -16,12 +16,20 @@ */ package kafka.admin +import java.util.Properties +import java.util.concurrent.ExecutionException + import kafka.common.AdminCommandFailedException import kafka.utils._ import kafka.zk.KafkaZkClient -import org.apache.kafka.common.TopicPartition -import org.apache.kafka.common.security.JaasUtils +import org.apache.kafka.clients.admin.AdminClientConfig +import org.apache.kafka.common.errors.TimeoutException + +import collection.JavaConverters._ import org.apache.kafka.common.utils.{Time, Utils} +import org.apache.kafka.common.security.JaasUtils +import org.apache.kafka.common.{KafkaFuture, TopicPartition} + import org.apache.zookeeper.KeeperException.NodeExistsException import collection._ @@ -29,33 +37,47 @@ import collection._ object PreferredReplicaLeaderElectionCommand extends Logging { def main(args: Array[String]): Unit = { + + val timeout = 30000 + run(args, timeout) + } + def run(args: Array[String], timeout: Int = 30000): Unit = { val commandOpts = new PreferredReplicaLeaderElectionCommandOptions(args) CommandLineUtils.printHelpAndExitIfNeeded(commandOpts, "This tool helps to causes leadership for each partition to be transferred back to the 'preferred replica'," + " it can be used to balance leadership among the servers.") + if(args.length == 0) + CommandLineUtils.printUsageAndDie(commandOpts.parser, "This tool causes leadership for each partition to be transferred back to the 'preferred replica'," + + " it can be used to balance leadership among the servers." + + " The preferred replica is the first one in the replica assignment (see kafka-reassign-partitions.sh)." + + " Using this command is not necessary when the broker is configured with \"auto.leader.rebalance.enable=true\".") + CommandLineUtils.checkRequiredArgs(commandOpts.parser, commandOpts.options, commandOpts.zkConnectOpt) - val zkConnect = commandOpts.options.valueOf(commandOpts.zkConnectOpt) - var zkClient: KafkaZkClient = null - try { - val time = Time.SYSTEM - zkClient = KafkaZkClient(zkConnect, JaasUtils.isZkSecurityEnabled, 30000, 30000, Int.MaxValue, time) + if (commandOpts.options.has(commandOpts.bootstrapServerOpt) == commandOpts.options.has(commandOpts.zkConnectOpt)) { + CommandLineUtils.printUsageAndDie(commandOpts.parser, s"Exactly one of '${commandOpts.bootstrapServerOpt}' or '${commandOpts.zkConnectOpt}' must be provided") + } - val partitionsForPreferredReplicaElection = - if (!commandOpts.options.has(commandOpts.jsonFileOpt)) - zkClient.getAllPartitions() - else - parsePreferredReplicaElectionData(Utils.readFileAsString(commandOpts.options.valueOf(commandOpts.jsonFileOpt))) - val preferredReplicaElectionCommand = new PreferredReplicaLeaderElectionCommand(zkClient, partitionsForPreferredReplicaElection) + val partitionsForPreferredReplicaElection = + if (commandOpts.options.has(commandOpts.jsonFileOpt)) + Some(parsePreferredReplicaElectionData(Utils.readFileAsString(commandOpts.options.valueOf(commandOpts.jsonFileOpt)))) + else + None - preferredReplicaElectionCommand.moveLeaderToPreferredReplica() - } catch { - case e: Throwable => - println("Failed to start preferred replica election") - println(Utils.stackTrace(e)) + val preferredReplicaElectionCommand = if (commandOpts.options.has(commandOpts.zkConnectOpt)) { + Console.err.println(s"$commandOpts.zkConnectOpt is deprecated and will be removed in a future version of Kafka.") + Console.err.println(s"Use $commandOpts.bootstrapServerOpt instead to specify a broker to connect to.") + new ZkCommand(commandOpts.options.valueOf(commandOpts.zkConnectOpt), + JaasUtils.isZkSecurityEnabled, + timeout) + } else { + new AdminClientCommand(commandOpts.options.valueOf(commandOpts.bootstrapServerOpt), timeout) + } + + try { + preferredReplicaElectionCommand.electPreferredLeaders(partitionsForPreferredReplicaElection) } finally { - if (zkClient != null) - zkClient.close() + preferredReplicaElectionCommand.close() } } @@ -106,9 +128,149 @@ object PreferredReplicaLeaderElectionCommand extends Logging { .withRequiredArg .describedAs("urls") .ofType(classOf[String]) + val bootstrapServerOpt = parser.accepts("bootstrap-server", "A hostname and port for the broker to connect to, " + + "in the form host:port. Multiple comma-separated URLs can be given. REQUIRED unless --zookeeper is given.") + .withRequiredArg + .describedAs("host:port") + .ofType(classOf[String]) options = parser.parse(args: _*) } + + /** Abstraction over different ways to perform a leader election */ + trait Command { + /** Elect the preferred leader for the given {@code partitionsForElection}. + * If the given {@code partitionsForElection} are None then elect the preferred leader for all partitions. + */ + def electPreferredLeaders(partitionsForElection: Option[Set[TopicPartition]]) : Unit + def close() : Unit + } + + class ZkCommand(zkConnect: String, isSecure: Boolean, timeout: Int) + extends Command { + //val zkConnect = options.valueOf(zkConnectOpt) + var zkClient: KafkaZkClient = null + + val time = Time.SYSTEM + zkClient = KafkaZkClient(zkConnect, isSecure, timeout, timeout, Int.MaxValue, time) + + override def electPreferredLeaders(partitionsFromUser: Option[Set[TopicPartition]]) { + try { + val topics = + partitionsFromUser match { + case Some(partitions) => + partitions.map(_.topic).toSet + case None => + zkClient.getAllPartitions().map(_.topic) + } + + val partitionsFromZk = zkClient.getPartitionsForTopics(topics).flatMap{ case (topic, partitions) => + partitions.map(new TopicPartition(topic, _)) + }.toSet + + val (validPartitions, invalidPartitions) = + partitionsFromUser match { + case Some(partitions) => + partitions.partition(partitionsFromZk.contains) + case None => + (zkClient.getAllPartitions(), Set.empty) + } + PreferredReplicaLeaderElectionCommand.writePreferredReplicaElectionData(zkClient, validPartitions) + + println("Successfully started preferred replica election for partitions %s".format(validPartitions)) + invalidPartitions.foreach(p => println("Skipping preferred replica leader election for partition %s since it doesn't exist.".format(p))) + } catch { + case e: Throwable => throw new AdminCommandFailedException("Admin command failed", e) + } + } + + override def close(): Unit = { + if (zkClient != null) + zkClient.close() + } + } + + /** Election via AdminClient.electPreferredLeaders() */ + class AdminClientCommand(bootstrapServers: String, timeout: Int) + extends Command with Logging { + val props = new Properties() + props.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, bootstrapServers) + props.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) + + val adminClient = org.apache.kafka.clients.admin.AdminClient.create(props) + + /** + * Wait until the given future has completed, then return whether it completed exceptionally. + * Because KafkaFuture.isCompletedExceptionally doesn't wait for a result + */ + private def completedExceptionally[T](future: KafkaFuture[T]): Boolean = { + try { + future.get() + false + } catch { + case (_: Throwable) => + true + } + } + + override def electPreferredLeaders(partitionsFromUser: Option[Set[TopicPartition]]): Unit = { + val partitions = partitionsFromUser match { + case Some(partitionsFromUser) => partitionsFromUser.asJava + case None => null + } + debug(s"Calling AdminClient.electPreferredLeaders($partitions)") + val result = adminClient.electPreferredLeaders(partitions) + // wait for all results + + val attemptedPartitions = partitionsFromUser match { + case Some(partitionsFromUser) => partitions.asScala + case None => try { + result.partitions().get.asScala + } catch { + case e: ExecutionException => + val cause = e.getCause + if (cause.isInstanceOf[TimeoutException]) { + // We timed out, or don't even know the attempted partitions + println("Timeout waiting for election results") + } + throw new AdminCommandFailedException(null, cause) + case e: Throwable => + // We don't even know the attempted partitions + println("Error while making request") + e.printStackTrace() + return + } + } + + val (exceptional, ok) = attemptedPartitions.map(tp => tp -> result.partitionResult(tp)). + partition { case (_, partitionResult) => completedExceptionally(partitionResult) } + + if (!ok.isEmpty) { + println(s"Successfully completed preferred replica election for partitions ${ok.map(_._1).mkString(", ")}") + } + if (!exceptional.isEmpty) { + val adminException = new AdminCommandFailedException( + s"${exceptional.size} preferred replica(s) could not be elected") + for ((partition, void) <- exceptional) { + val exception = try { + void.get() + new AdminCommandFailedException("Exceptional future with no exception") + } catch { + case e: ExecutionException => e.getCause + } + println(s"Error completing preferred replica election for partition $partition: $exception") + adminException.addSuppressed(exception) + } + throw adminException + } + + } + + override def close(): Unit = { + debug("Closing AdminClient") + adminClient.close() + } + } } class PreferredReplicaLeaderElectionCommand(zkClient: KafkaZkClient, partitionsFromUser: scala.collection.Set[TopicPartition]) { diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala index ca3abbbc97348..9f0bdb96ef498 100755 --- a/core/src/main/scala/kafka/cluster/Partition.scala +++ b/core/src/main/scala/kafka/cluster/Partition.scala @@ -627,6 +627,7 @@ class Partition(val topicPartition: TopicPartition, replicaManager.tryCompleteDelayedFetch(requestKey) replicaManager.tryCompleteDelayedProduce(requestKey) replicaManager.tryCompleteDelayedDeleteRecords(requestKey) + replicaManager.tryCompleteElection(requestKey) } def maybeShrinkIsr(replicaMaxLagTimeMs: Long) { diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index c8cf446edcd3c..8513bf062450d 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -32,7 +32,7 @@ import org.apache.kafka.common.{KafkaException, TopicPartition} import org.apache.kafka.common.errors.{BrokerNotAvailableException, ControllerMovedException, StaleBrokerEpochException} import org.apache.kafka.common.metrics.Metrics import org.apache.kafka.common.protocol.{ApiKeys, Errors} -import org.apache.kafka.common.requests.{AbstractControlRequest, AbstractResponse, LeaderAndIsrResponse, StopReplicaResponse} +import org.apache.kafka.common.requests.{AbstractControlRequest, AbstractResponse, ApiError, LeaderAndIsrResponse, StopReplicaResponse} import org.apache.kafka.common.utils.Time import org.apache.zookeeper.KeeperException import org.apache.zookeeper.KeeperException.Code @@ -628,17 +628,28 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti removePartitionsFromReassignedPartitions(partitionsToBeRemovedFromReassignment) } - private def onPreferredReplicaElection(partitions: Set[TopicPartition], isTriggeredByAutoRebalance: Boolean = false) { + private def onPreferredReplicaElection(partitions: Set[TopicPartition], + isTriggeredByAutoRebalance: Boolean = false, + newPath: Boolean = false): Map[TopicPartition, Throwable] = { info(s"Starting preferred replica leader election for partitions ${partitions.mkString(",")}") try { - partitionStateMachine.handleStateChanges(partitions.toSeq, OnlinePartition, Option(PreferredReplicaPartitionLeaderElectionStrategy)) - } catch { - case e: ControllerMovedException => - error(s"Error completing preferred replica leader election for partitions ${partitions.mkString(",")} because controller has moved to another broker.", e) - throw e - case e: Throwable => error(s"Error completing preferred replica leader election for partitions ${partitions.mkString(",")}", e) + val results = partitionStateMachine.handleStateChangesWithResults(partitions.toSeq, OnlinePartition, + Option(PreferredReplicaPartitionLeaderElectionStrategy)) + if (!newPath) { + results.foreach(entry => + if (entry._2.isInstanceOf[ControllerMovedException]) { + error(s"Error completing preferred replica leader election for partition ${entry._1} because controller has moved to another broker.", entry._2) + throw entry._2 + } else { + error(s"Error completing preferred replica leader election for partition ${entry._1}", entry._2) + } + ) + } + return results; } finally { - removePartitionsFromPreferredReplicaElection(partitions, isTriggeredByAutoRebalance) + if (!newPath) { + removePartitionsFromPreferredReplicaElection(partitions, isTriggeredByAutoRebalance) + } } } @@ -884,7 +895,8 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti if (!isTriggeredByAutoRebalance) { zkClient.deletePreferredReplicaElection(controllerContext.epochZkVersion) // Ensure we detect future preferred replica leader elections - eventManager.put(PreferredReplicaLeaderElection) + eventManager.put(PreferredReplicaLeaderElection(None)) + } } @@ -1517,22 +1529,57 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } } - case object PreferredReplicaLeaderElection extends ControllerEvent { + def electPreferredLeaders(partitions: Set[TopicPartition], callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = { (_,_) => }): Unit = + eventManager.put(PreferredReplicaLeaderElection(Some(partitions), true, callback)) + + case class PreferredReplicaLeaderElection(partitionsOpt: Option[Set[TopicPartition]], + newPath: Boolean = false, + callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends ControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance override def process(): Unit = { - if (!isActive) return + if (!isActive) { + callback(Set(), partitionsOpt match { + case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap + case None => Map.empty + }) + } else { + // We need to register the watcher if the path doesn't exist in order to detect future preferred replica + // leader elections and we get the `path exists` check for free + if (newPath || zkClient.registerZNodeChangeHandlerAndCheckExistence(preferredReplicaElectionHandler)) { + val partitions = partitionsOpt match { + case Some(partitions) => partitions + case None => zkClient.getPreferredReplicaElection + } + + val (partitionsBeingDeleted, livePartitions) = partitions.partition(partition => + topicDeletionManager.isTopicQueuedUpForDeletion(partition.topic)) + if (partitionsBeingDeleted.nonEmpty) { + error(s"Skipping preferred replica election for partitions $partitionsBeingDeleted " + + s"since the respective topics are being deleted") + } + // partition those where preferred is already leader + val (electablePartitions, alreadyPreferred) = livePartitions.partition { partition => + val assignedReplicas = controllerContext.partitionReplicaAssignment(partition) + val preferredReplica = assignedReplicas.head + val currentLeader = controllerContext.partitionLeadershipInfo(partition).leaderAndIsr.leader + currentLeader != preferredReplica + } - // We need to register the watcher if the path doesn't exist in order to detect future preferred replica - // leader elections and we get the `path exists` check for free - if (zkClient.registerZNodeChangeHandlerAndCheckExistence(preferredReplicaElectionHandler)) { - val partitions = zkClient.getPreferredReplicaElection - val partitionsForTopicsToBeDeleted = partitions.filter(p => topicDeletionManager.isTopicQueuedUpForDeletion(p.topic)) - if (partitionsForTopicsToBeDeleted.nonEmpty) { - error(s"Skipping preferred replica election for partitions $partitionsForTopicsToBeDeleted since the " + - "respective topics are being deleted") + val electionErrors = onPreferredReplicaElection(electablePartitions, newPath) + val successfulPartitions = electablePartitions -- electionErrors.keySet + val results = electionErrors.map { case (partition, ex) => + val apiError = if (ex.isInstanceOf[StateChangeFailedException]) + new ApiError(Errors.LEADER_NOT_AVAILABLE, ex.getMessage) + else + ApiError.fromThrowable(ex) + partition -> apiError + } ++ + alreadyPreferred.map(_ -> ApiError.NONE) ++ + partitionsBeingDeleted.map(_ -> new ApiError(Errors.INVALID_TOPIC_EXCEPTION, "The topic is being deleted")) + debug(s"PreferredReplicaLeaderElection waiting: $successfulPartitions, results: $results") + callback(successfulPartitions, results) } - onPreferredReplicaElection(partitions -- partitionsForTopicsToBeDeleted) } } } @@ -1653,7 +1700,7 @@ object IsrChangeNotificationHandler { class PreferredReplicaElectionHandler(controller: KafkaController, eventManager: ControllerEventManager) extends ZNodeChangeHandler { override val path: String = PreferredReplicaElectionZNode.path - override def handleCreation(): Unit = eventManager.put(controller.PreferredReplicaLeaderElection) + override def handleCreation(): Unit = eventManager.put(controller.PreferredReplicaLeaderElection(None)) } class ControllerChangeHandler(controller: KafkaController, eventManager: ControllerEventManager) extends ZNodeChangeHandler { diff --git a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala index e4f053264c479..e76540c669beb 100755 --- a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala +++ b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala @@ -141,6 +141,34 @@ class PartitionStateMachine(config: KafkaConfig, } } + /** + * Attempt to put each of the given partitions into the given targetState, returning + * a map of partition to exception for those partitions where the attempt failed with an exception. + * @param partitions The list of partitions that need to be transitioned to the target state + * @param targetState The state that the partitions should be moved to + */ + def handleStateChangesWithResults(partitions: Seq[TopicPartition], targetState: PartitionState, + partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy]): Map[TopicPartition, Throwable] = { + info(s"Invoking state change to $targetState for partitions ${partitions.mkString(",")}") + var result = Map.empty[TopicPartition, Throwable] + if (partitions.nonEmpty) { + try { + controllerBrokerRequestBatch.newBatch() + partitions.foreach { partition => + result ++= doHandleStateChanges(Seq(partition), targetState, partitionLeaderElectionStrategyOpt) + } + controllerBrokerRequestBatch.sendRequestsToBrokers(controllerContext.epoch) + } catch { + case e: Throwable => + error(s"Error changing state of some partitions to $targetState state", e) + result ++= (partitions.toSet -- result.keySet).map { + _ -> e + } + } + } + result + } + def partitionsInState(state: PartitionState): Set[TopicPartition] = { partitionState.filter { case (_, s) => s == state }.keySet.toSet } @@ -183,7 +211,7 @@ class PartitionStateMachine(config: KafkaConfig, * @param targetState The end state that the partition should be moved to */ private def doHandleStateChanges(partitions: Seq[TopicPartition], targetState: PartitionState, - partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy]): Unit = { + partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy]): Map[TopicPartition, Throwable] = { val stateChangeLog = stateChangeLogger.withControllerEpoch(controllerContext.epoch) partitions.foreach(partition => partitionState.getOrElseUpdate(partition, NonExistentPartition)) val (validPartitions, invalidPartitions) = partitions.partition(partition => isValidTransition(partition, targetState)) @@ -195,6 +223,7 @@ class PartitionStateMachine(config: KafkaConfig, s"assigned replicas ${controllerContext.partitionReplicaAssignment(partition).mkString(",")}") changeStateTo(partition, partitionState(partition), NewPartition) } + Map.empty case OnlinePartition => val uninitializedPartitions = validPartitions.filter(partition => partitionState(partition) == NewPartition) val partitionsToElectLeader = validPartitions.filter(partition => partitionState(partition) == OfflinePartition || partitionState(partition) == OnlinePartition) @@ -207,23 +236,28 @@ class PartitionStateMachine(config: KafkaConfig, } } if (partitionsToElectLeader.nonEmpty) { - val successfulElections = electLeaderForPartitions(partitionsToElectLeader, partitionLeaderElectionStrategyOpt.get) + val (successfulElections, failedElections) = electLeaderForPartitions(partitionsToElectLeader, partitionLeaderElectionStrategyOpt.get) successfulElections.foreach { partition => stateChangeLog.trace(s"Changed partition $partition from ${partitionState(partition)} to $targetState with state " + s"${controllerContext.partitionLeadershipInfo(partition).leaderAndIsr}") changeStateTo(partition, partitionState(partition), OnlinePartition) } + failedElections + } else { + Map.empty } case OfflinePartition => validPartitions.foreach { partition => stateChangeLog.trace(s"Changed partition $partition state from ${partitionState(partition)} to $targetState") changeStateTo(partition, partitionState(partition), OfflinePartition) } + Map.empty case NonExistentPartition => validPartitions.foreach { partition => stateChangeLog.trace(s"Changed partition $partition state from ${partitionState(partition)} to $targetState") changeStateTo(partition, partitionState(partition), NonExistentPartition) } + Map.empty } } @@ -285,9 +319,10 @@ class PartitionStateMachine(config: KafkaConfig, * @param partitionLeaderElectionStrategy The election strategy to use. * @return The partitions that successfully had a leader elected. */ - private def electLeaderForPartitions(partitions: Seq[TopicPartition], partitionLeaderElectionStrategy: PartitionLeaderElectionStrategy): Seq[TopicPartition] = { + private def electLeaderForPartitions(partitions: Seq[TopicPartition], partitionLeaderElectionStrategy: PartitionLeaderElectionStrategy): (Seq[TopicPartition], Map[TopicPartition, Throwable]) = { val successfulElections = mutable.Buffer.empty[TopicPartition] var remaining = partitions + var failures = Map.empty[TopicPartition, Throwable] while (remaining.nonEmpty) { val (success, updatesToRetry, failedElections) = doElectLeaderForPartitions(partitions, partitionLeaderElectionStrategy) remaining = updatesToRetry @@ -295,8 +330,9 @@ class PartitionStateMachine(config: KafkaConfig, failedElections.foreach { case (partition, e) => logFailedStateChange(partition, partitionState(partition), OnlinePartition, e) } + failures ++= failedElections } - successfulElections + (successfulElections, failures) } /** diff --git a/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala new file mode 100644 index 0000000000000..085648c4a71fe --- /dev/null +++ b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server + +import kafka.controller.ControllerContext +import org.apache.kafka.common.TopicPartition +import org.apache.kafka.common.protocol.Errors +import org.apache.kafka.common.requests.ApiError + +import scala.collection.{Map, Set, mutable} + +case class ElectPreferredLeaderMetadata(partition: TopicPartition, leader: Int) + +/** A delayed elect preferred leader operation that can be created by the replica manager and watched + * in the elect preferred leader purgatory + */ +class DelayedElectPreferredLeader(delayMs: Long, + expectedLeaders: Set[ElectPreferredLeaderMetadata], + results: Map[TopicPartition, ApiError], + replicaManager: ReplicaManager, + responseCallback: Map[TopicPartition, ApiError] => Unit) + extends DelayedOperation(delayMs) { + + var waitingPartitions = expectedLeaders.to[mutable.Set] + val fullResults = results.to[mutable.Set] + + + /** + * Call-back to execute when a delayed operation gets expired and hence forced to complete. + */ + override def onExpiration(): Unit = {} + + /** + * Process for completing an operation; This function needs to be defined + * in subclasses and will be called exactly once in forceComplete() + */ + override def onComplete(): Unit = { + // This could be called to force complete, so I need the full list of partitions, so I can time them all out. + updateWaiting() + val timedout = waitingPartitions.map(meta => meta.partition -> new ApiError(Errors.REQUEST_TIMED_OUT, null)).toMap + responseCallback(timedout ++ fullResults) + } + + private def timeoutWaiting = { + waitingPartitions.map(partition => partition -> new ApiError(Errors.REQUEST_TIMED_OUT, null)).toMap + } + + /** + * Try to complete the delayed operation by first checking if the operation + * can be completed by now. If yes execute the completion logic by calling + * forceComplete() and return true iff forceComplete returns true; otherwise return false + * + * This function needs to be defined in subclasses + */ + override def tryComplete(): Boolean = { + updateWaiting() + debug(s"tryComplete() waitingPartitions: $waitingPartitions") + waitingPartitions.isEmpty && forceComplete() + } + + private def updateWaiting() = { + waitingPartitions.foreach{m => + val ps = replicaManager.metadataCache.getPartitionInfo(m.partition.topic, m.partition.partition) + ps match { + case Some(ps) => + if (m.leader == ps.basePartitionState.leader) { + waitingPartitions -= m + fullResults += m.partition -> ApiError.NONE + } + case None => + } + } + } + +} diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index ad303ba3a4722..7302653fe92cc 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -146,6 +146,7 @@ class KafkaApis(val requestChannel: RequestChannel, case ApiKeys.EXPIRE_DELEGATION_TOKEN => handleExpireTokenRequest(request) case ApiKeys.DESCRIBE_DELEGATION_TOKEN => handleDescribeTokensRequest(request) case ApiKeys.DELETE_GROUPS => handleDeleteGroupsRequest(request) + case ApiKeys.ELECT_PREFERRED_LEADERS => handleElectPreferredReplicaLeader(request) } } catch { case e: FatalExitError => throw e @@ -253,6 +254,11 @@ class KafkaApis(val requestChannel: RequestChannel, quotas.request.updateQuotaMetricConfigs() } } + if (replicaManager.hasDelayedElectionOperations) { + updateMetadataRequest.partitionStates.asScala.foreach { case (tp, ps) => + replicaManager.tryCompleteElection(new TopicPartitionOperationKey(tp.topic(), tp.partition())) + } + } sendResponseExemptThrottle(request, new UpdateMetadataResponse(Errors.NONE)) } } @@ -2225,6 +2231,26 @@ class KafkaApis(val requestChannel: RequestChannel, true } + def handleElectPreferredReplicaLeader(request: RequestChannel.Request): Unit = { + + val electionRequest = request.body[ElectPreferredLeadersRequest] + val partitions = + if (electionRequest.topicPartitions() == null) { + zkClient.getAllPartitions() + } else { + electionRequest.topicPartitions().asScala + } + def sendResponseCallback(result: Map[TopicPartition, ApiError]): Unit = { + sendResponseMaybeThrottle(request, requestThrottleMs => + new ElectPreferredLeadersResponse(requestThrottleMs, result.asJava)) + } + if (!authorize(request.session, Alter, Resource.ClusterResource)) { + sendResponseCallback(partitions.map(partition => partition -> new ApiError(Errors.CLUSTER_AUTHORIZATION_FAILED, null)).toMap) + } else { + replicaManager.electPreferredLeaders(controller, partitions, sendResponseCallback, electionRequest.timeout) + } + } + def authorizeClusterAction(request: RequestChannel.Request): Unit = { if (!isAuthorizedClusterAction(request)) throw new ClusterAuthorizationException(s"Request $request is not authorized.") diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 5dfb2e66dff01..9ee6d7aa48262 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -146,6 +146,7 @@ class ReplicaManager(val config: KafkaConfig, val delayedProducePurgatory: DelayedOperationPurgatory[DelayedProduce], val delayedFetchPurgatory: DelayedOperationPurgatory[DelayedFetch], val delayedDeleteRecordsPurgatory: DelayedOperationPurgatory[DelayedDeleteRecords], + val delayedElectPreferredLeaderPurgatory: DelayedOperationPurgatory[DelayedElectPreferredLeader], threadNamePrefix: Option[String]) extends Logging with KafkaMetricsGroup { def this(config: KafkaConfig, @@ -171,6 +172,8 @@ class ReplicaManager(val config: KafkaConfig, DelayedOperationPurgatory[DelayedDeleteRecords]( purgatoryName = "DeleteRecords", brokerId = config.brokerId, purgeInterval = config.deleteRecordsPurgatoryPurgeIntervalRequests), + DelayedOperationPurgatory[DelayedElectPreferredLeader]( + purgatoryName = "ElectPreferredLeader", brokerId = config.brokerId), threadNamePrefix) } @@ -318,6 +321,13 @@ class ReplicaManager(val config: KafkaConfig, debug("Request key %s unblocked %d DeleteRecordsRequest.".format(key.keyLabel, completed)) } + def hasDelayedElectionOperations = delayedElectPreferredLeaderPurgatory.delayed != 0 + + def tryCompleteElection(key: DelayedOperationKey): Unit = { + val completed = delayedElectPreferredLeaderPurgatory.checkAndComplete(key) + debug("Request key %s unblocked %d ElectPreferredLeader.".format(key.keyLabel, completed)) + } + def startup() { // start ISR expiration thread // A follower can lag behind leader for up to config.replicaLagTimeMaxMs x 1.5 before it is removed from ISR @@ -1476,6 +1486,7 @@ class ReplicaManager(val config: KafkaConfig, delayedFetchPurgatory.shutdown() delayedProducePurgatory.shutdown() delayedDeleteRecordsPurgatory.shutdown() + delayedElectPreferredLeaderPurgatory.shutdown() if (checkpointHW) checkpointHighWatermarks() info("Shut down completely") @@ -1508,4 +1519,37 @@ class ReplicaManager(val config: KafkaConfig, tp -> epochEndOffset } } + + def electPreferredLeaders(controller: KafkaController, + partitions: Set[TopicPartition], + responseCallback: Map[TopicPartition, ApiError] => Unit, + requestTimeout: Long): Unit = { + + val (validPartitions, invalidPartitions) = partitions.partition(tp => metadataCache.contains(tp)) + + val invalidPartitionsResults = invalidPartitions.map { p => + val msg = s"Skipping preferred replica leader election for partition ${p} since it doesn't exist." + logger.info(msg) + p -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition '$p' does not exist.") + }.toMap + + def electionCallback(waiting: Set[TopicPartition], results: Map[TopicPartition, ApiError]) = { + if (waiting.nonEmpty) { + // timeout + val expectedLeaders = waiting.map( + tp => ElectPreferredLeaderMetadata(tp, controller.controllerContext.partitionReplicaAssignment(tp).head)) + + val watchKeys = waiting.map(p => new TopicPartitionOperationKey(p.topic, p.partition)).toSeq + delayedElectPreferredLeaderPurgatory.tryCompleteElseWatch( + new DelayedElectPreferredLeader(requestTimeout, expectedLeaders, results ++ invalidPartitionsResults, + this, responseCallback), + watchKeys) + } else { + // There are no partitions actually being elected, so return immediately + responseCallback(results ++ invalidPartitionsResults) + } + } + + controller.electPreferredLeaders(validPartitions, electionCallback) + } } diff --git a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala index 92d1758b19460..facf88e914a2d 100644 --- a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala @@ -47,13 +47,14 @@ import org.junit.Assert._ import scala.util.Random import scala.collection.JavaConverters._ -import java.lang.{Long => JLong} import kafka.zk.KafkaZkClient import scala.concurrent.duration.Duration import scala.concurrent.{Await, Future} +import java.lang.{Long => JLong} + /** * An integration test of the KafkaAdminClient. * @@ -1198,6 +1199,194 @@ class AdminClientIntegrationTest extends IntegrationTestHarness with Logging { } } + @Test + def testElectPreferredLeaders(): Unit = { + client = AdminClient.create(createConfig) + + val prefer0 = Seq(0, 1, 2) + val prefer1 = Seq(1, 2, 0) + val prefer2 = Seq(2, 0, 1) + + val partition1 = new TopicPartition("elect-preferred-leaders-topic-1", 0) + TestUtils.createTopic(zkClient, partition1.topic, Map[Int, Seq[Int]](partition1.partition -> prefer0), servers) + + val partition2 = new TopicPartition("elect-preferred-leaders-topic-2", 0) + TestUtils.createTopic(zkClient, partition2.topic, Map[Int, Seq[Int]](partition2.partition -> prefer0), servers) + + def currentLeader(topicPartition: TopicPartition) = + client.describeTopics(asList(topicPartition.topic)).values.get(topicPartition.topic). + get.partitions.get(topicPartition.partition).leader.id + + def preferredLeader(topicPartition: TopicPartition) = + client.describeTopics(asList(topicPartition.topic)).values.get(topicPartition.topic). + get.partitions.get(topicPartition.partition).replicas.get(0).id + + def waitForLeaderToBecome(topicPartition: TopicPartition, leader: Int) = + TestUtils.waitUntilTrue(() => currentLeader(topicPartition) == leader, s"Expected leader to become $leader", 10000) + + /** Changes the preferred leader without changing the current leader. */ + def changePreferredLeader(newAssignment: Seq[Int]) = { + val preferred = newAssignment.head + val prior1 = currentLeader(partition1) + val prior2 = currentLeader(partition2) + + var m = Map.empty[TopicPartition, Seq[Int]] + + if (prior1 != preferred) + m += partition1 -> newAssignment + if (prior2 != preferred) + m += partition2 -> newAssignment + + zkClient.createPartitionReassignment(m) + TestUtils.waitUntilTrue( + () => preferredLeader(partition1) == preferred && preferredLeader(partition2) == preferred, + s"Expected preferred leader to become $preferred, but is ${preferredLeader(partition1)} and ${preferredLeader(partition2)}", 10000) + // Check the leader hasn't moved + assertEquals(prior1, currentLeader(partition1)) + assertEquals(prior2, currentLeader(partition2)) + } + + // Check current leaders are 0 + assertEquals(0, currentLeader(partition1)) + assertEquals(0, currentLeader(partition2)) + + // Noop election + var electResult = client.electPreferredLeaders(asList(partition1)) + electResult.partitionResult(partition1).get() + assertEquals(0, currentLeader(partition1)) + + // Noop election with null partitions + electResult = client.electPreferredLeaders(null) + electResult.partitionResult(partition1).get() + assertEquals(0, currentLeader(partition1)) + electResult.partitionResult(partition2).get() + assertEquals(0, currentLeader(partition2)) + + // Now change the preferred leader to 1 + changePreferredLeader(prefer1) + + // meaningful election + electResult = client.electPreferredLeaders(asList(partition1)) + assertEquals(Set(partition1).asJava, electResult.partitions.get) + electResult.partitionResult(partition1).get() + waitForLeaderToBecome(partition1, 1) + + // topic 2 unchanged + try { + electResult.partitionResult(partition2).get() + fail("topic 2 wasn't requested") + } catch { + case e: ExecutionException => + val cause = e.getCause + assertTrue(cause.getClass.getName, cause.isInstanceOf[UnknownTopicOrPartitionException]) + assertEquals("Preferred leader election for partition \"elect-preferred-leaders-topic-2-0\" was not attempted", + cause.getMessage) + assertEquals(0, currentLeader(partition2)) + } + + // meaningful election with null partitions + electResult = client.electPreferredLeaders(null) + assertEquals(Set(partition1, partition2), electResult.partitions.get.asScala.filterNot(_.topic == "__consumer_offsets")) + electResult.partitionResult(partition1).get() + waitForLeaderToBecome(partition1, 1) + electResult.partitionResult(partition2).get() + waitForLeaderToBecome(partition2, 1) + + // unknown topic + val unknownPartition = new TopicPartition("topic-does-not-exist", 0) + electResult = client.electPreferredLeaders(asList(unknownPartition)) + assertEquals(Set(unknownPartition).asJava, electResult.partitions.get) + try { + electResult.partitionResult(unknownPartition).get() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.isInstanceOf[UnknownTopicOrPartitionException]) + assertEquals("The partition 'topic-does-not-exist-0' does not exist.", + cause.getMessage) + assertEquals(1, currentLeader(partition1)) + assertEquals(1, currentLeader(partition2)) + } + + // Now change the preferred leader to 2 + changePreferredLeader(prefer2) + + // mixed results + electResult = client.electPreferredLeaders(asList(unknownPartition, partition1)) + assertEquals(Set(unknownPartition, partition1).asJava, electResult.partitions.get) + waitForLeaderToBecome(partition1, 2) + assertEquals(1, currentLeader(partition2)) + try { + electResult.partitionResult(unknownPartition).get() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.isInstanceOf[UnknownTopicOrPartitionException]) + assertEquals("The partition 'topic-does-not-exist-0' does not exist.", + cause.getMessage) + } + + // dupe partitions + electResult = client.electPreferredLeaders(asList(partition2, partition2)) + assertEquals(Set(partition2).asJava, electResult.partitions.get) + electResult.partitionResult(partition2).get() + waitForLeaderToBecome(partition2, 2) + + // Now change the preferred leader to 1 + changePreferredLeader(prefer1) + // but shut it down... + servers(1).shutdown() + + // ... now what happens if we try to elect the preferred leader and it's down? + val shortTimeout = new ElectPreferredLeadersOptions().timeoutMs(10000) + electResult = client.electPreferredLeaders(asList(partition1), shortTimeout) + assertEquals(Set(partition1).asJava, electResult.partitions.get) + try { + electResult.partitionResult(partition1).get() + fail() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.getClass.getName, cause.isInstanceOf[LeaderNotAvailableException]) + assertTrue(s"Wrong message ${cause.getMessage}", cause.getMessage.contains( + "Failed to elect leader for partition elect-preferred-leaders-topic-1-0 under strategy PreferredReplicaPartitionLeaderElectionStrategy")) + } + assertEquals(2, currentLeader(partition1)) + + // preferred leader unavailable with null argument + electResult = client.electPreferredLeaders(null, shortTimeout) + try { + electResult.partitions.get() + fail() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.getClass.getName, cause.isInstanceOf[LeaderNotAvailableException]) + } + try { + electResult.partitionResult(partition1).get() + fail() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.getClass.getName, cause.isInstanceOf[LeaderNotAvailableException]) + assertTrue(s"Wrong message ${cause.getMessage}", cause.getMessage.contains( + "Failed to elect leader for partition elect-preferred-leaders-topic-1-0 under strategy PreferredReplicaPartitionLeaderElectionStrategy")) + } + try { + electResult.partitionResult(partition2).get() + fail() + } catch { + case e: Exception => + val cause = e.getCause + assertTrue(cause.getClass.getName, cause.isInstanceOf[LeaderNotAvailableException]) + assertTrue(s"Wrong message ${cause.getMessage}", cause.getMessage.contains( + "Failed to elect leader for partition elect-preferred-leaders-topic-2-0 under strategy PreferredReplicaPartitionLeaderElectionStrategy")) + } + + assertEquals(2, currentLeader(partition1)) + assertEquals(2, currentLeader(partition2)) + } } object AdminClientIntegrationTest { diff --git a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala index 4c0459e72e0aa..34d4ab5024852 100644 --- a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala @@ -146,7 +146,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.DESCRIBE_ACLS -> classOf[DescribeAclsResponse], ApiKeys.ALTER_REPLICA_LOG_DIRS -> classOf[AlterReplicaLogDirsResponse], ApiKeys.DESCRIBE_LOG_DIRS -> classOf[DescribeLogDirsResponse], - ApiKeys.CREATE_PARTITIONS -> classOf[CreatePartitionsResponse] + ApiKeys.CREATE_PARTITIONS -> classOf[CreatePartitionsResponse], + ApiKeys.ELECT_PREFERRED_LEADERS -> classOf[ElectPreferredLeadersResponse] ) val requestKeyToError = Map[ApiKeys, Nothing => Errors]( @@ -187,7 +188,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.ALTER_REPLICA_LOG_DIRS -> ((resp: AlterReplicaLogDirsResponse) => resp.responses.get(tp)), ApiKeys.DESCRIBE_LOG_DIRS -> ((resp: DescribeLogDirsResponse) => if (resp.logDirInfos.size() > 0) resp.logDirInfos.asScala.head._2.error else Errors.CLUSTER_AUTHORIZATION_FAILED), - ApiKeys.CREATE_PARTITIONS -> ((resp: CreatePartitionsResponse) => resp.errors.asScala.find(_._1 == topic).get._2.error) + ApiKeys.CREATE_PARTITIONS -> ((resp: CreatePartitionsResponse) => resp.errors.asScala.find(_._1 == topic).get._2.error), + ApiKeys.ELECT_PREFERRED_LEADERS -> ((resp: ElectPreferredLeadersResponse) => resp.errors.get(tp).error) ) val requestKeysToAcls = Map[ApiKeys, Map[Resource, Set[Acl]]]( @@ -225,7 +227,9 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.DELETE_ACLS -> clusterAlterAcl, ApiKeys.ALTER_REPLICA_LOG_DIRS -> clusterAlterAcl, ApiKeys.DESCRIBE_LOG_DIRS -> clusterDescribeAcl, - ApiKeys.CREATE_PARTITIONS -> topicAlterAcl + ApiKeys.CREATE_PARTITIONS -> topicAlterAcl, + ApiKeys.CREATE_PARTITIONS -> topicAlterAcl, + ApiKeys.ELECT_PREFERRED_LEADERS -> clusterAlterAcl ) @Before @@ -382,6 +386,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { private def addOffsetsToTxnRequest = new AddOffsetsToTxnRequest.Builder(transactionalId, 1, 1, group).build() + private def electPreferredLeadersRequest = new ElectPreferredLeadersRequest.Builder(Collections.singleton(tp), 10000).build() + @Test def testAuthorizationWithTopicExisting() { val requestKeyToRequest = mutable.LinkedHashMap[ApiKeys, AbstractRequest]( @@ -414,9 +420,9 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.CREATE_PARTITIONS -> createPartitionsRequest, ApiKeys.ADD_PARTITIONS_TO_TXN -> addPartitionsToTxnRequest, ApiKeys.ADD_OFFSETS_TO_TXN -> addOffsetsToTxnRequest, - // Check StopReplica last since some APIs depend on replica availability - ApiKeys.STOP_REPLICA -> stopReplicaRequest + ApiKeys.STOP_REPLICA -> stopReplicaRequest, + ApiKeys.ELECT_PREFERRED_LEADERS -> electPreferredLeadersRequest ) for ((key, request) <- requestKeyToRequest) { @@ -462,7 +468,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.ADD_OFFSETS_TO_TXN -> addOffsetsToTxnRequest, ApiKeys.CREATE_PARTITIONS -> createPartitionsRequest, ApiKeys.DELETE_GROUPS -> deleteGroupsRequest, - ApiKeys.OFFSET_FOR_LEADER_EPOCH -> offsetsForLeaderEpochRequest + ApiKeys.OFFSET_FOR_LEADER_EPOCH -> offsetsForLeaderEpochRequest, + ApiKeys.ELECT_PREFERRED_LEADERS -> electPreferredLeadersRequest ) for ((key, request) <- requestKeyToRequest) { diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala new file mode 100644 index 0000000000000..b4eac8e529694 --- /dev/null +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -0,0 +1,341 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package unit.kafka.admin + +import java.io.File +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Paths} +import java.util.Properties + +import kafka.admin.{PreferredReplicaLeaderElectionCommand} +import kafka.common.{AdminCommandFailedException, TopicAndPartition} +import kafka.network.RequestChannel +import kafka.security.auth._ +import kafka.server.{KafkaConfig, KafkaServer} +import kafka.utils.{Logging, TestUtils, ZkUtils} +import kafka.zk.ZooKeeperTestHarness +import org.apache.kafka.common.TopicPartition +import org.apache.kafka.common.errors.{LeaderNotAvailableException, TimeoutException, UnknownTopicOrPartitionException} +import org.apache.kafka.common.network.ListenerName +import org.junit.Assert._ +import org.junit.{After, Test} + +class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness with Logging /*with RackAwareTest*/ { + + var servers: Seq[KafkaServer] = Seq() + + @After + override def tearDown() { + TestUtils.shutdownServers(servers) + super.tearDown() + } + + private def createTestTopicAndCluster(topicPartition: Map[TopicPartition, List[Int]], + authorizer: Option[String] = None) { + + val brokerConfigs = TestUtils.createBrokerConfigs(3, zkConnect, false) + brokerConfigs.foreach(p => p.setProperty("auto.leader.rebalance.enable", "false")) + authorizer match { + case Some(className) => + brokerConfigs.foreach(p => p.setProperty("authorizer.class.name", className)) + case None => + } + createTestTopicAndCluster(topicPartition,brokerConfigs) + } + + private def createTestTopicAndCluster(partitionsAndAssignments: Map[TopicPartition, List[Int]], + brokerConfigs: Seq[Properties]) { + // create brokers + servers = brokerConfigs.map(b => TestUtils.createServer(KafkaConfig.fromProps(b))) + // create the topic +// partitionsAndAssignments.foreach { partitionAndAssignment => +// zkClient.createPartitionReassignment()OrUpdateTopicPartitionAssignmentPathInZK(partitionAndAssignment._1.topic(), +// Map(partitionAndAssignment._1.partition -> partitionAndAssignment._2)) +// } + // wait until replica log is created on every broker + TestUtils.waitUntilTrue(() => servers.forall(server => partitionsAndAssignments.forall(partitionAndAssignment => server.getLogManager().getLog(partitionAndAssignment._1).isDefined)), + "Replicas for topic test not created") + } + + /** Bounce the given server and wait for all servers to get metadata for the given partition */ + private def bounceServer(server: Int, partition: TopicPartition) { + info(s"Shutting down server $server so a non-preferred replica becomes leader") + servers(server).shutdown() + info(s"Starting server $server now that a non-preferred replica is leader") + servers(server).startup() + TestUtils.waitUntilTrue(() => servers.forall(server => server.metadataCache.getPartitionInfo(partition.topic(), partition.partition()).isDefined), + s"Replicas for partition $partition not created") + } + + private def getController() = { + servers.find(p => p.kafkaController.isActive) + } + + private def getLeader(topicPartition: TopicPartition) = { + servers(0).metadataCache.getPartitionInfo(topicPartition.topic(), topicPartition.partition()).get.basePartitionState.leader + } + + private def bootstrapServer(broker: Int = 0): String = { + val port = servers(broker).socketServer.boundPort(ListenerName.normalised("PLAINTEXT")) + info("Server bound to port "+port) + s"localhost:$port" + } + + val testPartition = new TopicPartition("test", 0) + val testPartitionAssignment = List(1, 2, 0) + val testPartitionPreferredLeader = testPartitionAssignment.head + val testPartitionAndAssignment = Map(testPartition -> testPartitionAssignment) + + /** Test the case multiple values are given for --bootstrap-broker */ + @Test + def testMultipleBrokersGiven() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", s"${bootstrapServer(1)},${bootstrapServer(0)}")) + // Check the leader for the partition IS the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartition)) + } + + /** Test the case when an invalid broker is given for --bootstrap-broker */ + @Test + def testInvalidBrokerGiven() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", "example.com:1234"), + timeout = 10000) + } catch { + case e: AdminCommandFailedException => + if (e.getCause.isInstanceOf[TimeoutException]) { + // Check the leader for the partition is STILL not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + } else { + throw e + } + } + + } + + /** Test the case where no partitions are given (=> elect all partitions) */ + @Test + def testNoPartitionsGiven() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer())) + // Check the leader for the partition IS the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartition)) + } + + private def toJsonFile(partitions: scala.collection.Set[TopicPartition]): File = { + val jsonFile = File.createTempFile("preferredreplicaelection", ".js") + jsonFile.deleteOnExit() + val jsonString = ZkUtils.preferredReplicaLeaderElectionZkData(partitions.map(new TopicAndPartition(_))) + info("Using json: "+jsonString) + Files.write(Paths.get(jsonFile.getAbsolutePath), jsonString.getBytes(StandardCharsets.UTF_8)) + jsonFile + } + + /** Test the case where a list of partitions is given */ + @Test + def testSingletonPartitionGiven() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + } finally { + jsonFile.delete() + } + // Check the leader for the partition IS the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartition)) + } + + /** Test the case where a topic does not exist */ + @Test + def testTopicDoesNotExist() { + val nonExistentPartition = new TopicPartition("does.not.exist", 0) + val nonExistentPartitionAssignment = List(1, 2, 0) + val nonExistentPartitionAndAssignment = Map(nonExistentPartition -> nonExistentPartitionAssignment) + + createTestTopicAndCluster(testPartitionAndAssignment) + val jsonFile = toJsonFile(nonExistentPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + } catch { + case e: AdminCommandFailedException => + val suppressed = e.getSuppressed()(0) + assertTrue(suppressed.isInstanceOf[UnknownTopicOrPartitionException]) + case e: Throwable => + e.printStackTrace() + throw e + } finally { + jsonFile.delete() + } + } + + /** Test the case where several partitions are given */ + @Test + def testMultiplePartitionsSameAssignment() { + val testPartitionA = new TopicPartition("testA", 0) + val testPartitionB = new TopicPartition("testB", 0) + val testPartitionAssignment = List(1, 2, 0) + val testPartitionPreferredLeader = testPartitionAssignment.head + val testPartitionAndAssignment = Map(testPartitionA -> testPartitionAssignment, testPartitionB -> testPartitionAssignment) + + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartitionA) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartitionA)) + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartitionB)) + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + } finally { + jsonFile.delete() + } + // Check the leader for the partition IS the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartitionA)) + assertEquals(testPartitionPreferredLeader, getLeader(testPartitionB)) + } + + /** What happens when the preferred replica is already the leader? */ + @Test + def testNoopElection() { + createTestTopicAndCluster(testPartitionAndAssignment) + // Don't bounce the server. Doublec heck the leader for the partition is the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartition)) + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + // Now do the election, even though the preferred replica is *already* the leader + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + // Check the leader for the partition still is the preferred one + assertEquals(testPartitionPreferredLeader, getLeader(testPartition)) + } finally { + jsonFile.delete() + } + } + + /** What happens if the preferred replica is offline? */ + @Test + def testWithOfflinePreferredReplica() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + val leader = getLeader(testPartition) + assertNotEquals(testPartitionPreferredLeader, leader) + // Now kill the preferred one + servers(testPartitionPreferredLeader).shutdown() + // Now try to elect the preferred one + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + fail(); + } catch { + case e: AdminCommandFailedException => + assertEquals("1 preferred replica(s) could not be elected", e.getMessage) + val suppressed = e.getSuppressed()(0) + assertTrue(suppressed.isInstanceOf[LeaderNotAvailableException]) + assertTrue(suppressed.getMessage, suppressed.getMessage.contains("Failed to elect leader for partition test-0 under strategy PreferredReplicaPartitionLeaderElectionStrategy")) + // Check we still have the same leader + assertEquals(leader, getLeader(testPartition)) + } finally { + jsonFile.delete() + } + } + + /** What happens if the controller gets killed just before an election? */ + @Test + def testTimeout() { + createTestTopicAndCluster(testPartitionAndAssignment) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + val leader = getLeader(testPartition) + assertNotEquals(testPartitionPreferredLeader, leader) + // Now kill the controller just before we trigger the election + servers(getController().get.config.brokerId).shutdown() + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath), + timeout = 10000) + fail(); + } catch { + case e: AdminCommandFailedException => + assertEquals("1 preferred replica(s) could not be elected", e.getMessage) + assertTrue(e.getSuppressed()(0).getMessage.contains("Timed out waiting for a node assignment")) + // Check we still have the same leader + assertEquals(leader, getLeader(testPartition)) + } finally { + jsonFile.delete() + } + } + + /** Test the case where a list of partitions is given */ + @Test + def testAuthzFailure() { + createTestTopicAndCluster(testPartitionAndAssignment, Some(classOf[PreferredReplicaLeaderElectionCommandTestAuthorizer].getName)) + bounceServer(testPartitionPreferredLeader, testPartition) + // Check the leader for the partition is not the preferred one + val leader = getLeader(testPartition) + assertNotEquals(testPartitionPreferredLeader, leader) + // Check the leader for the partition is not the preferred one + assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) + val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) + try { + PreferredReplicaLeaderElectionCommand.run(Array( + "--bootstrap-server", bootstrapServer(), + "--path-to-json-file", jsonFile.getAbsolutePath)) + fail(); + } catch { + case e: AdminCommandFailedException => + assertEquals("1 preferred replica(s) could not be elected", e.getMessage) + assertTrue(e.getSuppressed()(0).getMessage.contains("Cluster authorization failed")) + // Check we still have the same leader + assertEquals(leader, getLeader(testPartition)) + } finally { + jsonFile.delete() + } + } + +} + +class PreferredReplicaLeaderElectionCommandTestAuthorizer extends SimpleAclAuthorizer { + override def authorize(session: RequestChannel.Session, operation: Operation, resource: Resource): Boolean = + operation != Alter || resource.resourceType != Cluster +} diff --git a/core/src/test/scala/unit/kafka/coordinator/AbstractCoordinatorConcurrencyTest.scala b/core/src/test/scala/unit/kafka/coordinator/AbstractCoordinatorConcurrencyTest.scala index d4dcd9f100ff4..d5beceae8e574 100644 --- a/core/src/test/scala/unit/kafka/coordinator/AbstractCoordinatorConcurrencyTest.scala +++ b/core/src/test/scala/unit/kafka/coordinator/AbstractCoordinatorConcurrencyTest.scala @@ -158,7 +158,7 @@ object AbstractCoordinatorConcurrencyTest { } class TestReplicaManager extends ReplicaManager( - null, null, null, null, null, null, null, null, null, null, null, null, null, null, None) { + null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, None) { var producePurgatory: DelayedOperationPurgatory[DelayedProduce] = _ var watchKeys: mutable.Set[TopicPartitionOperationKey] = _ diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala index 1308820f48ca3..08aa624734b36 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala @@ -657,6 +657,8 @@ class ReplicaManagerTest { purgatoryName = "Fetch", timer, reaperEnabled = false) val mockDeleteRecordsPurgatory = new DelayedOperationPurgatory[DelayedDeleteRecords]( purgatoryName = "DeleteRecords", timer, reaperEnabled = false) + val mockElectPreferredLeaderPurgatory = new DelayedOperationPurgatory[DelayedElectPreferredLeader]( + purgatoryName = "ElectPreferredLeader", timer, reaperEnabled = false) // Mock network client to show leader offset of 5 val quota = QuotaFactory.instantiate(config, metrics, time, "") @@ -665,7 +667,7 @@ class ReplicaManagerTest { val replicaManager = new ReplicaManager(config, metrics, time, kafkaZkClient, mockScheduler, mockLogMgr, new AtomicBoolean(false), quota, mockBrokerTopicStats, metadataCache, mockLogDirFailureChannel, mockProducePurgatory, mockFetchPurgatory, - mockDeleteRecordsPurgatory, Option(this.getClass.getName)) { + mockDeleteRecordsPurgatory, mockElectPreferredLeaderPurgatory, Option(this.getClass.getName)) { override protected def createReplicaFetcherManager(metrics: Metrics, time: Time, @@ -815,11 +817,13 @@ class ReplicaManagerTest { purgatoryName = "Fetch", timer, reaperEnabled = false) val mockDeleteRecordsPurgatory = new DelayedOperationPurgatory[DelayedDeleteRecords]( purgatoryName = "DeleteRecords", timer, reaperEnabled = false) + val mockDelayedElectPreferredLeaderPurgatory = new DelayedOperationPurgatory[DelayedElectPreferredLeader]( + purgatoryName = "DelayedElectPreferredLeader", timer, reaperEnabled = false) new ReplicaManager(config, metrics, time, kafkaZkClient, new MockScheduler(time), mockLogMgr, new AtomicBoolean(false), QuotaFactory.instantiate(config, metrics, time, ""), new BrokerTopicStats, metadataCache, new LogDirFailureChannel(config.logDirs.size), mockProducePurgatory, mockFetchPurgatory, - mockDeleteRecordsPurgatory, Option(this.getClass.getName)) + mockDeleteRecordsPurgatory, mockDelayedElectPreferredLeaderPurgatory, Option(this.getClass.getName)) } } diff --git a/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala b/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala index c9e4e7890ebf6..ac380be0b96a4 100644 --- a/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala +++ b/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala @@ -359,6 +359,9 @@ class RequestQuotaTest extends BaseRequestTest { case ApiKeys.DELETE_GROUPS => new DeleteGroupsRequest.Builder(Collections.singleton("test-group")) + case ApiKeys.ELECT_PREFERRED_LEADERS => + new ElectPreferredLeadersRequest.Builder(Collections.singleton(new TopicPartition("my_topic", 0)), 0) + case _ => throw new IllegalArgumentException("Unsupported API key " + apiKey) } @@ -450,6 +453,7 @@ class RequestQuotaTest extends BaseRequestTest { case ApiKeys.RENEW_DELEGATION_TOKEN => new RenewDelegationTokenResponse(response).throttleTimeMs case ApiKeys.DELETE_GROUPS => new DeleteGroupsResponse(response).throttleTimeMs case ApiKeys.OFFSET_FOR_LEADER_EPOCH => new OffsetsForLeaderEpochResponse(response).throttleTimeMs + case ApiKeys.ELECT_PREFERRED_LEADERS => new ElectPreferredLeadersResponse(response).throttleTimeMs case requestId => throw new IllegalArgumentException(s"No throttle time for $requestId") } } From 1e01e86d1509c7c82e814f56e3e1069f79ace1aa Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 7 Jan 2019 16:12:20 +0000 Subject: [PATCH 02/22] wip --- ...referredReplicaLeaderElectionCommand.scala | 23 +++++++++++-------- ...rredReplicaLeaderElectionCommandTest.scala | 5 ++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 86924d49e9388..1c8deba6fe431 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -19,6 +19,7 @@ package kafka.admin import java.util.Properties import java.util.concurrent.ExecutionException +import joptsimple.OptionSpecBuilder import kafka.common.AdminCommandFailedException import kafka.utils._ import kafka.zk.KafkaZkClient @@ -29,7 +30,6 @@ import collection.JavaConverters._ import org.apache.kafka.common.utils.{Time, Utils} import org.apache.kafka.common.security.JaasUtils import org.apache.kafka.common.{KafkaFuture, TopicPartition} - import org.apache.zookeeper.KeeperException.NodeExistsException import collection._ @@ -52,7 +52,7 @@ object PreferredReplicaLeaderElectionCommand extends Logging { " The preferred replica is the first one in the replica assignment (see kafka-reassign-partitions.sh)." + " Using this command is not necessary when the broker is configured with \"auto.leader.rebalance.enable=true\".") - CommandLineUtils.checkRequiredArgs(commandOpts.parser, commandOpts.options, commandOpts.zkConnectOpt) + CommandLineUtils.checkRequiredArgs(commandOpts.parser, commandOpts.options) if (commandOpts.options.has(commandOpts.bootstrapServerOpt) == commandOpts.options.has(commandOpts.zkConnectOpt)) { CommandLineUtils.printUsageAndDie(commandOpts.parser, s"Exactly one of '${commandOpts.bootstrapServerOpt}' or '${commandOpts.zkConnectOpt}' must be provided") @@ -123,17 +123,22 @@ object PreferredReplicaLeaderElectionCommand extends Logging { .withRequiredArg .describedAs("list of partitions for which preferred replica leader election needs to be triggered") .ofType(classOf[String]) - val zkConnectOpt = parser.accepts("zookeeper", "REQUIRED: The connection string for the zookeeper connection in the " + - "form host:port. Multiple URLS can be given to allow fail-over.") - .withRequiredArg - .describedAs("urls") - .ofType(classOf[String]) - val bootstrapServerOpt = parser.accepts("bootstrap-server", "A hostname and port for the broker to connect to, " + + + private val zookeeperOptBuilder: OptionSpecBuilder = parser.accepts("zookeeper", "The connection string for the zookeeper connection in the " + + "form host:port. Multiple URLS can be given to allow fail-over. " + + "DEPRECATED, replaced by --bootstrap-server, REQUIRED unless --bootstrap-server is given.") + private val bootstrapOptBuilder: OptionSpecBuilder = parser + .accepts("bootstrap-server", "A hostname and port for the broker to connect to, " + "in the form host:port. Multiple comma-separated URLs can be given. REQUIRED unless --zookeeper is given.") + parser.mutuallyExclusive(zookeeperOptBuilder, bootstrapOptBuilder) + val bootstrapServerOpt = bootstrapOptBuilder .withRequiredArg .describedAs("host:port") .ofType(classOf[String]) - + val zkConnectOpt = zookeeperOptBuilder + .withRequiredArg + .describedAs("urls") + .ofType(classOf[String]) options = parser.parse(args: _*) } diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index b4eac8e529694..871de3e34e35c 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -62,10 +62,15 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit // create brokers servers = brokerConfigs.map(b => TestUtils.createServer(KafkaConfig.fromProps(b))) // create the topic + //zkClient.createPartitionReassignment(partitionsAndAssignments) // partitionsAndAssignments.foreach { partitionAndAssignment => // zkClient.createPartitionReassignment()OrUpdateTopicPartitionAssignmentPathInZK(partitionAndAssignment._1.topic(), // Map(partitionAndAssignment._1.partition -> partitionAndAssignment._2)) // } + partitionsAndAssignments.foreach { partitionAndAssignment => + zkClient.createTopicAssignment(partitionAndAssignment._1.topic(), + Map(partitionAndAssignment._1 -> partitionAndAssignment._2)) + } // wait until replica log is created on every broker TestUtils.waitUntilTrue(() => servers.forall(server => partitionsAndAssignments.forall(partitionAndAssignment => server.getLogManager().getLog(partitionAndAssignment._1).isDefined)), "Replicas for topic test not created") From 3620a0a846551c6e7ec8ff73cd00e35d81ba2099 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Tue, 8 Jan 2019 09:59:14 +0000 Subject: [PATCH 03/22] Easy review comments --- .../org/apache/kafka/clients/admin/AdminClient.java | 2 +- .../common/requests/ElectPreferredLeadersRequest.java | 10 ++++------ .../org/apache/kafka/common/utils/CollectionUtils.java | 2 +- .../kafka/clients/admin/KafkaAdminClientTest.java | 1 - .../admin/PreferredReplicaLeaderElectionCommand.scala | 7 +++---- core/src/main/scala/kafka/cluster/Partition.scala | 1 - .../scala/kafka/controller/PartitionStateMachine.scala | 6 ++++-- .../kafka/server/DelayedElectPreferredLeader.scala | 1 - core/src/main/scala/kafka/server/KafkaApis.scala | 3 ++- core/src/main/scala/kafka/server/MetadataCache.scala | 9 +++++++++ core/src/main/scala/kafka/server/ReplicaManager.scala | 8 ++++---- .../kafka/api/AdminClientIntegrationTest.scala | 4 ++-- .../PreferredReplicaLeaderElectionCommandTest.scala | 5 ----- 13 files changed, 30 insertions(+), 29 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java index 8504cba6fad88..e6c09b51c19a2 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java @@ -818,7 +818,7 @@ public ElectPreferredLeadersResult electPreferredLeaders(CollectionThe following exceptions can be anticipated when calling {@code get()} on the futures obtained from * the returned {@code ElectPreferredLeadersResult}:

diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java index fc42943877445..40343f09bdd99 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java @@ -92,12 +92,10 @@ public ElectPreferredLeadersRequest(Struct struct, short version) { Struct topicPartitionStruct = (Struct) topicPartitionObj; String topicName = topicPartitionStruct.get(TOPIC_NAME); Object[] partitionsArray = topicPartitionStruct.getArray(PARTITIONS_KEY_NAME); - if (partitionsArray != null) { - for (Object partitionObj : partitionsArray) { - Integer partition = (Integer) partitionObj; - TopicPartition topicPartition = new TopicPartition(topicName, partition); - topicPartitions.add(topicPartition); - } + for (Object partitionObj : partitionsArray) { + Integer partition = (Integer) partitionObj; + TopicPartition topicPartition = new TopicPartition(topicName, partition); + topicPartitions.add(topicPartition); } } } else { diff --git a/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java b/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java index a1df4276103fc..f3a7dbd52bf08 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/CollectionUtils.java @@ -19,10 +19,10 @@ import org.apache.kafka.common.TopicPartition; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Collection; import java.util.stream.Collectors; public final class CollectionUtils { 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 210c7e20d128c..efd37eeeea806 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 @@ -642,7 +642,6 @@ public void testElectPreferredLeaders() throws Exception { TopicPartition topic2 = new TopicPartition("topic", 2); try (AdminClientUnitTestEnv env = mockClientEnv()) { env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); - //env.kafkaClient().prepareMetadataUpdate(env.cluster(), Collections.emptySet()); // Test a call where one partition has an error. HashMap map = new HashMap<>(); diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 1c8deba6fe431..a1258089f1807 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -49,7 +49,7 @@ object PreferredReplicaLeaderElectionCommand extends Logging { if(args.length == 0) CommandLineUtils.printUsageAndDie(commandOpts.parser, "This tool causes leadership for each partition to be transferred back to the 'preferred replica'," + " it can be used to balance leadership among the servers." + - " The preferred replica is the first one in the replica assignment (see kafka-reassign-partitions.sh)." + + " The preferred replica is the first one in the replica assignment (see the output from kafka-topics.sh with the --describe option)." + " Using this command is not necessary when the broker is configured with \"auto.leader.rebalance.enable=true\".") CommandLineUtils.checkRequiredArgs(commandOpts.parser, commandOpts.options) @@ -65,8 +65,8 @@ object PreferredReplicaLeaderElectionCommand extends Logging { None val preferredReplicaElectionCommand = if (commandOpts.options.has(commandOpts.zkConnectOpt)) { - Console.err.println(s"$commandOpts.zkConnectOpt is deprecated and will be removed in a future version of Kafka.") - Console.err.println(s"Use $commandOpts.bootstrapServerOpt instead to specify a broker to connect to.") + println(s"Warning: $commandOpts.zkConnectOpt is deprecated and will be removed in a future version of Kafka.") + println(s"Use $commandOpts.bootstrapServerOpt instead to specify a broker to connect to.") new ZkCommand(commandOpts.options.valueOf(commandOpts.zkConnectOpt), JaasUtils.isZkSecurityEnabled, timeout) @@ -153,7 +153,6 @@ object PreferredReplicaLeaderElectionCommand extends Logging { class ZkCommand(zkConnect: String, isSecure: Boolean, timeout: Int) extends Command { - //val zkConnect = options.valueOf(zkConnectOpt) var zkClient: KafkaZkClient = null val time = Time.SYSTEM diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala index 9f0bdb96ef498..ca3abbbc97348 100755 --- a/core/src/main/scala/kafka/cluster/Partition.scala +++ b/core/src/main/scala/kafka/cluster/Partition.scala @@ -627,7 +627,6 @@ class Partition(val topicPartition: TopicPartition, replicaManager.tryCompleteDelayedFetch(requestKey) replicaManager.tryCompleteDelayedProduce(requestKey) replicaManager.tryCompleteDelayedDeleteRecords(requestKey) - replicaManager.tryCompleteElection(requestKey) } def maybeShrinkIsr(replicaMaxLagTimeMs: Long) { diff --git a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala index e76540c669beb..f35ce796a81c1 100755 --- a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala +++ b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala @@ -317,9 +317,11 @@ class PartitionStateMachine(config: KafkaConfig, * Repeatedly attempt to elect leaders for multiple partitions until there are no more remaining partitions to retry. * @param partitions The partitions that we're trying to elect leaders for. * @param partitionLeaderElectionStrategy The election strategy to use. - * @return The partitions that successfully had a leader elected. + * @return A pair with first element of which is the partitions that successfully had a leader elected + * and the second element a map of failed partition to the corresponding thrown exception. */ - private def electLeaderForPartitions(partitions: Seq[TopicPartition], partitionLeaderElectionStrategy: PartitionLeaderElectionStrategy): (Seq[TopicPartition], Map[TopicPartition, Throwable]) = { + private def electLeaderForPartitions(partitions: Seq[TopicPartition], + partitionLeaderElectionStrategy: PartitionLeaderElectionStrategy): (Seq[TopicPartition], Map[TopicPartition, Throwable]) = { val successfulElections = mutable.Buffer.empty[TopicPartition] var remaining = partitions var failures = Map.empty[TopicPartition, Throwable] diff --git a/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala index 085648c4a71fe..43eb600da8808 100644 --- a/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala +++ b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala @@ -17,7 +17,6 @@ package kafka.server -import kafka.controller.ControllerContext import org.apache.kafka.common.TopicPartition import org.apache.kafka.common.protocol.Errors import org.apache.kafka.common.requests.ApiError diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 7302653fe92cc..34a4193cb0310 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -38,6 +38,7 @@ import kafka.security.auth.{Resource, _} import kafka.server.QuotaFactory.{QuotaManagers, UnboundedQuota} import kafka.utils.{CoreUtils, Logging} import kafka.zk.{AdminZkClient, KafkaZkClient} +import org.apache.kafka.common import org.apache.kafka.common.acl.{AccessControlEntry, AclBinding} import org.apache.kafka.common.config.ConfigResource import org.apache.kafka.common.errors._ @@ -2236,7 +2237,7 @@ class KafkaApis(val requestChannel: RequestChannel, val electionRequest = request.body[ElectPreferredLeadersRequest] val partitions = if (electionRequest.topicPartitions() == null) { - zkClient.getAllPartitions() + metadataCache.getAllPartitions() } else { electionRequest.topicPartitions().asScala } diff --git a/core/src/main/scala/kafka/server/MetadataCache.scala b/core/src/main/scala/kafka/server/MetadataCache.scala index 3fefc7b84ef75..258184640b08c 100755 --- a/core/src/main/scala/kafka/server/MetadataCache.scala +++ b/core/src/main/scala/kafka/server/MetadataCache.scala @@ -137,6 +137,15 @@ class MetadataCache(brokerId: Int) extends Logging { getAllTopics(metadataSnapshot) } + def getAllPartitions(): Set[TopicPartition] = { + metadataSnapshot.partitionStates.foldLeft(Set.empty[TopicPartition]) { case (result, namePartitionsAndStates) => { + val topicName = namePartitionsAndStates._1 + val partitionsAndStates = namePartitionsAndStates._2 + result ++ partitionsAndStates.keys.map(partitionId => new TopicPartition(topicName, partitionId.toInt)) + } + } + } + private def getAllTopics(snapshot: MetadataSnapshot): Set[String] = { snapshot.partitionStates.keySet } diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 9ee6d7aa48262..9cfdd205aebdf 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1521,16 +1521,16 @@ class ReplicaManager(val config: KafkaConfig, } def electPreferredLeaders(controller: KafkaController, - partitions: Set[TopicPartition], - responseCallback: Map[TopicPartition, ApiError] => Unit, - requestTimeout: Long): Unit = { + partitions: Set[TopicPartition], + responseCallback: Map[TopicPartition, ApiError] => Unit, + requestTimeout: Long): Unit = { val (validPartitions, invalidPartitions) = partitions.partition(tp => metadataCache.contains(tp)) val invalidPartitionsResults = invalidPartitions.map { p => val msg = s"Skipping preferred replica leader election for partition ${p} since it doesn't exist." logger.info(msg) - p -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition '$p' does not exist.") + p -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition does not exist.") }.toMap def electionCallback(waiting: Set[TopicPartition], results: Map[TopicPartition, ApiError]) = { diff --git a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala index facf88e914a2d..dd183e39b3a02 100644 --- a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala @@ -1302,7 +1302,7 @@ class AdminClientIntegrationTest extends IntegrationTestHarness with Logging { case e: Exception => val cause = e.getCause assertTrue(cause.isInstanceOf[UnknownTopicOrPartitionException]) - assertEquals("The partition 'topic-does-not-exist-0' does not exist.", + assertEquals("The partition does not exist.", cause.getMessage) assertEquals(1, currentLeader(partition1)) assertEquals(1, currentLeader(partition2)) @@ -1322,7 +1322,7 @@ class AdminClientIntegrationTest extends IntegrationTestHarness with Logging { case e: Exception => val cause = e.getCause assertTrue(cause.isInstanceOf[UnknownTopicOrPartitionException]) - assertEquals("The partition 'topic-does-not-exist-0' does not exist.", + assertEquals("The partition does not exist.", cause.getMessage) } diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index 871de3e34e35c..dd1cfbe753a57 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -62,11 +62,6 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit // create brokers servers = brokerConfigs.map(b => TestUtils.createServer(KafkaConfig.fromProps(b))) // create the topic - //zkClient.createPartitionReassignment(partitionsAndAssignments) -// partitionsAndAssignments.foreach { partitionAndAssignment => -// zkClient.createPartitionReassignment()OrUpdateTopicPartitionAssignmentPathInZK(partitionAndAssignment._1.topic(), -// Map(partitionAndAssignment._1.partition -> partitionAndAssignment._2)) -// } partitionsAndAssignments.foreach { partitionAndAssignment => zkClient.createTopicAssignment(partitionAndAssignment._1.topic(), Map(partitionAndAssignment._1 -> partitionAndAssignment._2)) From b20c3a20b46df06fdbfaedcb0f1e343bc00cc453 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Tue, 8 Jan 2019 10:48:19 +0000 Subject: [PATCH 04/22] Don't leak partitions to unauthz clients --- core/src/main/scala/kafka/server/KafkaApis.scala | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 34a4193cb0310..2eb5b33c1d1de 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -2246,7 +2246,15 @@ class KafkaApis(val requestChannel: RequestChannel, new ElectPreferredLeadersResponse(requestThrottleMs, result.asJava)) } if (!authorize(request.session, Alter, Resource.ClusterResource)) { - sendResponseCallback(partitions.map(partition => partition -> new ApiError(Errors.CLUSTER_AUTHORIZATION_FAILED, null)).toMap) + val error = new ApiError(Errors.CLUSTER_AUTHORIZATION_FAILED, null); + val partitionErrors = + if (electionRequest.topicPartitions() == null) { + // Don't leak the set of partitions if the client lack authz + Map(new TopicPartition(null, -1) -> error) + } else { + partitions.map(partition => partition -> error).toMap + } + sendResponseCallback(partitionErrors) } else { replicaManager.electPreferredLeaders(controller, partitions, sendResponseCallback, electionRequest.timeout) } From 5b38fc1c68f871d1d9311a6528795a26b6733cb6 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Tue, 8 Jan 2019 16:00:53 +0000 Subject: [PATCH 05/22] Tidy up PSM --- .../kafka/controller/KafkaController.scala | 2 +- .../controller/PartitionStateMachine.scala | 40 +++++-------------- 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 8513bf062450d..2ea3d9906148f 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -633,7 +633,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti newPath: Boolean = false): Map[TopicPartition, Throwable] = { info(s"Starting preferred replica leader election for partitions ${partitions.mkString(",")}") try { - val results = partitionStateMachine.handleStateChangesWithResults(partitions.toSeq, OnlinePartition, + val results = partitionStateMachine.handleStateChanges(partitions.toSeq, OnlinePartition, Option(PreferredReplicaPartitionLeaderElectionStrategy)) if (!newPath) { results.foreach(entry => diff --git a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala index f35ce796a81c1..7d0ac4567e178 100755 --- a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala +++ b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala @@ -126,49 +126,27 @@ class PartitionStateMachine(config: KafkaConfig, } def handleStateChanges(partitions: Seq[TopicPartition], targetState: PartitionState, - partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy] = None): Unit = { - if (partitions.nonEmpty) { + partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy] = None): Map[TopicPartition, Throwable] = { + return if (partitions.nonEmpty) { try { controllerBrokerRequestBatch.newBatch() - doHandleStateChanges(partitions, targetState, partitionLeaderElectionStrategyOpt) + val errors = doHandleStateChanges(partitions, targetState, partitionLeaderElectionStrategyOpt) controllerBrokerRequestBatch.sendRequestsToBrokers(controllerContext.epoch) + errors } catch { case e: ControllerMovedException => error(s"Controller moved to another broker when moving some partitions to $targetState state", e) throw e - case e: Throwable => error(s"Error while moving some partitions to $targetState state", e) - } - } - } - - /** - * Attempt to put each of the given partitions into the given targetState, returning - * a map of partition to exception for those partitions where the attempt failed with an exception. - * @param partitions The list of partitions that need to be transitioned to the target state - * @param targetState The state that the partitions should be moved to - */ - def handleStateChangesWithResults(partitions: Seq[TopicPartition], targetState: PartitionState, - partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy]): Map[TopicPartition, Throwable] = { - info(s"Invoking state change to $targetState for partitions ${partitions.mkString(",")}") - var result = Map.empty[TopicPartition, Throwable] - if (partitions.nonEmpty) { - try { - controllerBrokerRequestBatch.newBatch() - partitions.foreach { partition => - result ++= doHandleStateChanges(Seq(partition), targetState, partitionLeaderElectionStrategyOpt) - } - controllerBrokerRequestBatch.sendRequestsToBrokers(controllerContext.epoch) - } catch { case e: Throwable => - error(s"Error changing state of some partitions to $targetState state", e) - result ++= (partitions.toSet -- result.keySet).map { - _ -> e - } + error(s"Error while moving some partitions to $targetState state", e) + partitions.map { _ -> e }.toMap } + } else { + Map.empty[TopicPartition, Throwable] } - result } + def partitionsInState(state: PartitionState): Set[TopicPartition] = { partitionState.filter { case (_, s) => s == state }.keySet.toSet } From bf1226d42afb2139387c3e86f68dd6041377c0b3 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Wed, 9 Jan 2019 11:13:39 +0000 Subject: [PATCH 06/22] Use empty errors map to indicate cluster auth failure --- .../kafka/clients/admin/ElectPreferredLeadersResult.java | 6 ++++++ core/src/main/scala/kafka/server/KafkaApis.scala | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java index 14ebfb01df98e..c76336a7cbc0a 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/ElectPreferredLeadersResult.java @@ -23,6 +23,7 @@ import org.apache.kafka.common.errors.ApiException; import org.apache.kafka.common.errors.UnknownTopicOrPartitionException; import org.apache.kafka.common.internals.KafkaFutureImpl; +import org.apache.kafka.common.protocol.Errors; import org.apache.kafka.common.requests.ApiError; import java.util.Collection; @@ -62,6 +63,9 @@ public void accept(Map topicPartitions, Throwable thro "Preferred leader election for partition \"" + partition + "\" was not attempted")); } else { + if (partitions == null && topicPartitions.isEmpty()) { + result.completeExceptionally(Errors.CLUSTER_AUTHORIZATION_FAILED.exception()); + } ApiException exception = topicPartitions.get(partition).exception(); if (exception == null) { result.complete(null); @@ -93,6 +97,8 @@ public KafkaFuture> partitions() { public void accept(Map topicPartitions, Throwable throwable) { if (throwable != null) { result.completeExceptionally(throwable); + } else if (topicPartitions.isEmpty()) { + result.completeExceptionally(Errors.CLUSTER_AUTHORIZATION_FAILED.exception()); } else { for (ApiError apiError : topicPartitions.values()) { if (apiError.isFailure()) { diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 2eb5b33c1d1de..c2b87beba9406 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -2250,7 +2250,7 @@ class KafkaApis(val requestChannel: RequestChannel, val partitionErrors = if (electionRequest.topicPartitions() == null) { // Don't leak the set of partitions if the client lack authz - Map(new TopicPartition(null, -1) -> error) + Map.empty[TopicPartition, ApiError] } else { partitions.map(partition => partition -> error).toMap } From 81670d08ccb70bd6ba7ca6cc56d04b6f59da1db5 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Wed, 9 Jan 2019 14:56:38 +0000 Subject: [PATCH 07/22] Support loading admin client config from file, or options --- ...referredReplicaLeaderElectionCommand.scala | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index a1258089f1807..676b38adbd2c9 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -65,13 +65,20 @@ object PreferredReplicaLeaderElectionCommand extends Logging { None val preferredReplicaElectionCommand = if (commandOpts.options.has(commandOpts.zkConnectOpt)) { - println(s"Warning: $commandOpts.zkConnectOpt is deprecated and will be removed in a future version of Kafka.") - println(s"Use $commandOpts.bootstrapServerOpt instead to specify a broker to connect to.") + println(s"Warning: --zookeeper is deprecated and will be removed in a future version of Kafka.") + println(s"Use --bootstrap-server instead to specify a broker to connect to.") new ZkCommand(commandOpts.options.valueOf(commandOpts.zkConnectOpt), JaasUtils.isZkSecurityEnabled, timeout) } else { - new AdminClientCommand(commandOpts.options.valueOf(commandOpts.bootstrapServerOpt), timeout) + val adminProps = if (commandOpts.options.has(commandOpts.adminClientConfigOpt)) + Utils.loadProps(commandOpts.options.valueOf(commandOpts.adminClientConfigOpt)) + else + new Properties() + adminProps.putAll(CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala)) + adminProps.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, commandOpts.options.valueOf(commandOpts.bootstrapServerOpt)) + adminProps.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) + new AdminClientCommand(adminProps) } try { @@ -124,11 +131,12 @@ object PreferredReplicaLeaderElectionCommand extends Logging { .describedAs("list of partitions for which preferred replica leader election needs to be triggered") .ofType(classOf[String]) - private val zookeeperOptBuilder: OptionSpecBuilder = parser.accepts("zookeeper", "The connection string for the zookeeper connection in the " + + private val zookeeperOptBuilder: OptionSpecBuilder = parser.accepts("zookeeper", + "DEPRECATED. The connection string for the zookeeper connection in the " + "form host:port. Multiple URLS can be given to allow fail-over. " + - "DEPRECATED, replaced by --bootstrap-server, REQUIRED unless --bootstrap-server is given.") - private val bootstrapOptBuilder: OptionSpecBuilder = parser - .accepts("bootstrap-server", "A hostname and port for the broker to connect to, " + + "Replaced by --bootstrap-server, REQUIRED unless --bootstrap-server is given.") + private val bootstrapOptBuilder: OptionSpecBuilder = parser.accepts("bootstrap-server", + "A hostname and port for the broker to connect to, " + "in the form host:port. Multiple comma-separated URLs can be given. REQUIRED unless --zookeeper is given.") parser.mutuallyExclusive(zookeeperOptBuilder, bootstrapOptBuilder) val bootstrapServerOpt = bootstrapOptBuilder @@ -139,6 +147,22 @@ object PreferredReplicaLeaderElectionCommand extends Logging { .withRequiredArg .describedAs("urls") .ofType(classOf[String]) + + val adminClientPropertyOpt = parser.accepts("admin-property", + "User-defined properties in the form key=value to pass to the admin client when --bootstrap-server is given") + .availableIf(bootstrapServerOpt) + .withRequiredArg + .describedAs("consumer_prop") + .ofType(classOf[String]) + val adminClientConfigOpt = parser.accepts("admin.config", + "Admin client config properties file to pass to the admin client when --bootstrap-server is given. " + + "Note that --admin-property takes precedence over this config.") + .availableIf(bootstrapServerOpt) + .withRequiredArg + .describedAs("config file") + .ofType(classOf[String]) + + parser.accepts("") options = parser.parse(args: _*) } @@ -195,13 +219,10 @@ object PreferredReplicaLeaderElectionCommand extends Logging { } /** Election via AdminClient.electPreferredLeaders() */ - class AdminClientCommand(bootstrapServers: String, timeout: Int) + class AdminClientCommand(adminClientProps: Properties) extends Command with Logging { - val props = new Properties() - props.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, bootstrapServers) - props.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) - val adminClient = org.apache.kafka.clients.admin.AdminClient.create(props) + val adminClient = org.apache.kafka.clients.admin.AdminClient.create(adminClientProps) /** * Wait until the given future has completed, then return whether it completed exceptionally. From f5973ba87b8e3d4c555b9403472882490d8294a4 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Thu, 10 Jan 2019 15:17:15 +0000 Subject: [PATCH 08/22] wip --- core/src/main/scala/kafka/controller/KafkaController.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 2ea3d9906148f..ddcec1d106db2 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -268,7 +268,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti maybeTriggerPartitionReassignment(controllerContext.partitionsBeingReassigned.keySet) topicDeletionManager.tryTopicDeletion() val pendingPreferredReplicaElections = fetchPendingPreferredReplicaElections() - onPreferredReplicaElection(pendingPreferredReplicaElections) + onPreferredReplicaElection(pendingPreferredReplicaElections, false, false) info("Starting the controller scheduler") kafkaScheduler.startup() if (config.autoLeaderRebalanceEnable) { @@ -995,7 +995,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti controllerContext.partitionsBeingReassigned.isEmpty && !topicDeletionManager.isTopicQueuedUpForDeletion(tp.topic) && controllerContext.allTopics.contains(tp.topic)) - onPreferredReplicaElection(candidatePartitions.toSet, isTriggeredByAutoRebalance = true) + onPreferredReplicaElection(candidatePartitions.toSet, isTriggeredByAutoRebalance = true, newPath = false) } } } @@ -1566,7 +1566,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti currentLeader != preferredReplica } - val electionErrors = onPreferredReplicaElection(electablePartitions, newPath) + val electionErrors = onPreferredReplicaElection(electablePartitions, false, newPath) val successfulPartitions = electablePartitions -- electionErrors.keySet val results = electionErrors.map { case (partition, ex) => val apiError = if (ex.isInstanceOf[StateChangeFailedException]) From dd0e146c30758724822f0ea8238051e92eda8613 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Thu, 10 Jan 2019 17:18:51 +0000 Subject: [PATCH 09/22] Use case classes --- .../kafka/controller/KafkaController.scala | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index ddcec1d106db2..0c64afe1b1a82 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -268,7 +268,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti maybeTriggerPartitionReassignment(controllerContext.partitionsBeingReassigned.keySet) topicDeletionManager.tryTopicDeletion() val pendingPreferredReplicaElections = fetchPendingPreferredReplicaElections() - onPreferredReplicaElection(pendingPreferredReplicaElections, false, false) + onPreferredReplicaElection(pendingPreferredReplicaElections, zkTriggered) info("Starting the controller scheduler") kafkaScheduler.startup() if (config.autoLeaderRebalanceEnable) { @@ -628,14 +628,18 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti removePartitionsFromReassignedPartitions(partitionsToBeRemovedFromReassignment) } + private sealed trait ElectionType + private object autoTriggered extends ElectionType + private object zkTriggered extends ElectionType + private object adminClientTriggered extends ElectionType + private def onPreferredReplicaElection(partitions: Set[TopicPartition], - isTriggeredByAutoRebalance: Boolean = false, - newPath: Boolean = false): Map[TopicPartition, Throwable] = { + electionType: ElectionType): Map[TopicPartition, Throwable] = { info(s"Starting preferred replica leader election for partitions ${partitions.mkString(",")}") try { val results = partitionStateMachine.handleStateChanges(partitions.toSeq, OnlinePartition, Option(PreferredReplicaPartitionLeaderElectionStrategy)) - if (!newPath) { + if (electionType != adminClientTriggered) { results.foreach(entry => if (entry._2.isInstanceOf[ControllerMovedException]) { error(s"Error completing preferred replica leader election for partition ${entry._1} because controller has moved to another broker.", entry._2) @@ -647,8 +651,8 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } return results; } finally { - if (!newPath) { - removePartitionsFromPreferredReplicaElection(partitions, isTriggeredByAutoRebalance) + if (electionType != adminClientTriggered) { + removePartitionsFromPreferredReplicaElection(partitions, electionType == autoTriggered) } } } @@ -995,7 +999,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti controllerContext.partitionsBeingReassigned.isEmpty && !topicDeletionManager.isTopicQueuedUpForDeletion(tp.topic) && controllerContext.allTopics.contains(tp.topic)) - onPreferredReplicaElection(candidatePartitions.toSet, isTriggeredByAutoRebalance = true, newPath = false) + onPreferredReplicaElection(candidatePartitions.toSet, autoTriggered) } } } @@ -1566,7 +1570,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti currentLeader != preferredReplica } - val electionErrors = onPreferredReplicaElection(electablePartitions, false, newPath) + val electionErrors = onPreferredReplicaElection(electablePartitions, if (newPath) adminClientTriggered else zkTriggered) val successfulPartitions = electablePartitions -- electionErrors.keySet val results = electionErrors.map { case (partition, ex) => val apiError = if (ex.isInstanceOf[StateChangeFailedException]) From 77d043e77f7bb58e721d6627ef73abb3b45204c8 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Fri, 11 Jan 2019 09:34:46 +0000 Subject: [PATCH 10/22] Fix compiler error? --- .../kafka/admin/PreferredReplicaLeaderElectionCommand.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 676b38adbd2c9..127e3c0a7b363 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -75,7 +75,7 @@ object PreferredReplicaLeaderElectionCommand extends Logging { Utils.loadProps(commandOpts.options.valueOf(commandOpts.adminClientConfigOpt)) else new Properties() - adminProps.putAll(CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala)) + adminProps.putAll(CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala).asInstanceOf[java.util.Map[_ <: Object, _ <: Object]]) adminProps.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, commandOpts.options.valueOf(commandOpts.bootstrapServerOpt)) adminProps.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) new AdminClientCommand(adminProps) From d8bf6a90a48455945e862bdf0e7b7aff0b41e85f Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Fri, 11 Jan 2019 12:26:07 +0000 Subject: [PATCH 11/22] Avoid https://github.com/scala/bug/issues/10418 using a loop --- .../kafka/admin/PreferredReplicaLeaderElectionCommand.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 127e3c0a7b363..e19f7f7f3ec42 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -75,7 +75,9 @@ object PreferredReplicaLeaderElectionCommand extends Logging { Utils.loadProps(commandOpts.options.valueOf(commandOpts.adminClientConfigOpt)) else new Properties() - adminProps.putAll(CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala).asInstanceOf[java.util.Map[_ <: Object, _ <: Object]]) + for ((k, v) <- CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala).asScala) { + adminProps.setProperty(k, v) + } adminProps.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, commandOpts.options.valueOf(commandOpts.bootstrapServerOpt)) adminProps.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) new AdminClientCommand(adminProps) From 96f682bf9642df95d1a28b7a0c7a973344f94594 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Fri, 11 Jan 2019 13:52:08 +0000 Subject: [PATCH 12/22] Use implicit instead --- .../kafka/admin/PreferredReplicaLeaderElectionCommand.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index e19f7f7f3ec42..9a79ec7b69a7a 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -22,6 +22,7 @@ import java.util.concurrent.ExecutionException import joptsimple.OptionSpecBuilder import kafka.common.AdminCommandFailedException import kafka.utils._ +import kafka.utils.Implicits._ import kafka.zk.KafkaZkClient import org.apache.kafka.clients.admin.AdminClientConfig import org.apache.kafka.common.errors.TimeoutException @@ -75,9 +76,7 @@ object PreferredReplicaLeaderElectionCommand extends Logging { Utils.loadProps(commandOpts.options.valueOf(commandOpts.adminClientConfigOpt)) else new Properties() - for ((k, v) <- CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala).asScala) { - adminProps.setProperty(k, v) - } + adminProps ++= CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala) adminProps.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, commandOpts.options.valueOf(commandOpts.bootstrapServerOpt)) adminProps.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) new AdminClientCommand(adminProps) From 0d5aa7bc0a93b5dfe62588cc0c1a1e562d231bc4 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 14 Jan 2019 09:52:40 +0000 Subject: [PATCH 13/22] Review comments 2 --- .../kafka/clients/admin/AdminClient.java | 4 +- .../PreferredLeaderNotAvailableException.java | 28 +++++++++ .../apache/kafka/common/protocol/Errors.java | 5 +- .../clients/admin/KafkaAdminClientTest.java | 2 +- ...referredReplicaLeaderElectionCommand.scala | 19 +------ .../kafka/controller/KafkaController.scala | 57 ++++++++++++------- .../controller/PartitionStateMachine.scala | 10 +++- .../scala/kafka/server/MetadataCache.scala | 9 +-- .../scala/kafka/server/ReplicaManager.scala | 17 ++---- .../api/AdminClientIntegrationTest.scala | 12 ++++ ...rredReplicaLeaderElectionCommandTest.scala | 51 ++++++++--------- 11 files changed, 125 insertions(+), 89 deletions(-) create mode 100644 clients/src/main/java/org/apache/kafka/common/errors/PreferredLeaderNotAvailableException.java diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java index e6c09b51c19a2..a2426982a2af4 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java @@ -800,7 +800,7 @@ public DeleteConsumerGroupsResult deleteConsumerGroups(Collection groupI * with default options. * See the overload for more details. * - * @param partitions The partitions for which the the preferred leader should be elected. + * @param partitions The partitions for which the preferred leader should be elected. * @return The ElectPreferredLeadersResult. */ public ElectPreferredLeadersResult electPreferredLeaders(Collection partitions) { @@ -837,7 +837,7 @@ public ElectPreferredLeadersResult electPreferredLeaders(Collection * * - * @param partitions The partitions for which the the preferred leader should be elected. + * @param partitions The partitions for which the preferred leader should be elected. * @param options The options to use when electing the preferred leaders. * @return The ElectPreferredLeadersResult. */ diff --git a/clients/src/main/java/org/apache/kafka/common/errors/PreferredLeaderNotAvailableException.java b/clients/src/main/java/org/apache/kafka/common/errors/PreferredLeaderNotAvailableException.java new file mode 100644 index 0000000000000..73dfd64ac6d23 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/common/errors/PreferredLeaderNotAvailableException.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.common.errors; + +public class PreferredLeaderNotAvailableException extends InvalidMetadataException { + + public PreferredLeaderNotAvailableException(String message) { + super(message); + } + + public PreferredLeaderNotAvailableException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/clients/src/main/java/org/apache/kafka/common/protocol/Errors.java b/clients/src/main/java/org/apache/kafka/common/protocol/Errors.java index 14ca06da8e463..5bcff4363dc01 100644 --- a/clients/src/main/java/org/apache/kafka/common/protocol/Errors.java +++ b/clients/src/main/java/org/apache/kafka/common/protocol/Errors.java @@ -72,6 +72,7 @@ import org.apache.kafka.common.errors.OperationNotAttemptedException; import org.apache.kafka.common.errors.OutOfOrderSequenceException; import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.common.errors.PreferredLeaderNotAvailableException; import org.apache.kafka.common.errors.ProducerFencedException; import org.apache.kafka.common.errors.ReassignmentInProgressException; import org.apache.kafka.common.errors.RebalanceInProgressException; @@ -297,7 +298,9 @@ public enum Errors { "election so the offsets cannot be guaranteed to be monotonically increasing", OffsetNotAvailableException::new), MEMBER_ID_REQUIRED(79, "The group member needs to have a valid member id before actually entering a consumer group", - MemberIdRequiredException::new); + MemberIdRequiredException::new), + PREFERRED_LEADER_NOT_AVAILABLE(80, "The preferred leader was not available", + PreferredLeaderNotAvailableException::new); private static final Logger log = LoggerFactory.getLogger(Errors.class); 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 efd37eeeea806..67121229ad6f1 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 @@ -665,7 +665,7 @@ public void testElectPreferredLeaders() throws Exception { results.partitionResult(topic2).get(); // Now try a timeout - results = env.adminClient().electPreferredLeaders(asList(topic1, topic2), new ElectPreferredLeadersOptions().timeoutMs(2000)); + results = env.adminClient().electPreferredLeaders(asList(topic1, topic2), new ElectPreferredLeadersOptions().timeoutMs(100)); TestUtils.assertFutureError(results.partitionResult(topic1), TimeoutException.class); TestUtils.assertFutureError(results.partitionResult(topic2), TimeoutException.class); } diff --git a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala index 9a79ec7b69a7a..8740ed45b89c5 100755 --- a/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala +++ b/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala @@ -22,7 +22,6 @@ import java.util.concurrent.ExecutionException import joptsimple.OptionSpecBuilder import kafka.common.AdminCommandFailedException import kafka.utils._ -import kafka.utils.Implicits._ import kafka.zk.KafkaZkClient import org.apache.kafka.clients.admin.AdminClientConfig import org.apache.kafka.common.errors.TimeoutException @@ -47,12 +46,6 @@ object PreferredReplicaLeaderElectionCommand extends Logging { CommandLineUtils.printHelpAndExitIfNeeded(commandOpts, "This tool helps to causes leadership for each partition to be transferred back to the 'preferred replica'," + " it can be used to balance leadership among the servers.") - if(args.length == 0) - CommandLineUtils.printUsageAndDie(commandOpts.parser, "This tool causes leadership for each partition to be transferred back to the 'preferred replica'," + - " it can be used to balance leadership among the servers." + - " The preferred replica is the first one in the replica assignment (see the output from kafka-topics.sh with the --describe option)." + - " Using this command is not necessary when the broker is configured with \"auto.leader.rebalance.enable=true\".") - CommandLineUtils.checkRequiredArgs(commandOpts.parser, commandOpts.options) if (commandOpts.options.has(commandOpts.bootstrapServerOpt) == commandOpts.options.has(commandOpts.zkConnectOpt)) { @@ -76,7 +69,6 @@ object PreferredReplicaLeaderElectionCommand extends Logging { Utils.loadProps(commandOpts.options.valueOf(commandOpts.adminClientConfigOpt)) else new Properties() - adminProps ++= CommandLineUtils.parseKeyValueArgs(commandOpts.options.valuesOf(commandOpts.adminClientPropertyOpt).asScala) adminProps.setProperty(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, commandOpts.options.valueOf(commandOpts.bootstrapServerOpt)) adminProps.setProperty(AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, timeout.toString) new AdminClientCommand(adminProps) @@ -149,15 +141,8 @@ object PreferredReplicaLeaderElectionCommand extends Logging { .describedAs("urls") .ofType(classOf[String]) - val adminClientPropertyOpt = parser.accepts("admin-property", - "User-defined properties in the form key=value to pass to the admin client when --bootstrap-server is given") - .availableIf(bootstrapServerOpt) - .withRequiredArg - .describedAs("consumer_prop") - .ofType(classOf[String]) val adminClientConfigOpt = parser.accepts("admin.config", - "Admin client config properties file to pass to the admin client when --bootstrap-server is given. " + - "Note that --admin-property takes precedence over this config.") + "Admin client config properties file to pass to the admin client when --bootstrap-server is given.") .availableIf(bootstrapServerOpt) .withRequiredArg .describedAs("config file") @@ -272,7 +257,7 @@ object PreferredReplicaLeaderElectionCommand extends Logging { partition { case (_, partitionResult) => completedExceptionally(partitionResult) } if (!ok.isEmpty) { - println(s"Successfully completed preferred replica election for partitions ${ok.map(_._1).mkString(", ")}") + println(s"Successfully completed preferred replica election for partitions ${ok.map{ case (tp, future) => tp }.mkString(", ")}") } if (!exceptional.isEmpty) { val adminException = new AdminCommandFailedException( diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 0c64afe1b1a82..be7758757fd62 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -268,7 +268,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti maybeTriggerPartitionReassignment(controllerContext.partitionsBeingReassigned.keySet) topicDeletionManager.tryTopicDeletion() val pendingPreferredReplicaElections = fetchPendingPreferredReplicaElections() - onPreferredReplicaElection(pendingPreferredReplicaElections, zkTriggered) + onPreferredReplicaElection(pendingPreferredReplicaElections, ZkTriggered) info("Starting the controller scheduler") kafkaScheduler.startup() if (config.autoLeaderRebalanceEnable) { @@ -628,31 +628,38 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti removePartitionsFromReassignedPartitions(partitionsToBeRemovedFromReassignment) } - private sealed trait ElectionType - private object autoTriggered extends ElectionType - private object zkTriggered extends ElectionType - private object adminClientTriggered extends ElectionType + sealed trait ElectionType + object AutoTriggered extends ElectionType + object ZkTriggered extends ElectionType + object AdminClientTriggered extends ElectionType + /** + * Attempt to elect the preferred replica as leader for each of the given partitions. + * @param partitions The partitions to have their preferred leader elected + * @param electionType The election type + * @return A map of failed elections where keys are partitions which had an error and the corresponding value is + * the exception that way thrown. + */ private def onPreferredReplicaElection(partitions: Set[TopicPartition], electionType: ElectionType): Map[TopicPartition, Throwable] = { info(s"Starting preferred replica leader election for partitions ${partitions.mkString(",")}") try { val results = partitionStateMachine.handleStateChanges(partitions.toSeq, OnlinePartition, Option(PreferredReplicaPartitionLeaderElectionStrategy)) - if (electionType != adminClientTriggered) { - results.foreach(entry => - if (entry._2.isInstanceOf[ControllerMovedException]) { - error(s"Error completing preferred replica leader election for partition ${entry._1} because controller has moved to another broker.", entry._2) - throw entry._2 + if (electionType != AdminClientTriggered) { + results.foreach { case (tp, throwable) => + if (throwable.isInstanceOf[ControllerMovedException]) { + error(s"Error completing preferred replica leader election for partition $tp because controller has moved to another broker.", throwable) + throw throwable } else { - error(s"Error completing preferred replica leader election for partition ${entry._1}", entry._2) + error(s"Error completing preferred replica leader election for partition $tp", throwable) } - ) + } } return results; } finally { - if (electionType != adminClientTriggered) { - removePartitionsFromPreferredReplicaElection(partitions, electionType == autoTriggered) + if (electionType != AdminClientTriggered) { + removePartitionsFromPreferredReplicaElection(partitions, electionType == AutoTriggered) } } } @@ -999,7 +1006,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti controllerContext.partitionsBeingReassigned.isEmpty && !topicDeletionManager.isTopicQueuedUpForDeletion(tp.topic) && controllerContext.allTopics.contains(tp.topic)) - onPreferredReplicaElection(candidatePartitions.toSet, autoTriggered) + onPreferredReplicaElection(candidatePartitions.toSet, AutoTriggered) } } } @@ -1534,10 +1541,11 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } def electPreferredLeaders(partitions: Set[TopicPartition], callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = { (_,_) => }): Unit = - eventManager.put(PreferredReplicaLeaderElection(Some(partitions), true, callback)) + eventManager.put(PreferredReplicaLeaderElection(Some(partitions), AdminClientTriggered, callback)) case class PreferredReplicaLeaderElection(partitionsOpt: Option[Set[TopicPartition]], - newPath: Boolean = false, + //newPath: Boolean = false, + electionType: ElectionType = ZkTriggered, callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends ControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance @@ -1550,13 +1558,18 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } else { // We need to register the watcher if the path doesn't exist in order to detect future preferred replica // leader elections and we get the `path exists` check for free - if (newPath || zkClient.registerZNodeChangeHandlerAndCheckExistence(preferredReplicaElectionHandler)) { + if (electionType == AdminClientTriggered || zkClient.registerZNodeChangeHandlerAndCheckExistence(preferredReplicaElectionHandler)) { val partitions = partitionsOpt match { case Some(partitions) => partitions case None => zkClient.getPreferredReplicaElection } - val (partitionsBeingDeleted, livePartitions) = partitions.partition(partition => + val (validPartitions, invalidPartitions) = partitions.partition(tp => controllerContext.allPartitions.contains(tp)) + invalidPartitions.foreach { p => + info(s"Skipping preferred replica leader election for partition ${p} since it doesn't exist.") + } + + val (partitionsBeingDeleted, livePartitions) = validPartitions.partition(partition => topicDeletionManager.isTopicQueuedUpForDeletion(partition.topic)) if (partitionsBeingDeleted.nonEmpty) { error(s"Skipping preferred replica election for partitions $partitionsBeingDeleted " + @@ -1570,7 +1583,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti currentLeader != preferredReplica } - val electionErrors = onPreferredReplicaElection(electablePartitions, if (newPath) adminClientTriggered else zkTriggered) + val electionErrors = onPreferredReplicaElection(electablePartitions, electionType) val successfulPartitions = electablePartitions -- electionErrors.keySet val results = electionErrors.map { case (partition, ex) => val apiError = if (ex.isInstanceOf[StateChangeFailedException]) @@ -1580,7 +1593,9 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti partition -> apiError } ++ alreadyPreferred.map(_ -> ApiError.NONE) ++ - partitionsBeingDeleted.map(_ -> new ApiError(Errors.INVALID_TOPIC_EXCEPTION, "The topic is being deleted")) + partitionsBeingDeleted.map(_ -> new ApiError(Errors.INVALID_TOPIC_EXCEPTION, "The topic is being deleted")) ++ + invalidPartitions.map ( tp => tp -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition does not exist.") + ) debug(s"PreferredReplicaLeaderElection waiting: $successfulPartitions, results: $results") callback(successfulPartitions, results) } diff --git a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala index 7d0ac4567e178..ad739797179c0 100755 --- a/core/src/main/scala/kafka/controller/PartitionStateMachine.scala +++ b/core/src/main/scala/kafka/controller/PartitionStateMachine.scala @@ -125,9 +125,17 @@ class PartitionStateMachine(config: KafkaConfig, // It is important to trigger leader election for those partitions. } + /** + * Try to change the state of the given partitions to the given targetState, using the given + * partitionLeaderElectionStrategyOpt if a leader election is required. + * @param partitions The partitions + * @param targetState The state + * @param partitionLeaderElectionStrategyOpt The leader election strategy if a leader election is required. + * @return partitions and corresponding throwable for those partitions which could not transition to the given state + */ def handleStateChanges(partitions: Seq[TopicPartition], targetState: PartitionState, partitionLeaderElectionStrategyOpt: Option[PartitionLeaderElectionStrategy] = None): Map[TopicPartition, Throwable] = { - return if (partitions.nonEmpty) { + if (partitions.nonEmpty) { try { controllerBrokerRequestBatch.newBatch() val errors = doHandleStateChanges(partitions, targetState, partitionLeaderElectionStrategyOpt) diff --git a/core/src/main/scala/kafka/server/MetadataCache.scala b/core/src/main/scala/kafka/server/MetadataCache.scala index 258184640b08c..ec5a2b9ed38b1 100755 --- a/core/src/main/scala/kafka/server/MetadataCache.scala +++ b/core/src/main/scala/kafka/server/MetadataCache.scala @@ -138,12 +138,9 @@ class MetadataCache(brokerId: Int) extends Logging { } def getAllPartitions(): Set[TopicPartition] = { - metadataSnapshot.partitionStates.foldLeft(Set.empty[TopicPartition]) { case (result, namePartitionsAndStates) => { - val topicName = namePartitionsAndStates._1 - val partitionsAndStates = namePartitionsAndStates._2 - result ++ partitionsAndStates.keys.map(partitionId => new TopicPartition(topicName, partitionId.toInt)) - } - } + metadataSnapshot.partitionStates.flatMap { case (topicName, partitionsAndStates) => + partitionsAndStates.keys.map(partitionId => new TopicPartition(topicName, partitionId.toInt)) + }.toSet } private def getAllTopics(snapshot: MetadataSnapshot): Set[String] = { diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 9cfdd205aebdf..78b6e0ea7a416 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1525,31 +1525,24 @@ class ReplicaManager(val config: KafkaConfig, responseCallback: Map[TopicPartition, ApiError] => Unit, requestTimeout: Long): Unit = { - val (validPartitions, invalidPartitions) = partitions.partition(tp => metadataCache.contains(tp)) + val deadline = time.milliseconds() + requestTimeout - val invalidPartitionsResults = invalidPartitions.map { p => - val msg = s"Skipping preferred replica leader election for partition ${p} since it doesn't exist." - logger.info(msg) - p -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition does not exist.") - }.toMap - - def electionCallback(waiting: Set[TopicPartition], results: Map[TopicPartition, ApiError]) = { + def electionCallback(waiting: Set[TopicPartition], results: Map[TopicPartition, ApiError]): Unit = { if (waiting.nonEmpty) { - // timeout val expectedLeaders = waiting.map( tp => ElectPreferredLeaderMetadata(tp, controller.controllerContext.partitionReplicaAssignment(tp).head)) val watchKeys = waiting.map(p => new TopicPartitionOperationKey(p.topic, p.partition)).toSeq delayedElectPreferredLeaderPurgatory.tryCompleteElseWatch( - new DelayedElectPreferredLeader(requestTimeout, expectedLeaders, results ++ invalidPartitionsResults, + new DelayedElectPreferredLeader(deadline - time.milliseconds(), expectedLeaders, results, this, responseCallback), watchKeys) } else { // There are no partitions actually being elected, so return immediately - responseCallback(results ++ invalidPartitionsResults) + responseCallback(results) } } - controller.electPreferredLeaders(validPartitions, electionCallback) + controller.electPreferredLeaders(partitions, electionCallback) } } diff --git a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala index dd183e39b3a02..5a3278cda62db 100644 --- a/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AdminClientIntegrationTest.scala @@ -100,6 +100,8 @@ class AdminClientIntegrationTest extends IntegrationTestHarness with Logging { config.setProperty(KafkaConfig.ListenerSecurityProtocolMapProp, s"${listenerName.value}:${securityProtocol.name}") config.setProperty(KafkaConfig.DeleteTopicEnableProp, "true") config.setProperty(KafkaConfig.GroupInitialRebalanceDelayMsProp, "0") + config.setProperty(KafkaConfig.AutoLeaderRebalanceEnableProp, "false") + config.setProperty(KafkaConfig.ControlledShutdownEnableProp, "false") // We set this in order to test that we don't expose sensitive data via describe configs. This will already be // set for subclasses with security enabled and we don't want to overwrite it. if (!config.containsKey(KafkaConfig.SslTruststorePasswordProp)) @@ -1336,6 +1338,16 @@ class AdminClientIntegrationTest extends IntegrationTestHarness with Logging { changePreferredLeader(prefer1) // but shut it down... servers(1).shutdown() + waitUntilTrue ( + () => { + val description = client.describeTopics(Set (partition1.topic(), partition2.topic()).asJava).all().get() + return !description.asScala.flatMap{ + case (topic, description) => description.partitions().asScala.map( + partition => partition.isr().asScala).flatten + }.exists(node => node.id == 1) + }, + "Expect broker 1 to no longer be in any ISR" + ) // ... now what happens if we try to elect the preferred leader and it's down? val shortTimeout = new ElectPreferredLeadersOptions().timeoutMs(10000) diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index dd1cfbe753a57..9e8b80d88ec9f 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -21,7 +21,7 @@ import java.nio.charset.StandardCharsets import java.nio.file.{Files, Paths} import java.util.Properties -import kafka.admin.{PreferredReplicaLeaderElectionCommand} +import kafka.admin.PreferredReplicaLeaderElectionCommand import kafka.common.{AdminCommandFailedException, TopicAndPartition} import kafka.network.RequestChannel import kafka.security.auth._ @@ -29,7 +29,7 @@ import kafka.server.{KafkaConfig, KafkaServer} import kafka.utils.{Logging, TestUtils, ZkUtils} import kafka.zk.ZooKeeperTestHarness import org.apache.kafka.common.TopicPartition -import org.apache.kafka.common.errors.{LeaderNotAvailableException, TimeoutException, UnknownTopicOrPartitionException} +import org.apache.kafka.common.errors.{ClusterAuthorizationException, LeaderNotAvailableException, TimeoutException, UnknownTopicOrPartitionException} import org.apache.kafka.common.network.ListenerName import org.junit.Assert._ import org.junit.{After, Test} @@ -62,22 +62,26 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit // create brokers servers = brokerConfigs.map(b => TestUtils.createServer(KafkaConfig.fromProps(b))) // create the topic - partitionsAndAssignments.foreach { partitionAndAssignment => - zkClient.createTopicAssignment(partitionAndAssignment._1.topic(), - Map(partitionAndAssignment._1 -> partitionAndAssignment._2)) + partitionsAndAssignments.foreach { case (tp, assigment) => + zkClient.createTopicAssignment(tp.topic(), + Map(tp -> assigment)) } // wait until replica log is created on every broker TestUtils.waitUntilTrue(() => servers.forall(server => partitionsAndAssignments.forall(partitionAndAssignment => server.getLogManager().getLog(partitionAndAssignment._1).isDefined)), "Replicas for topic test not created") } - /** Bounce the given server and wait for all servers to get metadata for the given partition */ - private def bounceServer(server: Int, partition: TopicPartition) { - info(s"Shutting down server $server so a non-preferred replica becomes leader") - servers(server).shutdown() - info(s"Starting server $server now that a non-preferred replica is leader") - servers(server).startup() - TestUtils.waitUntilTrue(() => servers.forall(server => server.metadataCache.getPartitionInfo(partition.topic(), partition.partition()).isDefined), + /** Bounce the given targetServer and wait for all servers to get metadata for the given partition */ + private def bounceServer(targetServer: Int, partition: TopicPartition) { + debug(s"Shutting down server $targetServer so a non-preferred replica becomes leader") + servers(targetServer).shutdown() + debug(s"Starting server $targetServer now that a non-preferred replica is leader") + servers(targetServer).startup() + TestUtils.waitUntilTrue(() => servers.forall { server => + server.metadataCache.getPartitionInfo(partition.topic(), partition.partition()).exists { partitionState => + partitionState.basePartitionState.isr.contains(targetServer) + } + }, s"Replicas for partition $partition not created") } @@ -91,7 +95,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit private def bootstrapServer(broker: Int = 0): String = { val port = servers(broker).socketServer.boundPort(ListenerName.normalised("PLAINTEXT")) - info("Server bound to port "+port) + debug("Server bound to port "+port) s"localhost:$port" } @@ -116,24 +120,15 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit /** Test the case when an invalid broker is given for --bootstrap-broker */ @Test def testInvalidBrokerGiven() { - createTestTopicAndCluster(testPartitionAndAssignment) - bounceServer(testPartitionPreferredLeader, testPartition) - // Check the leader for the partition is not the preferred one - assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) try { PreferredReplicaLeaderElectionCommand.run(Array( "--bootstrap-server", "example.com:1234"), - timeout = 10000) + timeout = 1000) + fail() } catch { case e: AdminCommandFailedException => - if (e.getCause.isInstanceOf[TimeoutException]) { - // Check the leader for the partition is STILL not the preferred one - assertNotEquals(testPartitionPreferredLeader, getLeader(testPartition)) - } else { - throw e - } + assertTrue(e.getCause.isInstanceOf[TimeoutException]) } - } /** Test the case where no partitions are given (=> elect all partitions) */ @@ -153,7 +148,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit val jsonFile = File.createTempFile("preferredreplicaelection", ".js") jsonFile.deleteOnExit() val jsonString = ZkUtils.preferredReplicaLeaderElectionZkData(partitions.map(new TopicAndPartition(_))) - info("Using json: "+jsonString) + debug("Using json: "+jsonString) Files.write(Paths.get(jsonFile.getAbsolutePath), jsonString.getBytes(StandardCharsets.UTF_8)) jsonFile } @@ -306,7 +301,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit } } - /** Test the case where a list of partitions is given */ + /** Test the case where client is not authorized */ @Test def testAuthzFailure() { createTestTopicAndCluster(testPartitionAndAssignment, Some(classOf[PreferredReplicaLeaderElectionCommandTestAuthorizer].getName)) @@ -325,7 +320,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit } catch { case e: AdminCommandFailedException => assertEquals("1 preferred replica(s) could not be elected", e.getMessage) - assertTrue(e.getSuppressed()(0).getMessage.contains("Cluster authorization failed")) + assertTrue(e.getSuppressed()(0).isInstanceOf[ClusterAuthorizationException]) // Check we still have the same leader assertEquals(leader, getLeader(testPartition)) } finally { From 1fb6c0580cdd0ec730a11fd63a57a42e27db52ec Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 14 Jan 2019 16:18:06 +0000 Subject: [PATCH 14/22] Use JSON request/response definitions --- .../message/ElectPreferredLeadersRequest.json | 33 ++++++++++++++++ .../ElectPreferredLeadersResponse.json | 39 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json create mode 100644 clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json diff --git a/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json b/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json new file mode 100644 index 0000000000000..30a5f8c64bb80 --- /dev/null +++ b/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json @@ -0,0 +1,33 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +{ + "apiKey": 43, + "type": "request", + "name": "ElectPreferredLeadersRequest", + "validVersions": "0", + "fields": [ + { "name": "TopicPartitions", "type": "[]TopicPartitions", "versions": "0+", "nullableVersions": "0+", + "about": "The topic partitions to elect the preferred leader of.", + "fields": [ + { "name": "Topic", "type": "string", "versions": "0+", + "about": "The name of a topic." }, + { "name": "PartitionId", "type": "[]int32", "versions": "0+", + "about": "The partitions of this topic whose preferred leader should be elected" } + ]}, + { "name": "TimeoutMs", "type": "int32", "versions": "0+", + "about": "The time in ms to wait for the election to complete." } + ] +} \ No newline at end of file diff --git a/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json b/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json new file mode 100644 index 0000000000000..4b07491aa6a9e --- /dev/null +++ b/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json @@ -0,0 +1,39 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +{ + "apiKey": 43, + "type": "response", + "name": "ElectPreferredLeadersResponse", + "validVersions": "0", + "fields": [ + { "name": "ThrottleTimeMs", "type": "int32", "versions": "0+", + "about": "The duration in milliseconds for which the request was throttled due to a quota violation, or zero if the request did not violate any quota." }, + { "name": "ReplicaElectionResult", "type": "[]ReplicaElectionResult", "versions": "0+", + "about": "The error code, or 0 if there was no error.", "fields": [ + { "name": "Topic", "type": "string", "versions": "0+", + "about": "The topic name" }, + { "name": "PartitionResult", "type": "[]PartitionResult", "versions": "0+", + "about": "The topic name", "fields": [ + { "name": "PartitionId", "type": "int32", "versions": "0+", + "about": "The partition id" }, + { "name": "ErrorCode", "type": "int16", "versions": "0+", + "about": "The result error, or zero if there was no error."}, + { "name": "ErrorMessage", "type": "string", "versions": "0+", "nullableVersions": "0+", + "about": "The result message, or null if there was no error."} + ]} + ]} + ] +} \ No newline at end of file From a181a9a704bc9cc0adf92fc452b299ee2fca8558 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Wed, 16 Jan 2019 13:50:16 +0000 Subject: [PATCH 15/22] Review comments --- .../common/message/ElectPreferredLeadersResponse.json | 4 ++-- core/src/main/scala/kafka/controller/KafkaController.scala | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json b/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json index 4b07491aa6a9e..f34599cf03f39 100644 --- a/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json +++ b/clients/src/main/resources/common/message/ElectPreferredLeadersResponse.json @@ -21,12 +21,12 @@ "fields": [ { "name": "ThrottleTimeMs", "type": "int32", "versions": "0+", "about": "The duration in milliseconds for which the request was throttled due to a quota violation, or zero if the request did not violate any quota." }, - { "name": "ReplicaElectionResult", "type": "[]ReplicaElectionResult", "versions": "0+", + { "name": "ReplicaElectionResults", "type": "[]ReplicaElectionResult", "versions": "0+", "about": "The error code, or 0 if there was no error.", "fields": [ { "name": "Topic", "type": "string", "versions": "0+", "about": "The topic name" }, { "name": "PartitionResult", "type": "[]PartitionResult", "versions": "0+", - "about": "The topic name", "fields": [ + "about": "The results for each partition", "fields": [ { "name": "PartitionId", "type": "int32", "versions": "0+", "about": "The partition id" }, { "name": "ErrorCode", "type": "int16", "versions": "0+", diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index be7758757fd62..94000977dc8f4 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -638,7 +638,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti * @param partitions The partitions to have their preferred leader elected * @param electionType The election type * @return A map of failed elections where keys are partitions which had an error and the corresponding value is - * the exception that way thrown. + * the exception that was thrown. */ private def onPreferredReplicaElection(partitions: Set[TopicPartition], electionType: ElectionType): Map[TopicPartition, Throwable] = { @@ -1544,7 +1544,6 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti eventManager.put(PreferredReplicaLeaderElection(Some(partitions), AdminClientTriggered, callback)) case class PreferredReplicaLeaderElection(partitionsOpt: Option[Set[TopicPartition]], - //newPath: Boolean = false, electionType: ElectionType = ZkTriggered, callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends ControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance @@ -1572,7 +1571,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti val (partitionsBeingDeleted, livePartitions) = validPartitions.partition(partition => topicDeletionManager.isTopicQueuedUpForDeletion(partition.topic)) if (partitionsBeingDeleted.nonEmpty) { - error(s"Skipping preferred replica election for partitions $partitionsBeingDeleted " + + warn(s"Skipping preferred replica election for partitions $partitionsBeingDeleted " + s"since the respective topics are being deleted") } // partition those where preferred is already leader @@ -1587,7 +1586,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti val successfulPartitions = electablePartitions -- electionErrors.keySet val results = electionErrors.map { case (partition, ex) => val apiError = if (ex.isInstanceOf[StateChangeFailedException]) - new ApiError(Errors.LEADER_NOT_AVAILABLE, ex.getMessage) + new ApiError(Errors.PREFERRED_LEADER_NOT_AVAILABLE, ex.getMessage) else ApiError.fromThrowable(ex) partition -> apiError From 560591c7e7e7ff95d50d10a829d4e16c6adc80e5 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Fri, 18 Jan 2019 15:08:27 +0000 Subject: [PATCH 16/22] Actually use generated proto messages --- checkstyle/import-control.xml | 2 + .../apache/kafka/clients/NetworkClient.java | 6 +- .../kafka/clients/admin/KafkaAdminClient.java | 11 +- .../apache/kafka/common/protocol/ApiKeys.java | 16 +- .../common/requests/AbstractResponse.java | 4 +- .../ElectPreferredLeadersRequest.java | 168 ++++++++---------- .../ElectPreferredLeadersResponse.java | 104 +++-------- .../clients/admin/KafkaAdminClientTest.java | 9 +- .../common/requests/RequestContextTest.java | 3 +- .../common/requests/RequestResponseTest.java | 37 +++- .../main/scala/kafka/server/KafkaApis.scala | 27 ++- .../kafka/api/AuthorizerIntegrationTest.scala | 6 +- ...rredReplicaLeaderElectionCommandTest.scala | 2 +- .../unit/kafka/server/KafkaApisTest.scala | 2 +- .../unit/kafka/server/RequestQuotaTest.scala | 11 +- 15 files changed, 192 insertions(+), 216 deletions(-) diff --git a/checkstyle/import-control.xml b/checkstyle/import-control.xml index a0bf7400407da..0d316c5774fe7 100644 --- a/checkstyle/import-control.xml +++ b/checkstyle/import-control.xml @@ -116,6 +116,7 @@ + @@ -140,6 +141,7 @@ + diff --git a/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java b/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java index 144987e84949f..3973701063dba 100644 --- a/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java @@ -676,7 +676,8 @@ public Node leastLoadedNode(long now) { public static AbstractResponse parseResponse(ByteBuffer responseBuffer, RequestHeader requestHeader) { Struct responseStruct = parseStructMaybeUpdateThrottleTimeMetrics(responseBuffer, requestHeader, null, 0); - return AbstractResponse.parseResponse(requestHeader.apiKey(), responseStruct); + return AbstractResponse.parseResponse(requestHeader.apiKey(), responseStruct, + requestHeader.apiVersion()); } private static Struct parseStructMaybeUpdateThrottleTimeMetrics(ByteBuffer responseBuffer, RequestHeader requestHeader, @@ -811,7 +812,8 @@ private void handleCompletedReceives(List responses, long now) { req.header.apiKey(), req.header.correlationId(), responseStruct); } // If the received response includes a throttle delay, throttle the connection. - AbstractResponse body = AbstractResponse.parseResponse(req.header.apiKey(), responseStruct); + AbstractResponse body = AbstractResponse. + parseResponse(req.header.apiKey(), responseStruct, req.header.apiVersion()); maybeThrottle(body, req.header.apiVersion(), req.destination, now); if (req.isInternalRequest && body instanceof MetadataResponse) metadataUpdater.handleCompletedMetadataResponse(req.header, now, (MetadataResponse) body); diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java index 9198dd995408a..58baab79c8309 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java @@ -105,14 +105,14 @@ import org.apache.kafka.common.requests.DescribeGroupsResponse; import org.apache.kafka.common.requests.DescribeLogDirsRequest; import org.apache.kafka.common.requests.DescribeLogDirsResponse; +import org.apache.kafka.common.requests.ElectPreferredLeadersRequest; +import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.ExpireDelegationTokenRequest; import org.apache.kafka.common.requests.ExpireDelegationTokenResponse; import org.apache.kafka.common.requests.FindCoordinatorRequest; import org.apache.kafka.common.requests.FindCoordinatorResponse; import org.apache.kafka.common.requests.ListGroupsRequest; import org.apache.kafka.common.requests.ListGroupsResponse; -import org.apache.kafka.common.requests.ElectPreferredLeadersRequest; -import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.MetadataRequest; import org.apache.kafka.common.requests.MetadataResponse; import org.apache.kafka.common.requests.OffsetFetchRequest; @@ -2791,13 +2791,15 @@ public ElectPreferredLeadersResult electPreferredLeaders(final Collection errorCounts, Errors error) protected abstract Struct toStruct(short version); - public static AbstractResponse parseResponse(ApiKeys apiKey, Struct struct) { + public static AbstractResponse parseResponse(ApiKeys apiKey, Struct struct, short version) { switch (apiKey) { case PRODUCE: return new ProduceResponse(struct); @@ -157,7 +157,7 @@ public static AbstractResponse parseResponse(ApiKeys apiKey, Struct struct) { case DELETE_GROUPS: return new DeleteGroupsResponse(struct); case ELECT_PREFERRED_LEADERS: - return new ElectPreferredLeadersResponse(struct); + return new ElectPreferredLeadersResponse(struct, version); default: throw new AssertionError(String.format("ApiKey %s is not currently handled in `parseResponse`, the " + "code should be updated to do so.", apiKey)); diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java index 40343f09bdd99..a3ca335a05446 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java @@ -18,141 +18,115 @@ package org.apache.kafka.common.requests; import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.message.ElectPreferredLeadersRequestData; +import org.apache.kafka.common.message.ElectPreferredLeadersRequestData.TopicPartitions; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.ReplicaElectionResult; import org.apache.kafka.common.protocol.ApiKeys; -import org.apache.kafka.common.protocol.types.ArrayOf; -import org.apache.kafka.common.protocol.types.Field; -import org.apache.kafka.common.protocol.types.Schema; +import org.apache.kafka.common.protocol.Errors; import org.apache.kafka.common.protocol.types.Struct; import org.apache.kafka.common.utils.CollectionUtils; import java.nio.ByteBuffer; -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; - -import static org.apache.kafka.common.protocol.CommonFields.TOPIC_NAME; -import static org.apache.kafka.common.protocol.types.Type.INT32; public class ElectPreferredLeadersRequest extends AbstractRequest { + public static class Builder extends AbstractRequest.Builder { + private final ElectPreferredLeadersRequestData data; - private static final String TIMEOUT_KEY_NAME = "timeout"; - private static final String TOPIC_PARTITIONS_KEY_NAME = "topic_partitions"; - private static final String PARTITIONS_KEY_NAME = "partitions"; - - public static final Schema ELECT_PREFERRED_LEADERS_REQUEST_V0 = new Schema( - new Field(TOPIC_PARTITIONS_KEY_NAME, ArrayOf.nullable( - new Schema( - TOPIC_NAME, - new Field(PARTITIONS_KEY_NAME, - new ArrayOf(INT32), - "The partitions of this topic whose preferred leader should be elected")))), - new Field(TIMEOUT_KEY_NAME, INT32, "The time in ms to wait for the elections to be completed.") - ); - - public static Schema[] schemaVersions() { - return new Schema[]{ELECT_PREFERRED_LEADERS_REQUEST_V0}; - } - - public static class Builder extends AbstractRequest.Builder { - - private final Set topicPartitions; - private final int timeout; - - public Builder(Collection topicPartitions, int timeout) { + public Builder(ElectPreferredLeadersRequestData data) { super(ApiKeys.ELECT_PREFERRED_LEADERS); - this.topicPartitions = topicPartitions != null ? new HashSet<>(topicPartitions) : null; - this.timeout = timeout; + this.data = data; } @Override public ElectPreferredLeadersRequest build(short version) { - return new ElectPreferredLeadersRequest(version, topicPartitions, timeout); + return new ElectPreferredLeadersRequest(data, version); } - } - - private final Set topicPartitions; - private final int timeout; - public ElectPreferredLeadersRequest(short version, Set topicPartitions, int timeout) { - super(ApiKeys.ELECT_PREFERRED_LEADERS, version); - this.topicPartitions = topicPartitions; - this.timeout = timeout; + @Override + public String toString() { + return data.toString(); + } } - public ElectPreferredLeadersRequest(Struct struct, short version) { - super(ApiKeys.ELECT_PREFERRED_LEADERS, version); - Object[] topicPartitionsArray = struct.getArray(TOPIC_PARTITIONS_KEY_NAME); - if (topicPartitionsArray != null) { - topicPartitions = new HashSet<>(topicPartitionsArray.length); - for (Object topicPartitionObj : topicPartitionsArray) { - Struct topicPartitionStruct = (Struct) topicPartitionObj; - String topicName = topicPartitionStruct.get(TOPIC_NAME); - Object[] partitionsArray = topicPartitionStruct.getArray(PARTITIONS_KEY_NAME); - for (Object partitionObj : partitionsArray) { - Integer partition = (Integer) partitionObj; - TopicPartition topicPartition = new TopicPartition(topicName, partition); - topicPartitions.add(topicPartition); - } + public static ElectPreferredLeadersRequestData toRequestData(Collection partitions, int timeoutMs) { + ElectPreferredLeadersRequestData d = new ElectPreferredLeadersRequestData() + .setTimeoutMs(timeoutMs); + if (partitions != null) { + for (Map.Entry> tp : CollectionUtils.groupPartitionsByTopic(partitions).entrySet()) { + d.topicPartitions().add(new ElectPreferredLeadersRequestData.TopicPartitions().setTopic(tp.getKey()).setPartitionId(tp.getValue())); } } else { - topicPartitions = null; + d.setTopicPartitions(null); + } + return d; + } + + public static Map fromResponseData(ElectPreferredLeadersResponseData data) { + Map map = new HashMap<>(); + for (ElectPreferredLeadersResponseData.ReplicaElectionResult topicResults : data.replicaElectionResults()) { + for (ElectPreferredLeadersResponseData.PartitionResult partitionResult : topicResults.partitionResult()) { + map.put(new TopicPartition(topicResults.topic(), partitionResult.partitionId()), + new ApiError(Errors.forCode(partitionResult.errorCode()), + partitionResult.errorMessage())); + } } - timeout = struct.getInt(TIMEOUT_KEY_NAME); + return map; } - public Set topicPartitions() { - return topicPartitions; + private final ElectPreferredLeadersRequestData data; + private final short version; + + private ElectPreferredLeadersRequest(ElectPreferredLeadersRequestData data, short version) { + super(ApiKeys.ELECT_PREFERRED_LEADERS, version); + this.data = data; + this.version = version; } - public int timeout() { - return timeout; + public ElectPreferredLeadersRequest(Struct struct, short version) { + super(ApiKeys.ELECT_PREFERRED_LEADERS, version); + this.data = new ElectPreferredLeadersRequestData(struct, version); + this.version = version; } - @Override - protected Struct toStruct() { - Struct struct = new Struct(ApiKeys.ELECT_PREFERRED_LEADERS.requestSchema(version())); - Struct[] topicPartitionsArray; - if (topicPartitions != null) { - Map> map = CollectionUtils.groupPartitionsByTopic(topicPartitions); - List topicPartitionsList = new ArrayList<>(topicPartitions.size()); - for (Map.Entry> entry : map.entrySet()) { - Struct partitionStruct = struct.instance(TOPIC_PARTITIONS_KEY_NAME); - partitionStruct.set(TOPIC_NAME, entry.getKey()); - partitionStruct.set(PARTITIONS_KEY_NAME, entry.getValue().toArray()); - topicPartitionsList.add(partitionStruct); - } - topicPartitionsArray = topicPartitionsList.toArray(new Struct[topicPartitionsList.size()]); - } else { - topicPartitionsArray = null; - } - struct.set(TOPIC_PARTITIONS_KEY_NAME, topicPartitionsArray); - struct.set(TIMEOUT_KEY_NAME, timeout); - return struct; + public ElectPreferredLeadersRequestData data() { + return data; } @Override public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) { - short version = version(); - switch (version) { - case 0: - ApiError error = ApiError.fromThrowable(e); - Map errors = new HashMap<>(topicPartitions.size()); - for (TopicPartition partition : topicPartitions) - errors.put(partition, error); - return new ElectPreferredLeadersResponse(throttleTimeMs, errors); - default: - throw new IllegalArgumentException(String.format( - "Version %d is not valid. Valid versions for %s are 0 to %d", - version, this.getClass().getSimpleName(), ApiKeys.ELECT_PREFERRED_LEADERS.latestVersion())); + ElectPreferredLeadersResponseData response = new ElectPreferredLeadersResponseData(); + if (version() >= 2) { + response.setThrottleTimeMs(throttleTimeMs); } + ApiError apiError = ApiError.fromThrowable(e); + for (TopicPartitions topic : data.topicPartitions()) { + ReplicaElectionResult electionResult = new ReplicaElectionResult().setTopic(topic.topic()); + for (Integer partitionId : topic.partitionId()) { + electionResult.partitionResult().add(new ElectPreferredLeadersResponseData.PartitionResult() + .setPartitionId(partitionId) + .setErrorCode(apiError.error().code()) + .setErrorMessage(apiError.message())); + } + response.replicaElectionResults().add( + electionResult); + } + return new ElectPreferredLeadersResponse(response); } public static ElectPreferredLeadersRequest parse(ByteBuffer buffer, short version) { return new ElectPreferredLeadersRequest(ApiKeys.ELECT_PREFERRED_LEADERS.parseRequest(version, buffer), version); } -} + /** + * Visible for testing. + */ + @Override + public Struct toStruct() { + return data.toStruct(version); + } +} \ No newline at end of file diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java index d8833ba92efcd..d19a51d4f6839 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersResponse.java @@ -17,110 +17,62 @@ package org.apache.kafka.common.requests; -import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.PartitionResult; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.ReplicaElectionResult; import org.apache.kafka.common.protocol.ApiKeys; import org.apache.kafka.common.protocol.Errors; -import org.apache.kafka.common.protocol.types.ArrayOf; -import org.apache.kafka.common.protocol.types.Field; -import org.apache.kafka.common.protocol.types.Schema; import org.apache.kafka.common.protocol.types.Struct; -import org.apache.kafka.common.utils.CollectionUtils; import java.nio.ByteBuffer; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; -import static org.apache.kafka.common.protocol.CommonFields.ERROR_CODE; -import static org.apache.kafka.common.protocol.CommonFields.ERROR_MESSAGE; -import static org.apache.kafka.common.protocol.CommonFields.PARTITION_ID; -import static org.apache.kafka.common.protocol.CommonFields.THROTTLE_TIME_MS; -import static org.apache.kafka.common.protocol.CommonFields.TOPIC_NAME; - public class ElectPreferredLeadersResponse extends AbstractResponse { - private static final String REPLICA_ELECTION_RESULT_KEY_NAME = "replica_election_result"; - private static final String PARTITION_RESULTS_KEY_NAME = "partition_results"; - - public static final Schema ELECT_PREFERRED_LEADERS_RESPONSE_V0 = new Schema( - THROTTLE_TIME_MS, - new Field(REPLICA_ELECTION_RESULT_KEY_NAME, new ArrayOf(new Schema( - TOPIC_NAME, - new Field(PARTITION_RESULTS_KEY_NAME, new ArrayOf( - new Schema( - PARTITION_ID, - ERROR_CODE, - ERROR_MESSAGE)), - "The results for each partition"))))); + private final ElectPreferredLeadersResponseData data; - public static Schema[] schemaVersions() { - return new Schema[]{ELECT_PREFERRED_LEADERS_RESPONSE_V0}; + public ElectPreferredLeadersResponse(ElectPreferredLeadersResponseData data) { + this.data = data; } - private final int throttleTimeMs; - private final Map errors; - - public ElectPreferredLeadersResponse(int throttleTimeMs, Map errors) { - this.throttleTimeMs = throttleTimeMs; - this.errors = errors; + public ElectPreferredLeadersResponse(Struct struct, short version) { + this.data = new ElectPreferredLeadersResponseData(struct, version); } - public ElectPreferredLeadersResponse(Struct struct) { - throttleTimeMs = struct.get(THROTTLE_TIME_MS); - Object[] resourcesArray = struct.getArray(REPLICA_ELECTION_RESULT_KEY_NAME); - errors = new HashMap<>(resourcesArray.length); - for (Object partitionObj : resourcesArray) { - Struct topicStruct = (Struct) partitionObj; - String topicName = topicStruct.get(TOPIC_NAME); - Object[] partitionResults = topicStruct.getArray(PARTITION_RESULTS_KEY_NAME); - for (Object partitionResult : partitionResults) { - Struct partitionStruct = (Struct) partitionResult; - int partitionId = partitionStruct.get(PARTITION_ID); - ApiError error = new ApiError(partitionStruct); - errors.put(new TopicPartition(topicName, partitionId), error); - } - } + public ElectPreferredLeadersResponseData data() { + return data; } - public Map errors() { - return errors; + @Override + protected Struct toStruct(short version) { + return data.toStruct(version); } + @Override public int throttleTimeMs() { - return throttleTimeMs; + return data.throttleTimeMs(); } @Override public Map errorCounts() { - return apiErrorCounts(errors); - } - - @Override - protected Struct toStruct(short version) { - Struct struct = new Struct(ApiKeys.ELECT_PREFERRED_LEADERS.responseSchema(version)); - struct.set(THROTTLE_TIME_MS, throttleTimeMs); - Map> groupedErrors = CollectionUtils.groupPartitionDataByTopic(errors); - List replicaElectionResultList = new ArrayList<>(errors.size()); - for (Map.Entry> topicToErrors : groupedErrors.entrySet()) { - Struct topicStruct = struct.instance(REPLICA_ELECTION_RESULT_KEY_NAME); - topicStruct.set(TOPIC_NAME, topicToErrors.getKey()); - List partitionResultList = new ArrayList<>(topicToErrors.getValue().size()); - for (Map.Entry partitionToError : topicToErrors.getValue().entrySet()) { - Struct partitionResultStruct = topicStruct.instance(PARTITION_RESULTS_KEY_NAME); - partitionResultStruct.set(PARTITION_ID, partitionToError.getKey()); - partitionToError.getValue().write(partitionResultStruct); - partitionResultList.add(partitionResultStruct); + HashMap counts = new HashMap<>(); + for (ReplicaElectionResult result : data.replicaElectionResults()) { + for (PartitionResult partitionResult : result.partitionResult()) { + Errors error = Errors.forCode(partitionResult.errorCode()); + counts.put(error, counts.getOrDefault(error, 0) + 1); } - topicStruct.set(PARTITION_RESULTS_KEY_NAME, partitionResultList.toArray()); - replicaElectionResultList.add(topicStruct); } - struct.set(REPLICA_ELECTION_RESULT_KEY_NAME, replicaElectionResultList.toArray(new Struct[0])); - return struct; + return counts; } public static ElectPreferredLeadersResponse parse(ByteBuffer buffer, short version) { - return new ElectPreferredLeadersResponse(ApiKeys.ELECT_PREFERRED_LEADERS.parseResponse(version, buffer)); + return new ElectPreferredLeadersResponse( + ApiKeys.ELECT_PREFERRED_LEADERS.responseSchema(version).read(buffer), version); } -} + @Override + public boolean shouldClientThrottle(short version) { + return version >= 3; + } +} \ No newline at end of file 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 67121229ad6f1..2955658ce0d5d 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 @@ -40,7 +40,6 @@ import org.apache.kafka.common.errors.AuthenticationException; import org.apache.kafka.common.errors.GroupAuthorizationException; import org.apache.kafka.common.errors.InvalidRequestException; -import org.apache.kafka.common.errors.ClusterAuthorizationException; import org.apache.kafka.common.errors.InvalidTopicException; import org.apache.kafka.common.errors.LeaderNotAvailableException; import org.apache.kafka.common.errors.NotLeaderForPartitionException; @@ -71,7 +70,6 @@ import org.apache.kafka.common.requests.FindCoordinatorResponse; import org.apache.kafka.common.requests.ListGroupsResponse; import org.apache.kafka.common.requests.MetadataRequest; -import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.MetadataResponse; import org.apache.kafka.common.requests.OffsetFetchResponse; import org.apache.kafka.common.resource.PatternType; @@ -638,7 +636,7 @@ public void testDeleteAcls() throws Exception { @Test public void testElectPreferredLeaders() throws Exception { - TopicPartition topic1 = new TopicPartition("topic", 0); + /*TopicPartition topic1 = new TopicPartition("topic", 0); TopicPartition topic2 = new TopicPartition("topic", 2); try (AdminClientUnitTestEnv env = mockClientEnv()) { env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); @@ -657,7 +655,8 @@ public void testElectPreferredLeaders() throws Exception { // Test a call where there are no errors. map.put(topic1, ApiError.NONE); map.put(topic2, ApiError.NONE); - env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(0, + env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse( + ElectPreferredLeadersUtil.fromElectPreferredLeadersRequest()0, map)); results = env.adminClient().electPreferredLeaders(asList(topic1, topic2)); @@ -668,7 +667,7 @@ public void testElectPreferredLeaders() throws Exception { results = env.adminClient().electPreferredLeaders(asList(topic1, topic2), new ElectPreferredLeadersOptions().timeoutMs(100)); TestUtils.assertFutureError(results.partitionResult(topic1), TimeoutException.class); TestUtils.assertFutureError(results.partitionResult(topic2), TimeoutException.class); - } + }*/ } /** diff --git a/clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java b/clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java index ed50b93bc4a5c..857869f316ec5 100644 --- a/clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java +++ b/clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java @@ -68,7 +68,8 @@ public void testSerdeUnsupportedApiVersionRequest() throws Exception { assertEquals(correlationId, responseHeader.correlationId()); Struct struct = ApiKeys.API_VERSIONS.parseResponse((short) 0, responseBuffer); - ApiVersionsResponse response = (ApiVersionsResponse) AbstractResponse.parseResponse(ApiKeys.API_VERSIONS, struct); + ApiVersionsResponse response = (ApiVersionsResponse) + AbstractResponse.parseResponse(ApiKeys.API_VERSIONS, struct, (short) 0); assertEquals(Errors.UNSUPPORTED_VERSION, response.error()); assertTrue(response.apiVersions().isEmpty()); } diff --git a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java index 5cb8335d5f5ff..a65ec3b1b6b30 100644 --- a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java +++ b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java @@ -32,6 +32,11 @@ import org.apache.kafka.common.errors.SecurityDisabledException; import org.apache.kafka.common.errors.UnknownServerException; import org.apache.kafka.common.errors.UnsupportedVersionException; +import org.apache.kafka.common.message.ElectPreferredLeadersRequestData; +import org.apache.kafka.common.message.ElectPreferredLeadersRequestData.TopicPartitions; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.PartitionResult; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.ReplicaElectionResult; import org.apache.kafka.common.network.ListenerName; import org.apache.kafka.common.network.Send; import org.apache.kafka.common.protocol.ApiKeys; @@ -464,7 +469,7 @@ public void produceResponseV5Test() { Struct deserializedStruct = ApiKeys.PRODUCE.parseResponse(version, buffer); ProduceResponse v5FromBytes = (ProduceResponse) AbstractResponse.parseResponse(ApiKeys.PRODUCE, - deserializedStruct); + deserializedStruct, version); assertEquals(1, v5FromBytes.responses().size()); assertTrue(v5FromBytes.responses().containsKey(tp0)); @@ -1328,20 +1333,34 @@ private DescribeDelegationTokenResponse createDescribeTokenResponse() { } private ElectPreferredLeadersRequest createElectPreferredLeadersRequestNullPartitions() { - return new ElectPreferredLeadersRequest.Builder(null, 100).build((short) 0); + return new ElectPreferredLeadersRequest.Builder( + new ElectPreferredLeadersRequestData() + .setTimeoutMs(100) + .setTopicPartitions(null)) + .build((short) 0); } private ElectPreferredLeadersRequest createElectPreferredLeadersRequest() { - return new ElectPreferredLeadersRequest.Builder(asList( - new TopicPartition("my_topic", 1), - new TopicPartition("my_topic", 2)), 100).build((short) 0); + ElectPreferredLeadersRequestData data = new ElectPreferredLeadersRequestData() + .setTimeoutMs(100); + data.topicPartitions().add(new TopicPartitions().setTopic("data").setPartitionId(asList(1, 2))); + return new ElectPreferredLeadersRequest.Builder(data).build((short) 0); } private ElectPreferredLeadersResponse createElectPreferredLeadersResponse() { - Map errors = new HashMap<>(); - errors.put(new TopicPartition("my_topic", 0), ApiError.NONE); - errors.put(new TopicPartition("my_topic", 1), ApiError.fromThrowable(Errors.UNKNOWN_TOPIC_OR_PARTITION.exception("blah"))); - return new ElectPreferredLeadersResponse(200, errors); + //Map errors = new HashMap<>(); + //errors.put(new TopicPartition("myTopic", 0), ApiError.NONE); + //errors.put(new TopicPartition("myTopic", 1), ApiError.fromThrowable(Errors.UNKNOWN_TOPIC_OR_PARTITION.exception("blah"))); + ElectPreferredLeadersResponseData data = new ElectPreferredLeadersResponseData().setThrottleTimeMs(200); + ReplicaElectionResult resultsByTopic = new ReplicaElectionResult().setTopic("myTopic"); + resultsByTopic.partitionResult().add(new PartitionResult().setPartitionId(0) + .setErrorCode(Errors.NONE.code()) + .setErrorMessage(Errors.NONE.message())); + resultsByTopic.partitionResult().add(new PartitionResult().setPartitionId(0) + .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) + .setErrorMessage(Errors.UNKNOWN_TOPIC_OR_PARTITION.message())); + data.replicaElectionResults().add(resultsByTopic); + return new ElectPreferredLeadersResponse(data); } } diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index c2b87beba9406..880f698a4af83 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -44,6 +44,7 @@ import org.apache.kafka.common.config.ConfigResource import org.apache.kafka.common.errors._ import org.apache.kafka.common.internals.FatalExitError import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME, isInternal} +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData import org.apache.kafka.common.metrics.Metrics import org.apache.kafka.common.network.{ListenerName, Send} import org.apache.kafka.common.protocol.{ApiKeys, Errors} @@ -2236,19 +2237,33 @@ class KafkaApis(val requestChannel: RequestChannel, val electionRequest = request.body[ElectPreferredLeadersRequest] val partitions = - if (electionRequest.topicPartitions() == null) { + if (electionRequest.data().topicPartitions() == null) { metadataCache.getAllPartitions() } else { - electionRequest.topicPartitions().asScala + electionRequest.data().topicPartitions().asScala.flatMap{tp => + tp.partitionId().asScala.map(partitionId => new TopicPartition(tp.topic, partitionId))}.toSet } def sendResponseCallback(result: Map[TopicPartition, ApiError]): Unit = { - sendResponseMaybeThrottle(request, requestThrottleMs => - new ElectPreferredLeadersResponse(requestThrottleMs, result.asJava)) + sendResponseMaybeThrottle(request, requestThrottleMs => { + val results = result. + groupBy{case (tp, error) => tp.topic}. + map{case (topic, ps) => new ElectPreferredLeadersResponseData.ReplicaElectionResult() + .setTopic(topic) + .setPartitionResult(ps.map{ + case (tp, error) => + new ElectPreferredLeadersResponseData.PartitionResult() + .setErrorCode(error.error.code) + .setErrorMessage(error.message()) + .setPartitionId(tp.partition)}.toList.asJava)} + val data = new ElectPreferredLeadersResponseData() + .setThrottleTimeMs(requestThrottleMs) + .setReplicaElectionResults(results.toList.asJava) + new ElectPreferredLeadersResponse(data)}) } if (!authorize(request.session, Alter, Resource.ClusterResource)) { val error = new ApiError(Errors.CLUSTER_AUTHORIZATION_FAILED, null); val partitionErrors = - if (electionRequest.topicPartitions() == null) { + if (electionRequest.data().topicPartitions() == null) { // Don't leak the set of partitions if the client lack authz Map.empty[TopicPartition, ApiError] } else { @@ -2256,7 +2271,7 @@ class KafkaApis(val requestChannel: RequestChannel, } sendResponseCallback(partitionErrors) } else { - replicaManager.electPreferredLeaders(controller, partitions, sendResponseCallback, electionRequest.timeout) + replicaManager.electPreferredLeaders(controller, partitions, sendResponseCallback, electionRequest.data().timeoutMs()) } } diff --git a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala index 34d4ab5024852..93b49ee538939 100644 --- a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala @@ -189,7 +189,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.DESCRIBE_LOG_DIRS -> ((resp: DescribeLogDirsResponse) => if (resp.logDirInfos.size() > 0) resp.logDirInfos.asScala.head._2.error else Errors.CLUSTER_AUTHORIZATION_FAILED), ApiKeys.CREATE_PARTITIONS -> ((resp: CreatePartitionsResponse) => resp.errors.asScala.find(_._1 == topic).get._2.error), - ApiKeys.ELECT_PREFERRED_LEADERS -> ((resp: ElectPreferredLeadersResponse) => resp.errors.get(tp).error) + ApiKeys.ELECT_PREFERRED_LEADERS -> ((resp: ElectPreferredLeadersResponse) => + ElectPreferredLeadersRequest.fromResponseData(resp.data()).get(tp).error()) ) val requestKeysToAcls = Map[ApiKeys, Map[Resource, Set[Acl]]]( @@ -386,7 +387,8 @@ class AuthorizerIntegrationTest extends BaseRequestTest { private def addOffsetsToTxnRequest = new AddOffsetsToTxnRequest.Builder(transactionalId, 1, 1, group).build() - private def electPreferredLeadersRequest = new ElectPreferredLeadersRequest.Builder(Collections.singleton(tp), 10000).build() + private def electPreferredLeadersRequest = new ElectPreferredLeadersRequest.Builder( + ElectPreferredLeadersRequest.toRequestData(Collections.singleton(tp), 10000)).build() @Test def testAuthorizationWithTopicExisting() { diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index 9e8b80d88ec9f..2d4ce805803fd 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -288,7 +288,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit PreferredReplicaLeaderElectionCommand.run(Array( "--bootstrap-server", bootstrapServer(), "--path-to-json-file", jsonFile.getAbsolutePath), - timeout = 10000) + timeout = 2000) fail(); } catch { case e: AdminCommandFailedException => diff --git a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala index 9b4210ed59e32..a3ecb0773c124 100644 --- a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala +++ b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala @@ -524,7 +524,7 @@ class KafkaApisTest { channel.buffer.getInt() // read the size ResponseHeader.parse(channel.buffer) val struct = api.responseSchema(request.version).read(channel.buffer) - AbstractResponse.parseResponse(api, struct) + AbstractResponse.parseResponse(api, struct, request.version) } private def expectNoThrottling(): Capture[RequestChannel.Response] = { diff --git a/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala b/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala index ac380be0b96a4..3176f72939aaa 100644 --- a/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala +++ b/core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala @@ -24,6 +24,7 @@ import kafka.security.auth._ import kafka.utils.TestUtils import org.apache.kafka.common.acl.{AccessControlEntry, AccessControlEntryFilter, AclBinding, AclBindingFilter, AclOperation, AclPermissionType} import org.apache.kafka.common.config.ConfigResource +import org.apache.kafka.common.message.ElectPreferredLeadersRequestData import org.apache.kafka.common.resource.{PatternType, ResourcePattern, ResourcePatternFilter, ResourceType => AdminResourceType} import org.apache.kafka.common.{Node, TopicPartition} import org.apache.kafka.common.metrics.{KafkaMetric, Quota, Sensor} @@ -360,7 +361,13 @@ class RequestQuotaTest extends BaseRequestTest { new DeleteGroupsRequest.Builder(Collections.singleton("test-group")) case ApiKeys.ELECT_PREFERRED_LEADERS => - new ElectPreferredLeadersRequest.Builder(Collections.singleton(new TopicPartition("my_topic", 0)), 0) + val partition = new ElectPreferredLeadersRequestData.TopicPartitions() + .setPartitionId(Collections.singletonList(0)) + .setTopic("my_topic") + new ElectPreferredLeadersRequest.Builder( + new ElectPreferredLeadersRequestData() + .setTimeoutMs(0) + .setTopicPartitions(Collections.singletonList(partition))) case _ => throw new IllegalArgumentException("Unsupported API key " + apiKey) @@ -453,7 +460,7 @@ class RequestQuotaTest extends BaseRequestTest { case ApiKeys.RENEW_DELEGATION_TOKEN => new RenewDelegationTokenResponse(response).throttleTimeMs case ApiKeys.DELETE_GROUPS => new DeleteGroupsResponse(response).throttleTimeMs case ApiKeys.OFFSET_FOR_LEADER_EPOCH => new OffsetsForLeaderEpochResponse(response).throttleTimeMs - case ApiKeys.ELECT_PREFERRED_LEADERS => new ElectPreferredLeadersResponse(response).throttleTimeMs + case ApiKeys.ELECT_PREFERRED_LEADERS => new ElectPreferredLeadersResponse(response, 0).throttleTimeMs case requestId => throw new IllegalArgumentException(s"No throttle time for $requestId") } } From cfb6bb9dff8e18f85b3a27a6b6d6b4c6e3245592 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 21 Jan 2019 14:07:37 +0000 Subject: [PATCH 17/22] Fix test --- .../admin/PreferredReplicaLeaderElectionCommandTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index 2d4ce805803fd..1f9777f59ba14 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -29,7 +29,7 @@ import kafka.server.{KafkaConfig, KafkaServer} import kafka.utils.{Logging, TestUtils, ZkUtils} import kafka.zk.ZooKeeperTestHarness import org.apache.kafka.common.TopicPartition -import org.apache.kafka.common.errors.{ClusterAuthorizationException, LeaderNotAvailableException, TimeoutException, UnknownTopicOrPartitionException} +import org.apache.kafka.common.errors.{ClusterAuthorizationException, PreferredLeaderNotAvailableException, TimeoutException, UnknownTopicOrPartitionException} import org.apache.kafka.common.network.ListenerName import org.junit.Assert._ import org.junit.{After, Test} @@ -264,7 +264,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit case e: AdminCommandFailedException => assertEquals("1 preferred replica(s) could not be elected", e.getMessage) val suppressed = e.getSuppressed()(0) - assertTrue(suppressed.isInstanceOf[LeaderNotAvailableException]) + assertTrue(suppressed.isInstanceOf[PreferredLeaderNotAvailableException]) assertTrue(suppressed.getMessage, suppressed.getMessage.contains("Failed to elect leader for partition test-0 under strategy PreferredReplicaPartitionLeaderElectionStrategy")) // Check we still have the same leader assertEquals(leader, getLeader(testPartition)) From b2d4eb13c24f86cd32f263e5fe582434472c4078 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 21 Jan 2019 11:54:19 +0000 Subject: [PATCH 18/22] Jun comments --- .../src/main/scala/kafka/controller/KafkaController.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 94000977dc8f4..1b83e57c59dfe 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -586,7 +586,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti val partitionsToBeRemovedFromReassignment = scala.collection.mutable.Set.empty[TopicPartition] topicPartitions.foreach { tp => if (topicDeletionManager.isTopicQueuedUpForDeletion(tp.topic)) { - error(s"Skipping reassignment of $tp since the topic is currently being deleted") + info(s"Skipping reassignment of $tp since the topic is currently being deleted") partitionsToBeRemovedFromReassignment.add(tp) } else { val reassignedPartitionContext = controllerContext.partitionsBeingReassigned.get(tp).getOrElse { @@ -1543,14 +1543,14 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti def electPreferredLeaders(partitions: Set[TopicPartition], callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = { (_,_) => }): Unit = eventManager.put(PreferredReplicaLeaderElection(Some(partitions), AdminClientTriggered, callback)) - case class PreferredReplicaLeaderElection(partitionsOpt: Option[Set[TopicPartition]], + case class PreferredReplicaLeaderElection(partitionsFromAdminClientOpt: Option[Set[TopicPartition]], electionType: ElectionType = ZkTriggered, callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends ControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance override def process(): Unit = { if (!isActive) { - callback(Set(), partitionsOpt match { + callback(Set(), partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap case None => Map.empty }) @@ -1558,7 +1558,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti // We need to register the watcher if the path doesn't exist in order to detect future preferred replica // leader elections and we get the `path exists` check for free if (electionType == AdminClientTriggered || zkClient.registerZNodeChangeHandlerAndCheckExistence(preferredReplicaElectionHandler)) { - val partitions = partitionsOpt match { + val partitions = partitionsFromAdminClientOpt match { case Some(partitions) => partitions case None => zkClient.getPreferredReplicaElection } From 2cd37d5d61faf3fc902bc1b8f20b031dee89ad55 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Mon, 21 Jan 2019 14:07:15 +0000 Subject: [PATCH 19/22] Invoke callbacks when clearing queue --- .../controller/ControllerEventManager.scala | 6 +++ .../kafka/controller/KafkaController.scala | 46 +++++++++++++++++-- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/kafka/controller/ControllerEventManager.scala b/core/src/main/scala/kafka/controller/ControllerEventManager.scala index c93e9e79ec21f..9dc26982eaa36 100644 --- a/core/src/main/scala/kafka/controller/ControllerEventManager.scala +++ b/core/src/main/scala/kafka/controller/ControllerEventManager.scala @@ -28,6 +28,7 @@ import org.apache.kafka.common.errors.ControllerMovedException import org.apache.kafka.common.utils.Time import scala.collection._ +import scala.collection.JavaConverters._ object ControllerEventManager { val ControllerEventThreadName = "controller-event-thread" @@ -69,6 +70,11 @@ class ControllerEventManager(controllerId: Int, rateAndTimeMetrics: Map[Controll } def clearAndPut(event: ControllerEvent): Unit = inLock(putLock) { + queue.asScala.foreach(evt => + if (evt.isInstanceOf[PreemptableControllerEvent]) { + evt.asInstanceOf[PreemptableControllerEvent].preempt() + } + ) queue.clear() put(event) } diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 1b83e57c59dfe..db025d91b6fe6 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -16,6 +16,7 @@ */ package kafka.controller +import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.{CountDownLatch, TimeUnit} import com.yammer.metrics.core.Gauge @@ -38,7 +39,7 @@ import org.apache.zookeeper.KeeperException import org.apache.zookeeper.KeeperException.Code import scala.collection._ -import scala.util.Try +import scala.util.{Failure, Try} object KafkaController extends Logging { val InitialControllerEpoch = 0 @@ -1045,11 +1046,15 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } } - case class ControlledShutdown(id: Int, brokerEpoch: Long, controlledShutdownCallback: Try[Set[TopicPartition]] => Unit) extends ControllerEvent { + case class ControlledShutdown(id: Int, brokerEpoch: Long, controlledShutdownCallback: Try[Set[TopicPartition]] => Unit) extends PreemptableControllerEvent { def state = ControllerState.ControlledShutdown - override def process(): Unit = { + override def handlePreempt(): Unit = { + controlledShutdownCallback(Failure(new ControllerMovedException("Controller moved to another broker"))) + } + + override def handleProcess(): Unit = { val controlledShutdownResult = Try { doControlledShutdown(id) } controlledShutdownCallback(controlledShutdownResult) } @@ -1545,10 +1550,17 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti case class PreferredReplicaLeaderElection(partitionsFromAdminClientOpt: Option[Set[TopicPartition]], electionType: ElectionType = ZkTriggered, - callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends ControllerEvent { + callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends PreemptableControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance - override def process(): Unit = { + override def handlePreempt(): Unit = { + callback(Set(), partitionsFromAdminClientOpt match { + case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap + case None => Map.empty + }) + } + + override def handleProcess(): Unit = { if (!isActive) { callback(Set(), partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap @@ -1777,3 +1789,27 @@ sealed trait ControllerEvent { def state: ControllerState def process(): Unit } + +/** + * A `ControllerEvent`, such as one with a client callback, which needs specific handing in the event of ZK session expiration. + */ +sealed trait PreemptableControllerEvent extends ControllerEvent { + + val spent = new AtomicBoolean(false) + + final def preempt(): Unit = { + if (!spent.getAndSet(true)) { + handlePreempt() + } + } + + final def process(): Unit = { + if (!spent.getAndSet(true)) { + handleProcess() + } + } + + def handlePreempt(): Unit + + def handleProcess(): Unit +} From fce97efd7f189f826f8e7e51af443cd7b1a6b978 Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Thu, 24 Jan 2019 11:16:21 +0000 Subject: [PATCH 20/22] Review comments --- .../kafka/clients/admin/AdminClient.java | 1 - .../ElectPreferredLeadersRequest.java | 7 +--- .../main/resources/common/message/README.md | 2 +- .../clients/admin/KafkaAdminClientTest.java | 42 +++++++++++++------ .../common/requests/RequestResponseTest.java | 5 +-- .../controller/ControllerEventManager.scala | 3 +- .../kafka/controller/KafkaController.scala | 27 ++++++------ .../main/scala/kafka/server/KafkaApis.scala | 1 - .../scala/kafka/server/ReplicaManager.scala | 7 ++-- ...rredReplicaLeaderElectionCommandTest.scala | 2 +- 10 files changed, 53 insertions(+), 44 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java index a2426982a2af4..b823cdc0509ed 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClient.java @@ -848,5 +848,4 @@ public abstract ElectPreferredLeadersResult electPreferredLeaders(Collection metrics(); - } diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java index a3ca335a05446..ab96e3ba9e98c 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/ElectPreferredLeadersRequest.java @@ -100,9 +100,7 @@ public ElectPreferredLeadersRequestData data() { @Override public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) { ElectPreferredLeadersResponseData response = new ElectPreferredLeadersResponseData(); - if (version() >= 2) { - response.setThrottleTimeMs(throttleTimeMs); - } + response.setThrottleTimeMs(throttleTimeMs); ApiError apiError = ApiError.fromThrowable(e); for (TopicPartitions topic : data.topicPartitions()) { ReplicaElectionResult electionResult = new ReplicaElectionResult().setTopic(topic.topic()); @@ -112,8 +110,7 @@ public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) { .setErrorCode(apiError.error().code()) .setErrorMessage(apiError.message())); } - response.replicaElectionResults().add( - electionResult); + response.replicaElectionResults().add(electionResult); } return new ElectPreferredLeadersResponse(response); } diff --git a/clients/src/main/resources/common/message/README.md b/clients/src/main/resources/common/message/README.md index 5648f37812d7a..482b1dd29f8cd 100644 --- a/clients/src/main/resources/common/message/README.md +++ b/clients/src/main/resources/common/message/README.md @@ -187,7 +187,7 @@ One very common pattern in Kafka is to load array elements from a message into a Map or Set for easier access. The message protocol makes this easier with the "mapKey" concept. -If some of the elemements of an array are annotated with "mapKey": true, the +If some of the elements of an array are annotated with "mapKey": true, the entire array will be treated as a linked hash set rather than a list. Elements in this set will be accessible in O(1) time with an automatically generated "find" function. The order of elements in the set will still be preserved, 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 2955658ce0d5d..12b076d08bc90 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 @@ -38,6 +38,7 @@ import org.apache.kafka.common.acl.AclPermissionType; import org.apache.kafka.common.config.ConfigResource; import org.apache.kafka.common.errors.AuthenticationException; +import org.apache.kafka.common.errors.ClusterAuthorizationException; import org.apache.kafka.common.errors.GroupAuthorizationException; import org.apache.kafka.common.errors.InvalidRequestException; import org.apache.kafka.common.errors.InvalidTopicException; @@ -50,6 +51,9 @@ import org.apache.kafka.common.errors.TopicDeletionDisabledException; import org.apache.kafka.common.errors.UnknownServerException; import org.apache.kafka.common.errors.UnknownTopicOrPartitionException; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.PartitionResult; +import org.apache.kafka.common.message.ElectPreferredLeadersResponseData.ReplicaElectionResult; import org.apache.kafka.common.protocol.Errors; import org.apache.kafka.common.requests.ApiError; import org.apache.kafka.common.requests.CreateAclsResponse; @@ -67,6 +71,7 @@ import org.apache.kafka.common.requests.DescribeAclsResponse; import org.apache.kafka.common.requests.DescribeConfigsResponse; import org.apache.kafka.common.requests.DescribeGroupsResponse; +import org.apache.kafka.common.requests.ElectPreferredLeadersResponse; import org.apache.kafka.common.requests.FindCoordinatorResponse; import org.apache.kafka.common.requests.ListGroupsResponse; import org.apache.kafka.common.requests.MetadataRequest; @@ -636,28 +641,41 @@ public void testDeleteAcls() throws Exception { @Test public void testElectPreferredLeaders() throws Exception { - /*TopicPartition topic1 = new TopicPartition("topic", 0); + TopicPartition topic1 = new TopicPartition("topic", 0); TopicPartition topic2 = new TopicPartition("topic", 2); try (AdminClientUnitTestEnv env = mockClientEnv()) { env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); // Test a call where one partition has an error. - HashMap map = new HashMap<>(); - map.put(topic1, ApiError.NONE); - map.put(topic2, ApiError.fromThrowable(new ClusterAuthorizationException(null))); - env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(0, - map)); + ApiError value = ApiError.fromThrowable(new ClusterAuthorizationException(null)); + ElectPreferredLeadersResponseData responseData = new ElectPreferredLeadersResponseData(); + ReplicaElectionResult r = new ReplicaElectionResult().setTopic(topic1.topic()); + r.partitionResult().add(new PartitionResult() + .setPartitionId(topic1.partition()) + .setErrorCode(ApiError.NONE.error().code()) + .setErrorMessage(ApiError.NONE.message())); + r.partitionResult().add(new PartitionResult() + .setPartitionId(topic2.partition()) + .setErrorCode(value.error().code()) + .setErrorMessage(value.message())); + responseData.replicaElectionResults().add(r); + env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(responseData)); ElectPreferredLeadersResult results = env.adminClient().electPreferredLeaders(asList(topic1, topic2)); results.partitionResult(topic1).get(); TestUtils.assertFutureError(results.partitionResult(topic2), ClusterAuthorizationException.class); TestUtils.assertFutureError(results.all(), ClusterAuthorizationException.class); // Test a call where there are no errors. - map.put(topic1, ApiError.NONE); - map.put(topic2, ApiError.NONE); - env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse( - ElectPreferredLeadersUtil.fromElectPreferredLeadersRequest()0, - map)); + r.partitionResult().clear(); + r.partitionResult().add(new PartitionResult() + .setPartitionId(topic1.partition()) + .setErrorCode(ApiError.NONE.error().code()) + .setErrorMessage(ApiError.NONE.message())); + r.partitionResult().add(new PartitionResult() + .setPartitionId(topic2.partition()) + .setErrorCode(ApiError.NONE.error().code()) + .setErrorMessage(ApiError.NONE.message())); + env.kafkaClient().prepareResponse(new ElectPreferredLeadersResponse(responseData)); results = env.adminClient().electPreferredLeaders(asList(topic1, topic2)); results.partitionResult(topic1).get(); @@ -667,7 +685,7 @@ public void testElectPreferredLeaders() throws Exception { results = env.adminClient().electPreferredLeaders(asList(topic1, topic2), new ElectPreferredLeadersOptions().timeoutMs(100)); TestUtils.assertFutureError(results.partitionResult(topic1), TimeoutException.class); TestUtils.assertFutureError(results.partitionResult(topic2), TimeoutException.class); - }*/ + } } /** diff --git a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java index a65ec3b1b6b30..2892bb67c62ea 100644 --- a/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java +++ b/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java @@ -1348,15 +1348,12 @@ private ElectPreferredLeadersRequest createElectPreferredLeadersRequest() { } private ElectPreferredLeadersResponse createElectPreferredLeadersResponse() { - //Map errors = new HashMap<>(); - //errors.put(new TopicPartition("myTopic", 0), ApiError.NONE); - //errors.put(new TopicPartition("myTopic", 1), ApiError.fromThrowable(Errors.UNKNOWN_TOPIC_OR_PARTITION.exception("blah"))); ElectPreferredLeadersResponseData data = new ElectPreferredLeadersResponseData().setThrottleTimeMs(200); ReplicaElectionResult resultsByTopic = new ReplicaElectionResult().setTopic("myTopic"); resultsByTopic.partitionResult().add(new PartitionResult().setPartitionId(0) .setErrorCode(Errors.NONE.code()) .setErrorMessage(Errors.NONE.message())); - resultsByTopic.partitionResult().add(new PartitionResult().setPartitionId(0) + resultsByTopic.partitionResult().add(new PartitionResult().setPartitionId(1) .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) .setErrorMessage(Errors.UNKNOWN_TOPIC_OR_PARTITION.message())); data.replicaElectionResults().add(resultsByTopic); diff --git a/core/src/main/scala/kafka/controller/ControllerEventManager.scala b/core/src/main/scala/kafka/controller/ControllerEventManager.scala index 9dc26982eaa36..54e3a9e126b6c 100644 --- a/core/src/main/scala/kafka/controller/ControllerEventManager.scala +++ b/core/src/main/scala/kafka/controller/ControllerEventManager.scala @@ -71,9 +71,8 @@ class ControllerEventManager(controllerId: Int, rateAndTimeMetrics: Map[Controll def clearAndPut(event: ControllerEvent): Unit = inLock(putLock) { queue.asScala.foreach(evt => - if (evt.isInstanceOf[PreemptableControllerEvent]) { + if (evt.isInstanceOf[PreemptableControllerEvent]) evt.asInstanceOf[PreemptableControllerEvent].preempt() - } ) queue.clear() put(event) diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index db025d91b6fe6..7b5120cfed25f 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -659,9 +659,8 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } return results; } finally { - if (electionType != AdminClientTriggered) { + if (electionType != AdminClientTriggered) removePartitionsFromPreferredReplicaElection(partitions, electionType == AutoTriggered) - } } } @@ -908,7 +907,6 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti zkClient.deletePreferredReplicaElection(controllerContext.epochZkVersion) // Ensure we detect future preferred replica leader elections eventManager.put(PreferredReplicaLeaderElection(None)) - } } @@ -1545,16 +1543,18 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } } - def electPreferredLeaders(partitions: Set[TopicPartition], callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = { (_,_) => }): Unit = + type ElectPreferredLeadersCallback = (Set[TopicPartition], Set[ElectPreferredLeaderMetadata], Map[TopicPartition, ApiError])=>Unit + + def electPreferredLeaders(partitions: Set[TopicPartition], callback: ElectPreferredLeadersCallback = { (_,_,_) => }): Unit = eventManager.put(PreferredReplicaLeaderElection(Some(partitions), AdminClientTriggered, callback)) case class PreferredReplicaLeaderElection(partitionsFromAdminClientOpt: Option[Set[TopicPartition]], electionType: ElectionType = ZkTriggered, - callback: (Set[TopicPartition], Map[TopicPartition, ApiError])=>Unit = (_,_) =>{}) extends PreemptableControllerEvent { + callback: ElectPreferredLeadersCallback = (_,_,_) =>{}) extends PreemptableControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance override def handlePreempt(): Unit = { - callback(Set(), partitionsFromAdminClientOpt match { + callback(Set(), Set(), partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap case None => Map.empty }) @@ -1562,7 +1562,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti override def handleProcess(): Unit = { if (!isActive) { - callback(Set(), partitionsFromAdminClientOpt match { + callback(Set(), Set(), partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap case None => Map.empty }) @@ -1608,7 +1608,10 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti invalidPartitions.map ( tp => tp -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition does not exist.") ) debug(s"PreferredReplicaLeaderElection waiting: $successfulPartitions, results: $results") - callback(successfulPartitions, results) + callback(successfulPartitions, + successfulPartitions.map( + tp => ElectPreferredLeaderMetadata(tp, controllerContext.partitionReplicaAssignment(tp).head)), + results) } } } @@ -1791,22 +1794,20 @@ sealed trait ControllerEvent { } /** - * A `ControllerEvent`, such as one with a client callback, which needs specific handing in the event of ZK session expiration. + * A `ControllerEvent`, such as one with a client callback, which needs specific handling in the event of ZK session expiration. */ sealed trait PreemptableControllerEvent extends ControllerEvent { val spent = new AtomicBoolean(false) final def preempt(): Unit = { - if (!spent.getAndSet(true)) { + if (!spent.getAndSet(true)) handlePreempt() - } } final def process(): Unit = { - if (!spent.getAndSet(true)) { + if (!spent.getAndSet(true)) handleProcess() - } } def handlePreempt(): Unit diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 880f698a4af83..f3513aa257e81 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -38,7 +38,6 @@ import kafka.security.auth.{Resource, _} import kafka.server.QuotaFactory.{QuotaManagers, UnboundedQuota} import kafka.utils.{CoreUtils, Logging} import kafka.zk.{AdminZkClient, KafkaZkClient} -import org.apache.kafka.common import org.apache.kafka.common.acl.{AccessControlEntry, AclBinding} import org.apache.kafka.common.config.ConfigResource import org.apache.kafka.common.errors._ diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 78b6e0ea7a416..cc69ac84e16c9 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1527,11 +1527,10 @@ class ReplicaManager(val config: KafkaConfig, val deadline = time.milliseconds() + requestTimeout - def electionCallback(waiting: Set[TopicPartition], results: Map[TopicPartition, ApiError]): Unit = { + def electionCallback(waiting: Set[TopicPartition], + expectedLeaders: Set[ElectPreferredLeaderMetadata], + results: Map[TopicPartition, ApiError]): Unit = { if (waiting.nonEmpty) { - val expectedLeaders = waiting.map( - tp => ElectPreferredLeaderMetadata(tp, controller.controllerContext.partitionReplicaAssignment(tp).head)) - val watchKeys = waiting.map(p => new TopicPartitionOperationKey(p.topic, p.partition)).toSeq delayedElectPreferredLeaderPurgatory.tryCompleteElseWatch( new DelayedElectPreferredLeader(deadline - time.milliseconds(), expectedLeaders, results, diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index 1f9777f59ba14..56db2c39f7017 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package unit.kafka.admin +package kafka.admin import java.io.File import java.nio.charset.StandardCharsets From 1257d8fb0832d576c79631150d89d5244ea6de5d Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Thu, 24 Jan 2019 11:24:30 +0000 Subject: [PATCH 21/22] Fix test by make sure the bootstrap server is the controller --- .../admin/PreferredReplicaLeaderElectionCommandTest.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala index 56db2c39f7017..824e8fb4253ec 100644 --- a/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala +++ b/core/src/test/scala/unit/kafka/admin/PreferredReplicaLeaderElectionCommandTest.scala @@ -282,11 +282,12 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit val leader = getLeader(testPartition) assertNotEquals(testPartitionPreferredLeader, leader) // Now kill the controller just before we trigger the election - servers(getController().get.config.brokerId).shutdown() + val controller = getController().get.config.brokerId + servers(controller).shutdown() val jsonFile = toJsonFile(testPartitionAndAssignment.keySet) try { PreferredReplicaLeaderElectionCommand.run(Array( - "--bootstrap-server", bootstrapServer(), + "--bootstrap-server", bootstrapServer(controller), "--path-to-json-file", jsonFile.getAbsolutePath), timeout = 2000) fail(); From 9df18c6b3232f3e8b9bf879b7a8dd762a5968c6c Mon Sep 17 00:00:00 2001 From: Tom Bentley Date: Fri, 25 Jan 2019 09:57:01 +0000 Subject: [PATCH 22/22] Review comments --- .../message/ElectPreferredLeadersRequest.json | 2 +- .../kafka/controller/KafkaController.scala | 15 ++++++------- .../server/DelayedElectPreferredLeader.scala | 22 +++++++++---------- .../scala/kafka/server/ReplicaManager.scala | 9 ++++---- .../kafka/api/AuthorizerIntegrationTest.scala | 1 - 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json b/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json index 30a5f8c64bb80..f566cdf3f6416 100644 --- a/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json +++ b/clients/src/main/resources/common/message/ElectPreferredLeadersRequest.json @@ -27,7 +27,7 @@ { "name": "PartitionId", "type": "[]int32", "versions": "0+", "about": "The partitions of this topic whose preferred leader should be elected" } ]}, - { "name": "TimeoutMs", "type": "int32", "versions": "0+", + { "name": "TimeoutMs", "type": "int32", "versions": "0+", "default": "60000", "about": "The time in ms to wait for the election to complete." } ] } \ No newline at end of file diff --git a/core/src/main/scala/kafka/controller/KafkaController.scala b/core/src/main/scala/kafka/controller/KafkaController.scala index 7b5120cfed25f..ea23beb419234 100644 --- a/core/src/main/scala/kafka/controller/KafkaController.scala +++ b/core/src/main/scala/kafka/controller/KafkaController.scala @@ -1543,18 +1543,18 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti } } - type ElectPreferredLeadersCallback = (Set[TopicPartition], Set[ElectPreferredLeaderMetadata], Map[TopicPartition, ApiError])=>Unit + type ElectPreferredLeadersCallback = (Map[TopicPartition, Int], Map[TopicPartition, ApiError])=>Unit - def electPreferredLeaders(partitions: Set[TopicPartition], callback: ElectPreferredLeadersCallback = { (_,_,_) => }): Unit = + def electPreferredLeaders(partitions: Set[TopicPartition], callback: ElectPreferredLeadersCallback = { (_,_) => }): Unit = eventManager.put(PreferredReplicaLeaderElection(Some(partitions), AdminClientTriggered, callback)) case class PreferredReplicaLeaderElection(partitionsFromAdminClientOpt: Option[Set[TopicPartition]], electionType: ElectionType = ZkTriggered, - callback: ElectPreferredLeadersCallback = (_,_,_) =>{}) extends PreemptableControllerEvent { + callback: ElectPreferredLeadersCallback = (_,_) =>{}) extends PreemptableControllerEvent { override def state: ControllerState = ControllerState.ManualLeaderBalance override def handlePreempt(): Unit = { - callback(Set(), Set(), partitionsFromAdminClientOpt match { + callback(Map.empty, partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap case None => Map.empty }) @@ -1562,7 +1562,7 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti override def handleProcess(): Unit = { if (!isActive) { - callback(Set(), Set(), partitionsFromAdminClientOpt match { + callback(Map.empty, partitionsFromAdminClientOpt match { case Some(partitions) => partitions.map(partition => partition -> new ApiError(Errors.NOT_CONTROLLER, null)).toMap case None => Map.empty }) @@ -1608,9 +1608,8 @@ class KafkaController(val config: KafkaConfig, zkClient: KafkaZkClient, time: Ti invalidPartitions.map ( tp => tp -> new ApiError(Errors.UNKNOWN_TOPIC_OR_PARTITION, s"The partition does not exist.") ) debug(s"PreferredReplicaLeaderElection waiting: $successfulPartitions, results: $results") - callback(successfulPartitions, - successfulPartitions.map( - tp => ElectPreferredLeaderMetadata(tp, controllerContext.partitionReplicaAssignment(tp).head)), + callback(successfulPartitions.map( + tp => tp->controllerContext.partitionReplicaAssignment(tp).head).toMap, results) } } diff --git a/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala index 43eb600da8808..38b07ad5ce67a 100644 --- a/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala +++ b/core/src/main/scala/kafka/server/DelayedElectPreferredLeader.scala @@ -21,21 +21,19 @@ import org.apache.kafka.common.TopicPartition import org.apache.kafka.common.protocol.Errors import org.apache.kafka.common.requests.ApiError -import scala.collection.{Map, Set, mutable} - -case class ElectPreferredLeaderMetadata(partition: TopicPartition, leader: Int) +import scala.collection.{Map, mutable} /** A delayed elect preferred leader operation that can be created by the replica manager and watched * in the elect preferred leader purgatory */ class DelayedElectPreferredLeader(delayMs: Long, - expectedLeaders: Set[ElectPreferredLeaderMetadata], + expectedLeaders: Map[TopicPartition, Int], results: Map[TopicPartition, ApiError], replicaManager: ReplicaManager, responseCallback: Map[TopicPartition, ApiError] => Unit) extends DelayedOperation(delayMs) { - var waitingPartitions = expectedLeaders.to[mutable.Set] + var waitingPartitions = expectedLeaders val fullResults = results.to[mutable.Set] @@ -51,7 +49,9 @@ class DelayedElectPreferredLeader(delayMs: Long, override def onComplete(): Unit = { // This could be called to force complete, so I need the full list of partitions, so I can time them all out. updateWaiting() - val timedout = waitingPartitions.map(meta => meta.partition -> new ApiError(Errors.REQUEST_TIMED_OUT, null)).toMap + val timedout = waitingPartitions.map{ + case (tp, leader) => tp -> new ApiError(Errors.REQUEST_TIMED_OUT, null) + }.toMap responseCallback(timedout ++ fullResults) } @@ -73,13 +73,13 @@ class DelayedElectPreferredLeader(delayMs: Long, } private def updateWaiting() = { - waitingPartitions.foreach{m => - val ps = replicaManager.metadataCache.getPartitionInfo(m.partition.topic, m.partition.partition) + waitingPartitions.foreach{case (tp, leader) => + val ps = replicaManager.metadataCache.getPartitionInfo(tp.topic, tp.partition) ps match { case Some(ps) => - if (m.leader == ps.basePartitionState.leader) { - waitingPartitions -= m - fullResults += m.partition -> ApiError.NONE + if (leader == ps.basePartitionState.leader) { + waitingPartitions -= tp + fullResults += tp -> ApiError.NONE } case None => } diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index cc69ac84e16c9..5e41e35f87981 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1527,11 +1527,12 @@ class ReplicaManager(val config: KafkaConfig, val deadline = time.milliseconds() + requestTimeout - def electionCallback(waiting: Set[TopicPartition], - expectedLeaders: Set[ElectPreferredLeaderMetadata], + def electionCallback(expectedLeaders: Map[TopicPartition, Int], results: Map[TopicPartition, ApiError]): Unit = { - if (waiting.nonEmpty) { - val watchKeys = waiting.map(p => new TopicPartitionOperationKey(p.topic, p.partition)).toSeq + if (expectedLeaders.nonEmpty) { + val watchKeys = expectedLeaders.map{ + case (tp, leader) => new TopicPartitionOperationKey(tp.topic, tp.partition) + }.toSeq delayedElectPreferredLeaderPurgatory.tryCompleteElseWatch( new DelayedElectPreferredLeader(deadline - time.milliseconds(), expectedLeaders, results, this, responseCallback), diff --git a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala index 93b49ee538939..ad7fdbbf5a3c1 100644 --- a/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala +++ b/core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala @@ -229,7 +229,6 @@ class AuthorizerIntegrationTest extends BaseRequestTest { ApiKeys.ALTER_REPLICA_LOG_DIRS -> clusterAlterAcl, ApiKeys.DESCRIBE_LOG_DIRS -> clusterDescribeAcl, ApiKeys.CREATE_PARTITIONS -> topicAlterAcl, - ApiKeys.CREATE_PARTITIONS -> topicAlterAcl, ApiKeys.ELECT_PREFERRED_LEADERS -> clusterAlterAcl )