-
Notifications
You must be signed in to change notification settings - Fork 11
feat(inkless:switch): implement AlterDisklessSwitch and tooling [KC-97] #665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0ef5df9
057faa7
abc803f
449e09b
c98755c
5445261
947c4dd
93f9909
f099337
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| /** | ||
| * Options for {@link Admin#alterDisklessSwitch(String, int, long, AlterDisklessSwitchOptions)}. | ||
| * | ||
| * The API of this class is evolving, see {@link Admin} for details. | ||
| */ | ||
| @InterfaceStability.Unstable | ||
| public class AlterDisklessSwitchOptions extends AbstractOptions<AlterDisklessSwitchOptions> { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * 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.annotation.InterfaceStability; | ||
|
|
||
| /** | ||
| * The result of the {@link Admin#alterDisklessSwitch(String, int, long, AlterDisklessSwitchOptions)} call. | ||
| * | ||
| * The API of this class is evolving, see {@link Admin} for details. | ||
| */ | ||
| @InterfaceStability.Unstable | ||
| public class AlterDisklessSwitchResult { | ||
| private final KafkaFuture<Void> future; | ||
|
|
||
| AlterDisklessSwitchResult(final KafkaFuture<Void> future) { | ||
| this.future = future; | ||
| } | ||
|
|
||
| /** | ||
| * Return a future which succeeds if the operation is successful. | ||
| */ | ||
| public KafkaFuture<Void> all() { | ||
| return future; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,6 +108,7 @@ | |
| import org.apache.kafka.common.errors.UnsupportedVersionException; | ||
| import org.apache.kafka.common.internals.KafkaFutureImpl; | ||
| import org.apache.kafka.common.message.AddRaftVoterRequestData; | ||
| import org.apache.kafka.common.message.AlterDisklessSwitchRequestData; | ||
| import org.apache.kafka.common.message.AlterPartitionReassignmentsRequestData; | ||
| import org.apache.kafka.common.message.AlterPartitionReassignmentsRequestData.ReassignableTopic; | ||
| import org.apache.kafka.common.message.AlterReplicaLogDirsRequestData; | ||
|
|
@@ -186,6 +187,8 @@ | |
| import org.apache.kafka.common.requests.AddRaftVoterResponse; | ||
| import org.apache.kafka.common.requests.AlterClientQuotasRequest; | ||
| import org.apache.kafka.common.requests.AlterClientQuotasResponse; | ||
| import org.apache.kafka.common.requests.AlterDisklessSwitchRequest; | ||
| import org.apache.kafka.common.requests.AlterDisklessSwitchResponse; | ||
| import org.apache.kafka.common.requests.AlterPartitionReassignmentsRequest; | ||
| import org.apache.kafka.common.requests.AlterPartitionReassignmentsResponse; | ||
| import org.apache.kafka.common.requests.AlterReplicaLogDirsRequest; | ||
|
|
@@ -2508,9 +2511,11 @@ void handleResponse(AbstractResponse abstractResponse) { | |
| "Topic " + topic.name() + " changed while fetching paginated response")); | ||
| return; | ||
| } | ||
| existing.partitions().addAll(topic.partitions()); | ||
| existing.partitions().addAll(copyPartitionsWithTaggedFields(topic.partitions())); | ||
| } else { | ||
| accumulated.topics().add(topic.duplicate()); | ||
| DescribeTopicPartitionsResponseTopic copy = topic.duplicate(); | ||
| copy.setPartitions(copyPartitionsWithTaggedFields(topic.partitions())); | ||
| accumulated.topics().add(copy); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -2531,6 +2536,18 @@ void handleFailure(Throwable throwable) { | |
| return new DescribeTopicPartitionsResult(future); | ||
| } | ||
|
|
||
| private static List<DescribeTopicPartitionsResponsePartition> copyPartitionsWithTaggedFields( | ||
| List<DescribeTopicPartitionsResponsePartition> partitions) { | ||
| List<DescribeTopicPartitionsResponsePartition> copies = new ArrayList<>(partitions.size()); | ||
| for (DescribeTopicPartitionsResponsePartition partition : partitions) { | ||
| DescribeTopicPartitionsResponsePartition copy = partition.duplicate(); | ||
| // duplicate() drops the unknown tagged fields | ||
| copy.unknownTaggedFields().addAll(partition.unknownTaggedFields()); | ||
| copies.add(copy); | ||
| } | ||
| return copies; | ||
| } | ||
|
|
||
| @Override | ||
| public DescribeClusterResult describeCluster(DescribeClusterOptions options) { | ||
| final KafkaFutureImpl<Collection<Node>> describeClusterFuture = new KafkaFutureImpl<>(); | ||
|
|
@@ -4882,6 +4899,43 @@ void handleFailure(Throwable throwable) { | |
| return new UnregisterBrokerResult(future); | ||
| } | ||
|
|
||
| @Override | ||
| public AlterDisklessSwitchResult alterDisklessSwitch(String topic, int partition, long sealOffset, | ||
| AlterDisklessSwitchOptions options) { | ||
| final KafkaFutureImpl<Void> future = new KafkaFutureImpl<>(); | ||
| final long now = time.milliseconds(); | ||
| final Call call = new Call("alterDisklessSwitch", calcDeadlineMs(now, options.timeoutMs()), | ||
| new LeastLoadedBrokerOrActiveKController()) { | ||
|
|
||
| @Override | ||
| AlterDisklessSwitchRequest.Builder createRequest(int timeoutMs) { | ||
| AlterDisklessSwitchRequestData data = new AlterDisklessSwitchRequestData() | ||
| .setTopicName(topic) | ||
| .setPartitionIndex(partition) | ||
| .setSealOffset(sealOffset); | ||
| return new AlterDisklessSwitchRequest.Builder(data); | ||
| } | ||
|
|
||
| @Override | ||
| void handleResponse(AbstractResponse abstractResponse) { | ||
| final AlterDisklessSwitchResponse response = (AlterDisklessSwitchResponse) abstractResponse; | ||
| Errors error = Errors.forCode(response.data().errorCode()); | ||
| if (error == Errors.NONE) { | ||
| future.complete(null); | ||
| } else { | ||
| future.completeExceptionally(error.exception(response.data().errorMessage())); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| void handleFailure(Throwable throwable) { | ||
| future.completeExceptionally(throwable); | ||
| } | ||
| }; | ||
| runnable.call(call, now); | ||
| return new AlterDisklessSwitchResult(future); | ||
| } | ||
|
|
||
|
Comment on lines
+4888
to
+4924
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That seems inconsistent with the intended state model: -1 means not switched/aborted, and -2 means pending/re-armed. In both states, producer states and disklessLeaderEpoch should be chosen again by the next initDisklessLog flow, not carried over from the previous completed switch. Otherwise the metadata can contain stale switch state even after an abort or re-arm. I think this path should explicitly clear dependent switch metadata when sealOffset < 0, probably by making
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a follow up, in case of sealOffset >= 0, we should also give the possibility to the client to reset the |
||
| @Override | ||
| public DescribeProducersResult describeProducers(Collection<TopicPartition> topicPartitions, DescribeProducersOptions options) { | ||
| PartitionLeaderStrategy.PartitionLeaderFuture<DescribeProducersResult.PartitionProducerState> future = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| /* | ||
| * 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.message.AlterDisklessSwitchRequestData; | ||
| import org.apache.kafka.common.message.AlterDisklessSwitchResponseData; | ||
| import org.apache.kafka.common.protocol.ApiKeys; | ||
| import org.apache.kafka.common.protocol.Readable; | ||
|
|
||
| public class AlterDisklessSwitchRequest extends AbstractRequest { | ||
|
|
||
| public static class Builder extends AbstractRequest.Builder<AlterDisklessSwitchRequest> { | ||
| private final AlterDisklessSwitchRequestData data; | ||
|
|
||
| public Builder(AlterDisklessSwitchRequestData data) { | ||
| super(ApiKeys.ALTER_DISKLESS_SWITCH); | ||
| this.data = data; | ||
| } | ||
|
|
||
| @Override | ||
| public AlterDisklessSwitchRequest build(short version) { | ||
| return new AlterDisklessSwitchRequest(data, version); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return data.toString(); | ||
| } | ||
| } | ||
|
|
||
| private final AlterDisklessSwitchRequestData data; | ||
|
|
||
| public AlterDisklessSwitchRequest(AlterDisklessSwitchRequestData data, short version) { | ||
| super(ApiKeys.ALTER_DISKLESS_SWITCH, version); | ||
| this.data = data; | ||
| } | ||
|
|
||
| @Override | ||
| public AlterDisklessSwitchRequestData data() { | ||
| return data; | ||
| } | ||
|
|
||
| @Override | ||
| public AlterDisklessSwitchResponse getErrorResponse(int throttleTimeMs, Throwable e) { | ||
| ApiError apiError = ApiError.fromThrowable(e); | ||
| return new AlterDisklessSwitchResponse(new AlterDisklessSwitchResponseData() | ||
| .setThrottleTimeMs(throttleTimeMs) | ||
| .setErrorCode(apiError.error().code()) | ||
| .setErrorMessage(apiError.message())); | ||
| } | ||
|
|
||
| public static AlterDisklessSwitchRequest parse(Readable readable, short version) { | ||
| return new AlterDisklessSwitchRequest(new AlterDisklessSwitchRequestData(readable, version), version); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| /* | ||
| * 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.message.AlterDisklessSwitchResponseData; | ||
| import org.apache.kafka.common.protocol.ApiKeys; | ||
| import org.apache.kafka.common.protocol.Errors; | ||
| import org.apache.kafka.common.protocol.Readable; | ||
|
|
||
| import java.util.EnumMap; | ||
| import java.util.Map; | ||
|
|
||
| public class AlterDisklessSwitchResponse extends AbstractResponse { | ||
| private final AlterDisklessSwitchResponseData data; | ||
|
|
||
| public AlterDisklessSwitchResponse(AlterDisklessSwitchResponseData data) { | ||
| super(ApiKeys.ALTER_DISKLESS_SWITCH); | ||
| this.data = data; | ||
| } | ||
|
|
||
| @Override | ||
| public AlterDisklessSwitchResponseData data() { | ||
| return data; | ||
| } | ||
|
|
||
| @Override | ||
| public int throttleTimeMs() { | ||
| return data.throttleTimeMs(); | ||
| } | ||
|
|
||
| @Override | ||
| public void maybeSetThrottleTimeMs(int throttleTimeMs) { | ||
| data.setThrottleTimeMs(throttleTimeMs); | ||
| } | ||
|
|
||
| @Override | ||
| public Map<Errors, Integer> errorCounts() { | ||
| Map<Errors, Integer> errorCounts = new EnumMap<>(Errors.class); | ||
| if (data.errorCode() != 0) { | ||
| errorCounts.put(Errors.forCode(data.errorCode()), 1); | ||
| } | ||
| return errorCounts; | ||
| } | ||
|
|
||
| public static AlterDisklessSwitchResponse parse(Readable readable, short version) { | ||
| return new AlterDisklessSwitchResponse(new AlterDisklessSwitchResponseData(readable, version)); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldClientThrottle(short version) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some validation here? For example, abort the request if
disklessStartOffsetwas never set at all, and ifdiskless.enableis not set to true this should not work at all, as it's a recovery tool and not a tool to switch to diskless.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover, in case the switch is aborted,
diskless.enableshould also be reset to false.