You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/05 19:12:13 UTC

[GitHub] [ignite] shishkovilja commented on a diff in pull request #10328: IGNITE-17383 Stop IdleVerify process for an inactive cluster with per…

shishkovilja commented on code in PR #10328:
URL: https://github.com/apache/ignite/pull/10328#discussion_r1039923757


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/verify/VerifyBackupPartitionsTaskV2.java:
##########
@@ -193,6 +193,11 @@ public VerifyBackupPartitionsJobV2(VisorIdleVerifyTaskArg arg) {
 
         /** {@inheritDoc} */
         @Override public Map<PartitionKeyV2, PartitionHashRecordV2> execute() throws IgniteException {
+            if (!ignite.context().state().publicApiActiveState(true)) {

Review Comment:
   @J-Bakuli , can we use public API here like in example below?
   `ignite.cluster().state() == ClusterState.INACTIVE`



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {

Review Comment:
   @J-Bakuli , have you checked, that we have no positive scenario tests for IdleVerify?



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+        assertContains(log, testOut.toString(), "The check procedure has finished, no conflicts have been found.");
+    }
+
+    /**
+     * Tests idle_verify working on an inactive cluster with persistence. Works via control.sh.
+     *
+     * @throws IgniteException if succeeded.
+     */
+    @Test
+    public void testIdleVerifyOnInactiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(2);
+
+        ignite.cluster().state(INACTIVE);
+
+        assertFalse(ignite.cluster().active());

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/cache/IdleVerify.java:
##########
@@ -346,6 +347,9 @@ private void cacheIdleVerifyV2(
                 IDLE_VERIFY_FILE_PREFIX + LocalDateTime.now().format(TIME_FORMATTER) + ".txt");
 
             try (PrintWriter pw = new PrintWriter(f)) {
+                if (client.state().state() == ClusterState.INACTIVE)

Review Comment:
   @J-Bakuli , this section is for printing into file, it seems, that we should move it outside of `try` block.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/verify/VerifyBackupPartitionsTaskV2.java:
##########
@@ -193,6 +193,11 @@ public VerifyBackupPartitionsJobV2(VisorIdleVerifyTaskArg arg) {
 
         /** {@inheritDoc} */
         @Override public Map<PartitionKeyV2, PartitionHashRecordV2> execute() throws IgniteException {
+            if (!ignite.context().state().publicApiActiveState(true)) {
+                throw new IgniteException("Can not perform the operation because the cluster is inactive. Note, that " +
+                        "the cluster is considered inactive by default if Ignite Persistent Store is used to let all the nodes " +
+                        "join the cluster. To activate the cluster call Ignite.active(true).");
+            }

Review Comment:
   `Ignite#active(boolean)` is deprecated, should be changed to `Ignite.cluster().state(ClusterState.ACTIVE)`



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")

Review Comment:
   What purposes of this cache?



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+        assertContains(log, testOut.toString(), "The check procedure has finished, no conflicts have been found.");
+    }
+
+    /**
+     * Tests idle_verify working on an inactive cluster with persistence. Works via control.sh.
+     *
+     * @throws IgniteException if succeeded.
+     */
+    @Test
+    public void testIdleVerifyOnInactiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(2);
+
+        ignite.cluster().state(INACTIVE);
+
+        assertFalse(ignite.cluster().active());
+        assertEquals(INACTIVE, ignite.cluster().state());
+
+        injectTestSystemOut();
+
+        GridTestUtils.assertThrows(
+                log,
+                () -> ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                        .setDataRegionName("persistent-dataRegion")),
+                IgniteException.class,
+                "Can not perform the operation because the cluster is inactive. " +
+                        "Note, that the cluster is considered inactive by default if Ignite Persistent Store is used to " +
+                        "let all the nodes join the cluster. To activate the cluster call Ignite.active(true).");
+
+        execute("--cache", "idle_verify");
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        String out = testOut.toString();
+
+        assertContains(log, out, "idle_verify does not work on an inactive cluster with persistence");
+        assertFalse(ignite.cluster().active());
+        assertEquals(INACTIVE, ignite.cluster().state());

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+        assertContains(log, testOut.toString(), "The check procedure has finished, no conflicts have been found.");
+    }
+
+    /**
+     * Tests idle_verify working on an inactive cluster with persistence. Works via control.sh.
+     *
+     * @throws IgniteException if succeeded.
+     */
+    @Test
+    public void testIdleVerifyOnInactiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(2);
+
+        ignite.cluster().state(INACTIVE);

Review Comment:
   Cluster is inactive by default, it is unecessary.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+        assertContains(log, testOut.toString(), "The check procedure has finished, no conflicts have been found.");
+    }
+
+    /**
+     * Tests idle_verify working on an inactive cluster with persistence. Works via control.sh.
+     *
+     * @throws IgniteException if succeeded.
+     */
+    @Test
+    public void testIdleVerifyOnInactiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(2);
+
+        ignite.cluster().state(INACTIVE);
+
+        assertFalse(ignite.cluster().active());
+        assertEquals(INACTIVE, ignite.cluster().state());
+
+        injectTestSystemOut();
+
+        GridTestUtils.assertThrows(

Review Comment:
   IMHO, this check is excessive.



##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java:
##########
@@ -704,6 +704,77 @@ public void testClusterChangeTag() throws Exception {
         assertTrue("Tag has not been updated in 10 seconds", tagUpdated);
     }
 
+    /**
+     * Tests idle_verify working on an active cluster with persistence. Works via control.sh.
+     *
+     * @throws Exception if failed.
+     */
+    @Test
+    public void testIdleVerifyOnActiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(1);
+
+        ignite.cluster().state(ACTIVE);
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+
+        ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                .setDataRegionName("persistent-dataRegion"));
+
+        injectTestSystemOut();
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        assertTrue(ignite.cluster().active());
+        assertEquals(ACTIVE, ignite.cluster().state());
+        assertContains(log, testOut.toString(), "The check procedure has finished, no conflicts have been found.");
+    }
+
+    /**
+     * Tests idle_verify working on an inactive cluster with persistence. Works via control.sh.
+     *
+     * @throws IgniteException if succeeded.
+     */
+    @Test
+    public void testIdleVerifyOnInactiveClusterWithPersistence() throws Exception {
+        dataRegionConfiguration = new DataRegionConfiguration()
+                .setName("persistent-dataRegion")
+                .setPersistenceEnabled(true);
+
+        Ignite ignite = startGrids(2);
+
+        ignite.cluster().state(INACTIVE);
+
+        assertFalse(ignite.cluster().active());
+        assertEquals(INACTIVE, ignite.cluster().state());
+
+        injectTestSystemOut();
+
+        GridTestUtils.assertThrows(
+                log,
+                () -> ignite.createCache(new CacheConfiguration<>("persistent-cache")
+                        .setDataRegionName("persistent-dataRegion")),
+                IgniteException.class,
+                "Can not perform the operation because the cluster is inactive. " +
+                        "Note, that the cluster is considered inactive by default if Ignite Persistent Store is used to " +
+                        "let all the nodes join the cluster. To activate the cluster call Ignite.active(true).");
+
+        execute("--cache", "idle_verify");
+
+        assertEquals(EXIT_CODE_OK, execute("--cache", "idle_verify"));
+
+        String out = testOut.toString();
+
+        assertContains(log, out, "idle_verify does not work on an inactive cluster with persistence");
+        assertFalse(ignite.cluster().active());

Review Comment:
   IMHO, this check is excessive.



-- 
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