IGNITE-29052 Fix flaky IgniteCacheGroupsTest.testRestartsAndCacheCreateDestroy

modules/core/src/main/java/org/apache/ignite/internal/processors/metric/GridMetricManager.java

@@ -428,16 +428,22 @@ else if (m instanceof HistogramMetric)
             return null;
         });
 
-        try {
-            opsFut.markInitialized();
-            opsFut.get();
-        }
-        catch (NodeStoppingException ignored) {
-            // No-op.
-        }
-        catch (IgniteCheckedException e) {
-            log.error("Failed to remove metrics configuration.", e);
-        }
+        opsFut.markInitialized();
+
+        // Do not wait for the removal here: this method is invoked from the partition map exchange (cache stop), and
+        // the removal is a discovery custom message round trip. On a client node such message can be lost during
+        // reconnect to another router, so waiting for it would block the exchange on the client forever.
+        opsFut.listen(() -> {
+            try {
+                opsFut.get();
+            }
+            catch (NodeStoppingException ignored) {
+                // No-op.
+            }
+            catch (IgniteCheckedException e) {
+                log.error("Failed to remove metrics configuration [regName=" + regName + ']', e);
+            }
+        });
     }
 
     /**

modules/core/src/test/java/org/apache/ignite/internal/metric/MetricsConfigurationTest.java

@@ -17,8 +17,11 @@
 
 package org.apache.ignite.internal.metric;
 
+import java.util.Arrays;
 import java.util.Collections;
 import javax.management.DynamicMBean;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
 import org.apache.ignite.cluster.ClusterState;
 import org.apache.ignite.configuration.CacheConfiguration;
 import org.apache.ignite.configuration.DataStorageConfiguration;
@@ -48,6 +51,7 @@
 import static org.apache.ignite.internal.processors.pool.PoolProcessor.TASK_EXEC_TIME;
 import static org.apache.ignite.internal.processors.pool.PoolProcessor.THREAD_POOLS;
 import static org.apache.ignite.testframework.GridTestUtils.assertThrowsWithCause;
+import static org.apache.ignite.testframework.GridTestUtils.waitForCondition;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertNotEquals;
 
@@ -337,22 +341,39 @@ public void testConfigRemovedOnRegistryRemove() throws Exception {
 
             g0.context().metric().remove(TEST_REG);
 
-            assertNull(
-                g0.context().distributedMetastorage().read(metricName(HITRATE_CFG_PREFIX, TEST_REG, HITRATE_NAME)));
-            assertNull(
-                g0.context().distributedMetastorage().read(metricName(MAXVAL_CFG_PREFIX, TEST_REG, MAXVAL_NAME)));
-            assertNull(
-                g0.context().distributedMetastorage().read(metricName(HISTOGRAM_CFG_PREFIX, TEST_REG, HISTOGRAM_NAME)));
-
-            assertNull(
-                g1.context().distributedMetastorage().read(metricName(HITRATE_CFG_PREFIX, TEST_REG, HITRATE_NAME)));
-            assertNull(
-                g1.context().distributedMetastorage().read(metricName(MAXVAL_CFG_PREFIX, TEST_REG, MAXVAL_NAME)));
-            assertNull(
-                g1.context().distributedMetastorage().read(metricName(HISTOGRAM_CFG_PREFIX, TEST_REG, HISTOGRAM_NAME)));
+            assertConfigRemoved(g0, g1,
+                metricName(HITRATE_CFG_PREFIX, TEST_REG, HITRATE_NAME),
+                metricName(MAXVAL_CFG_PREFIX, TEST_REG, MAXVAL_NAME),
+                metricName(HISTOGRAM_CFG_PREFIX, TEST_REG, HISTOGRAM_NAME));
         });
     }
 
+    /**
+     * Metric configuration is removed from the distributed metastorage asynchronously, so wait for the removal.
+     *
+     * @param g0 First node.
+     * @param g1 Second node.
+     * @param keys Distributed metastorage keys that must be removed.
+     * @throws IgniteCheckedException If failed.
+     */
+    private void assertConfigRemoved(IgniteEx g0, IgniteEx g1, String... keys) throws IgniteCheckedException {
+        assertTrue(waitForCondition(() -> {
+            try {
+                for (IgniteEx node : Arrays.asList(g0, g1)) {
+                    for (String key : keys) {
+                        if (node.context().distributedMetastorage().read(key) != null)
+                            return false;
+                    }
+                }
+
+                return true;
+            }
+            catch (IgniteCheckedException e) {
+                throw new IgniteException(e);
+            }
+        }, getTestTimeout()));
+    }
+
     /** Tests metric configuration removed on registry remove. */
     @Test
     public void testConfigRemovedOnCacheRemove() throws Exception {

modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheGroupsTest.java

@@ -3710,9 +3710,11 @@ public void testDataCleanup() throws Exception {
     public void testRestartsAndCacheCreateDestroy() throws Exception {
         final int SRVS = 5;
 
-        startGrids(SRVS);
+        startGrid(0);
+
+        final Ignite clientNode = startClientGrid(1);
 
-        final Ignite clientNode = startClientGrid(SRVS);
+        startGridsMultiThreaded(2, SRVS - 1);
 
         final int CACHES = SF.applyLB(10, 2);
 
@@ -3744,7 +3746,8 @@ public void testRestartsAndCacheCreateDestroy() throws Exception {
                             ThreadLocalRandom rnd = ThreadLocalRandom.current();
 
                             while (!stop.get()) {
-                                int node = rnd.nextInt(SRVS);
+                                // 0 - server node, 1 - client node.
+                                int node = rnd.nextInt(2, SRVS + 1);
 
                                 log.info("Stop node: " + node);
 
@@ -3879,17 +3882,22 @@ public void testRestartsAndCacheCreateDestroy() throws Exception {
                     assertTrue(cacheIds.add(CU.cacheId(cache.getName())));
                 }
 
-                for (int n = 0; n < SRVS; n++) {
-                    CacheGroupContext grp = cacheGroup(ignite(n), GROUP1);
+                for (int n = 0; n <= SRVS; n++) {
+                    for (String grpName : Arrays.asList(GROUP1, GROUP2)) {
+                        CacheGroupContext grp = cacheGroup(ignite(n), grpName);
 
-                    assertNotNull(grp);
+                        // Group may be absent on a node if all its caches were
+                        // re-created under another group during this iteration.
+                        if (grp == null)
+                            continue;
 
-                    for (GridDhtLocalPartition part : grp.topology().currentLocalPartitions()) {
-                        IntMap<Object> cachesMap = GridTestUtils.getFieldValue(part, "cacheMaps");
+                        for (GridDhtLocalPartition part : grp.topology().currentLocalPartitions()) {
+                            IntMap<Object> cachesMap = GridTestUtils.getFieldValue(part, "cacheMaps");
 
-                        assertTrue(cachesMap.size() <= cacheIds.size());
+                            assertTrue(cachesMap.size() <= cacheIds.size());
 
-                        cachesMap.forEach((cacheId, v) -> assertTrue(cachesMap.containsKey(cacheId)));
+                            cachesMap.forEach((cacheId, v) -> assertTrue(cachesMap.containsKey(cacheId)));
+                        }
                     }
                 }
             }