You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Phillippko (via GitHub)" <gi...@apache.org> on 2024/05/20 13:37:23 UTC

[PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Phillippko opened a new pull request, #3792:
URL: https://github.com/apache/ignite-3/pull/3792

   https://issues.apache.org/jira/browse/IGNITE-22282
   
   Thank you for submitting the pull request.
   
   To streamline the review process of the patch and ensure better code quality
   we ask both an author and a reviewer to verify the following:
   
   ### The Review Checklist
   - [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:  
   \- There is a single JIRA ticket related to the pull request.  
   \- The web-link to the pull request is attached to the JIRA ticket.  
   \- The JIRA ticket has the Patch Available state.  
   \- The description of the JIRA ticket explains WHAT was made, WHY and HOW.  
   \- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
   - [ ] **Design:** new code conforms with the design principles of the components it is added to.
   - [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
   - [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
   - [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
   
   ### Notes
   - [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)


-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1612897043


##########
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:
   I would expect the CLI command to complete when the response is returned, which happens when future completes. It seems that this has to be verified.



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 REST/CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch merged PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792


-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611253318


##########
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:
   Actually it reports minimal negative or maximum exceeding the limit number. But it's a detail of implementation, so removed -1 from test case.



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1612955572


##########
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:
   You're right, we wait for restart. I was confused because of snapshot commands (they complete when process is started, but only because future doesn't wait for it to finish). Fixed 



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1612901113


##########
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:
   Yep, in this case it's enought to make sure that the needed parameters get passed  to each next layer.



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611144571


##########
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:
   In slack thread Ivan Bessonov said that for CLI / REST test we should just check that CLI / REST works and we get "success". Restart logic should be tested in DisasterRecoveryManager and it's expensive to duplicate this checks in every REST / CLI test - sometimes it's not trivial. 
   
   IMHO we need unit tests, to check that CLI calls REST with correct parameters... Will add them



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611267111


##########
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:
   Manager returns future, CLI command completes when the future is returned, not when it's finished



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611144571


##########
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:
   In slack thread Ivan Bessonov said that for CLI / REST test we should just check that CLI / REST works and we get "success". Restart logic should be tested in DisasterRecoveryManager and it's expensive to duplicate this checks in every REST / CLI test - sometimes it's not trivial. 
   
   IMHO we need unit tests, to check that CLI calls REST with correct parameters...



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611144571


##########
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:
   In slack thread Ivan Bessonov said that for CLI / REST test we should just check that CLI / REST works and we get "success". Restart logic should be tested in DisasterRecoveryManager and it's expensive to duplicate this checks in every REST / CLI test - sometimes it's not trivial. 
   
   IMHO we need unit tests, to check that CLI calls REST with correct parameters... Added them



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611267111


##########
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:
   Manager returns future, CLI command completes when the future is returned, not when it's finished
   
   Could you elaborate why you suspect otherwise? Maybe I'm missing something



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611614974


##########
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:
   But do we have a test that verifies this? :)



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1611272149


##########
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:
   CLI framework verifies that parameters fit into command contract (checks exclusive, mandatory parameters, etc)



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 REST/CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on code in PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#discussion_r1620661586


##########
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:
   Talked to Mikhail, we usually don't write such tests)



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-22282 CLI for disaster recovery: restart-partitions [ignite-3]

Posted by "Phillippko (via GitHub)" <gi...@apache.org>.
Phillippko commented on PR #3792:
URL: https://github.com/apache/ignite-3/pull/3792#issuecomment-2132763026

   @Pochatkin Hello! Could CLI team also take a look? 


-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org