rpuch commented on code in PR #3792: URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611093149
########## modules/cli/src/main/java/org/apache/ignite/internal/cli/call/recovery/restart/RestartPartitionsCall.java: ########## @@ -0,0 +1,60 @@ +/* + * 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.ignite.internal.cli.call.recovery.restart; + +import jakarta.inject.Singleton; +import org.apache.ignite.internal.cli.core.call.Call; +import org.apache.ignite.internal.cli.core.call.DefaultCallOutput; +import org.apache.ignite.internal.cli.core.exception.IgniteCliApiException; +import org.apache.ignite.internal.cli.core.rest.ApiClientFactory; +import org.apache.ignite.rest.client.api.RecoveryApi; +import org.apache.ignite.rest.client.invoker.ApiException; +import org.apache.ignite.rest.client.model.RestartPartitionsRequest; + +/** Call to restart partitions. */ +@Singleton +public class RestartPartitionsCall implements Call<RestartPartitionsCallInput, String> { + private final ApiClientFactory clientFactory; + + public RestartPartitionsCall(ApiClientFactory clientFactory) { + this.clientFactory = clientFactory; + } + + @Override + public DefaultCallOutput<String> execute(RestartPartitionsCallInput input) { + RecoveryApi client = new RecoveryApi(clientFactory.getClient(input.clusterUrl())); + + RestartPartitionsRequest command = new RestartPartitionsRequest(); + + command.setPartitionIds(input.partitionIds()); + command.setNodeNames(input.nodeNames()); + command.setTableName(input.tableName().trim()); + command.setZoneName(input.zoneName().trim()); Review Comment: Are you sure it is correct to trim here? Can't a table/zone name contain spaces heading/trailing spaces? ########## modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/restart/ItRestartPartitionsTest.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.ignite.internal.cli.commands.recovery.restart; + +import static java.util.stream.Collectors.joining; +import static org.apache.ignite.internal.TestDefaultProfilesNames.DEFAULT_AIPERSIST_PROFILE_NAME; +import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_NODE_NAMES_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PARTITION_IDS_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PURGE_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_TABLE_NAME_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_ZONE_NAME_OPTION; + +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.cli.CliIntegrationTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** Base test class for Recovery restart partitions commands. */ Review Comment: ```suggestion /** Base test class for Cluster Recovery restart partitions commands. */ ``` ########## modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/restart/ItRestartPartitionsTest.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.ignite.internal.cli.commands.recovery.restart; + +import static java.util.stream.Collectors.joining; +import static org.apache.ignite.internal.TestDefaultProfilesNames.DEFAULT_AIPERSIST_PROFILE_NAME; +import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_NODE_NAMES_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PARTITION_IDS_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PURGE_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_TABLE_NAME_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_ZONE_NAME_OPTION; + +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.cli.CliIntegrationTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** Base test class for Recovery restart partitions commands. */ +public abstract class ItRestartPartitionsTest extends CliIntegrationTest { + private static final String ZONE = "first_ZONE"; + + private static final String TABLE_NAME = "first_ZONE_table"; + + private static final String QUALIFIED_TABLE_NAME = "PUBLIC." + TABLE_NAME; + + private static final int DEFAULT_PARTITION_COUNT = 25; + + @BeforeAll + public void createTables() { + sql(String.format("CREATE ZONE \"%s\" WITH storage_profiles='%s'", ZONE, DEFAULT_AIPERSIST_PROFILE_NAME)); + sql(String.format("CREATE TABLE PUBLIC.\"%s\" (id INT PRIMARY KEY, val INT) WITH PRIMARY_ZONE = '%s'", TABLE_NAME, ZONE)); + } + + @Test + public void testRestartAllPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartAllPartitionsPurge() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PURGE_OPTION + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions with cleanup."); + } + + @Test + public void testRestartSpecifiedPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "1,2" Review Comment: Do we have a test that makes sure that only the requested partitions are restarted? ########## modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/recovery/RestartPartitionsRequest.java: ########## @@ -0,0 +1,101 @@ +/* + * 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.ignite.internal.rest.api.recovery; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import java.util.Collection; +import java.util.Objects; +import java.util.Set; +import org.apache.ignite.internal.tostring.S; +import org.jetbrains.annotations.Nullable; + +/** Disaster recovery request to restart partitions. */ +@Schema(description = "restart partitions configuration.") +public class RestartPartitionsRequest { + @Schema(description = "Names specifying nodes to restart partitions. Case-sensitive, empty set means all nodes.") + private final Set<String> nodeNames; + + @Schema(description = "Name of the zone to restart partitions of. Without quotes, case-sensitive.") + private final String zoneName; + + @Schema(description = "IDs of partitions to restart. All if empty.") + private final Set<Integer> partitionIds; + + @Schema(description = "Fully-qualified name of the table to restart partitions of. Without quotes, case-sensitive.") + private final String tableName; + + @Schema(description = "If partitions should be cleaned.") Review Comment: This is a very delicate (and dangerous) parameter, and we probably must be more specific in the description of what 'cleaned' means here: destruction of data of the given partition on the given node (if I understand its behavior correctly). ########## modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/restart/ItRestartPartitionsTest.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.ignite.internal.cli.commands.recovery.restart; + +import static java.util.stream.Collectors.joining; +import static org.apache.ignite.internal.TestDefaultProfilesNames.DEFAULT_AIPERSIST_PROFILE_NAME; +import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_NODE_NAMES_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PARTITION_IDS_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PURGE_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_TABLE_NAME_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_ZONE_NAME_OPTION; + +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.cli.CliIntegrationTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** Base test class for Recovery restart partitions commands. */ +public abstract class ItRestartPartitionsTest extends CliIntegrationTest { + private static final String ZONE = "first_ZONE"; + + private static final String TABLE_NAME = "first_ZONE_table"; + + private static final String QUALIFIED_TABLE_NAME = "PUBLIC." + TABLE_NAME; + + private static final int DEFAULT_PARTITION_COUNT = 25; + + @BeforeAll + public void createTables() { + sql(String.format("CREATE ZONE \"%s\" WITH storage_profiles='%s'", ZONE, DEFAULT_AIPERSIST_PROFILE_NAME)); + sql(String.format("CREATE TABLE PUBLIC.\"%s\" (id INT PRIMARY KEY, val INT) WITH PRIMARY_ZONE = '%s'", TABLE_NAME, ZONE)); + } + + @Test + public void testRestartAllPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartAllPartitionsPurge() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, Review Comment: Do we have tests that verify what happens if a mandatory parameter is omitted (like zone or table)? ########## modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/recovery/RestartPartitionsRequest.java: ########## @@ -0,0 +1,101 @@ +/* + * 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.ignite.internal.rest.api.recovery; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import java.util.Collection; +import java.util.Objects; +import java.util.Set; +import org.apache.ignite.internal.tostring.S; +import org.jetbrains.annotations.Nullable; + +/** Disaster recovery request to restart partitions. */ +@Schema(description = "restart partitions configuration.") +public class RestartPartitionsRequest { + @Schema(description = "Names specifying nodes to restart partitions. Case-sensitive, empty set means all nodes.") + private final Set<String> nodeNames; + + @Schema(description = "Name of the zone to restart partitions of. Without quotes, case-sensitive.") + private final String zoneName; + + @Schema(description = "IDs of partitions to restart. All if empty.") Review Comment: ```suggestion @Schema(description = "IDs of partitions to restart. If empty/omitted, all partitions will be restarted.") ``` ########## modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/restart/ItRestartPartitionsTest.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.ignite.internal.cli.commands.recovery.restart; + +import static java.util.stream.Collectors.joining; +import static org.apache.ignite.internal.TestDefaultProfilesNames.DEFAULT_AIPERSIST_PROFILE_NAME; +import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_NODE_NAMES_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PARTITION_IDS_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PURGE_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_TABLE_NAME_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_ZONE_NAME_OPTION; + +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.cli.CliIntegrationTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** Base test class for Recovery restart partitions commands. */ +public abstract class ItRestartPartitionsTest extends CliIntegrationTest { + private static final String ZONE = "first_ZONE"; + + private static final String TABLE_NAME = "first_ZONE_table"; + + private static final String QUALIFIED_TABLE_NAME = "PUBLIC." + TABLE_NAME; + + private static final int DEFAULT_PARTITION_COUNT = 25; + + @BeforeAll + public void createTables() { + sql(String.format("CREATE ZONE \"%s\" WITH storage_profiles='%s'", ZONE, DEFAULT_AIPERSIST_PROFILE_NAME)); + sql(String.format("CREATE TABLE PUBLIC.\"%s\" (id INT PRIMARY KEY, val INT) WITH PRIMARY_ZONE = '%s'", TABLE_NAME, ZONE)); + } + + @Test + public void testRestartAllPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartAllPartitionsPurge() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PURGE_OPTION + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions with cleanup."); + } + + @Test + public void testRestartSpecifiedPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "1,2" + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartPartitionsByNodes() { + String nodeNames = CLUSTER.runningNodes() + .limit(initialNodes() - 1) + .map(IgniteImpl::name) + .collect(joining(",")); + + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "1,2", + RECOVERY_NODE_NAMES_OPTION, nodeNames Review Comment: Do we have a test that makes sure that only the requested nodes are affected? ########## modules/cli/src/main/java/org/apache/ignite/internal/cli/call/recovery/restart/RestartPartitionsCallInput.java: ########## @@ -0,0 +1,167 @@ +/* + * 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.ignite.internal.cli.call.recovery.restart; + +import java.util.List; +import org.apache.ignite.internal.cli.commands.recovery.restart.RestartPartitionsMixin; +import org.apache.ignite.internal.cli.core.call.CallInput; +import org.jetbrains.annotations.Nullable; + +/** Input for the {@link RestartPartitionsCall} call. */ +public class RestartPartitionsCallInput implements CallInput { + private final String clusterUrl; + + private final String zoneName; + + private final String tableName; + + private final List<String> nodeNames; + + private final List<Integer> partitionIds; + + private final boolean purge; + + /** Cluster url. */ + public String clusterUrl() { + return clusterUrl; + } + + /** Returns zone name to restart partitions of. */ + public String zoneName() { + return zoneName; + } + + /** Returns table name to restart partitions of. */ + public String tableName() { + return tableName; + } + + /** IDs of partitions to restart. */ + public List<Integer> partitionIds() { + return partitionIds; + } + + /** Returns names of nodes to restart partitions of. */ Review Comment: Please elaborate what happens for empty/null list ########## modules/cli/src/main/java/org/apache/ignite/internal/cli/call/recovery/restart/RestartPartitionsCall.java: ########## @@ -0,0 +1,60 @@ +/* + * 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.ignite.internal.cli.call.recovery.restart; + +import jakarta.inject.Singleton; +import org.apache.ignite.internal.cli.core.call.Call; +import org.apache.ignite.internal.cli.core.call.DefaultCallOutput; +import org.apache.ignite.internal.cli.core.exception.IgniteCliApiException; +import org.apache.ignite.internal.cli.core.rest.ApiClientFactory; +import org.apache.ignite.rest.client.api.RecoveryApi; +import org.apache.ignite.rest.client.invoker.ApiException; +import org.apache.ignite.rest.client.model.RestartPartitionsRequest; + +/** Call to restart partitions. */ +@Singleton +public class RestartPartitionsCall implements Call<RestartPartitionsCallInput, String> { + private final ApiClientFactory clientFactory; + + public RestartPartitionsCall(ApiClientFactory clientFactory) { + this.clientFactory = clientFactory; + } + + @Override + public DefaultCallOutput<String> execute(RestartPartitionsCallInput input) { + RecoveryApi client = new RecoveryApi(clientFactory.getClient(input.clusterUrl())); + + RestartPartitionsRequest command = new RestartPartitionsRequest(); + + command.setPartitionIds(input.partitionIds()); + command.setNodeNames(input.nodeNames()); + command.setTableName(input.tableName().trim()); + command.setZoneName(input.zoneName().trim()); + command.setPurge(input.purge()); + + try { + client.restartPartitions(command); + + return DefaultCallOutput.success( + "Restarting partitions with" + (input.purge() ? "" : "out") + " cleanup." Review Comment: Does the response arrive when we *start* restarting, or when the restart has *already* been done? Judging by the code of the manager, I suspect the latter, but the message says 'restarting' like it was just started. ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java: ########## @@ -246,9 +246,17 @@ public CompletableFuture<Void> resetPartitions(String zoneName, String tableName * @param zoneName Name of the distribution zone. Case-sensitive, without quotes. * @param tableName Fully-qualified table name. Case-sensitive, without quotes. Example: "PUBLIC.Foo". * @param partitionIds IDs of partitions to restart. If empty, restart all zone's partitions. + * @param purge If partitions should be cleaned. Review Comment: Please elaborate what 'cleaning' means ########## modules/cli/src/main/java/org/apache/ignite/internal/cli/call/recovery/restart/RestartPartitionsCallInput.java: ########## @@ -0,0 +1,167 @@ +/* + * 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.ignite.internal.cli.call.recovery.restart; + +import java.util.List; +import org.apache.ignite.internal.cli.commands.recovery.restart.RestartPartitionsMixin; +import org.apache.ignite.internal.cli.core.call.CallInput; +import org.jetbrains.annotations.Nullable; + +/** Input for the {@link RestartPartitionsCall} call. */ +public class RestartPartitionsCallInput implements CallInput { + private final String clusterUrl; + + private final String zoneName; + + private final String tableName; + + private final List<String> nodeNames; + + private final List<Integer> partitionIds; + + private final boolean purge; + + /** Cluster url. */ + public String clusterUrl() { + return clusterUrl; + } + + /** Returns zone name to restart partitions of. */ + public String zoneName() { + return zoneName; + } + + /** Returns table name to restart partitions of. */ + public String tableName() { + return tableName; + } + + /** IDs of partitions to restart. */ Review Comment: Please elaborate what happens for empty/null list ########## modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/restart/ItRestartPartitionsTest.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.ignite.internal.cli.commands.recovery.restart; + +import static java.util.stream.Collectors.joining; +import static org.apache.ignite.internal.TestDefaultProfilesNames.DEFAULT_AIPERSIST_PROFILE_NAME; +import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_NODE_NAMES_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PARTITION_IDS_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_PURGE_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_TABLE_NAME_OPTION; +import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_ZONE_NAME_OPTION; + +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.cli.CliIntegrationTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** Base test class for Recovery restart partitions commands. */ +public abstract class ItRestartPartitionsTest extends CliIntegrationTest { + private static final String ZONE = "first_ZONE"; + + private static final String TABLE_NAME = "first_ZONE_table"; + + private static final String QUALIFIED_TABLE_NAME = "PUBLIC." + TABLE_NAME; + + private static final int DEFAULT_PARTITION_COUNT = 25; + + @BeforeAll + public void createTables() { + sql(String.format("CREATE ZONE \"%s\" WITH storage_profiles='%s'", ZONE, DEFAULT_AIPERSIST_PROFILE_NAME)); + sql(String.format("CREATE TABLE PUBLIC.\"%s\" (id INT PRIMARY KEY, val INT) WITH PRIMARY_ZONE = '%s'", TABLE_NAME, ZONE)); + } + + @Test + public void testRestartAllPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartAllPartitionsPurge() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PURGE_OPTION + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions with cleanup."); + } + + @Test + public void testRestartSpecifiedPartitions() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "1,2" + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartPartitionsByNodes() { + String nodeNames = CLUSTER.runningNodes() + .limit(initialNodes() - 1) + .map(IgniteImpl::name) + .collect(joining(",")); + + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "1,2", + RECOVERY_NODE_NAMES_OPTION, nodeNames + ); + + assertErrOutputIsEmpty(); + assertOutputContains("Restarting partitions without cleanup."); + } + + @Test + public void testRestartPartitionZoneNotFound() { + String unknownZone = "unknown_zone"; + + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, unknownZone, + RECOVERY_PARTITION_IDS_OPTION, "1,2" + ); + + assertOutputIsEmpty(); + assertErrOutputContains("Distribution zone was not found [zoneName=" + unknownZone + "]"); + } + + @Test + public void testRestartPartitionTableNotFound() { + String unknownTable = "PUBLIC.unknown_table"; + + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, unknownTable, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertOutputIsEmpty(); + assertErrOutputContains("The table does not exist [name=" + unknownTable + "]"); + } + + @Test + public void testRestartPartitionNodeNotFound() { + String unknownNode = "unknownNode"; + + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_NODE_NAMES_OPTION, unknownNode, + RECOVERY_ZONE_NAME_OPTION, ZONE + ); + + assertOutputIsEmpty(); + assertErrOutputContains("Some nodes are missing: [" + unknownNode + "]"); + } + + @Test + public void testRestartPartitionsIllegalPartitionNegative() { + execute(CLUSTER_URL_OPTION, NODE_URL, + RECOVERY_TABLE_NAME_OPTION, QUALIFIED_TABLE_NAME, + RECOVERY_ZONE_NAME_OPTION, ZONE, + RECOVERY_PARTITION_IDS_OPTION, "0,5,-1,-10" Review Comment: Why do we have -1 here? Does the command report all illegal IDs? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
