anton-vinogradov commented on code in PR #10552:
URL: https://github.com/apache/ignite/pull/10552#discussion_r1111899657


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -1400,8 +1402,12 @@ private void processLocalSnapshotEndStageResult(UUID id, 
Map<UUID, SnapshotOpera
 
                         clusterSnpFut.onDone(wrn);
                     }
-                    else
+                    else {
+                        if (snpReq.incremental())
+                            
warnAtomicCachesInIncrementalSnapshot(snpReq.snapshotName(), 
snpReq.incrementIndex(), snpReq.groups());

Review Comment:
   Please avoid logging inside the synchronized section



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -2120,6 +2126,34 @@ public IgniteFutureImpl<Void> createSnapshot(String 
name, @Nullable String snpPa
         }
     }
 
+    /** Writes a warning message if an incremental snapshot contains atomic 
caches. */
+    void warnAtomicCachesInIncrementalSnapshot(String snpName, int incIdx, 
Collection<String> cacheGrps) {
+        List<String> warnCaches = new ArrayList<>();
+
+        for (String cacheGrp: cacheGrps) {
+            CacheGroupContext cgctx = 
cctx.cache().cacheGroup(CU.cacheId(cacheGrp));
+
+            if (cgctx != null && cgctx.hasAtomicCaches()) {
+                for (GridCacheContext<?, ?> c : cgctx.caches()) {
+                    CacheConfiguration<?, ?> ccfg = c.config();
+
+                    if (ccfg.getAtomicityMode() == CacheAtomicityMode.ATOMIC 
&& ccfg.getBackups() > 0)
+                        warnCaches.add(ccfg.getName());
+                }
+            }
+        }
+
+        if (warnCaches.isEmpty())
+            return;
+
+        U.warn(log, "Incremental snapshot [snpName=" + snpName + ", incIdx=" + 
incIdx + "] contains ATOMIC caches with backups: "
+            + warnCaches + ". Please note, incremental snapshots doesn't 
guarantee consistency of restored atomic caches. " +
+            "It is highly recommended to verify those caches after restoring 
with the command: " +
+            "\"control.sh --cache idle_verify $cache\". If it is needed it's 
possible to repair inconsistent partitions " +
+            "with the command: \"control.sh --consistency repair --cache 
$cache --partition $partition\". " +

Review Comment:
   Command params may be changed, not refactoring friendly.



##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/incremental/IncrementalSnapshotWarnAtomicCachesTest.java:
##########
@@ -0,0 +1,235 @@
+/*
+ * 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.processors.cache.persistence.snapshot.incremental;
+
+import java.util.Collection;
+import java.util.regex.Pattern;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.affinity.rendezvous.RendezvousAffinityFunction;
+import org.apache.ignite.cluster.ClusterState;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+/** */
+public class IncrementalSnapshotWarnAtomicCachesTest extends 
GridCommonAbstractTest {
+    /** */
+    private static final String SNP = "snapshot";
+
+    /** */
+    private static final ListeningTestLogger lsnLogger = new 
ListeningTestLogger();
+
+    /** */
+    private CacheConfiguration<Integer, Integer>[] ccfgs;
+
+    /** */
+    @Override protected IgniteConfiguration getConfiguration(String 
instanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(instanceName);
+
+        cfg.setDataStorageConfiguration(new DataStorageConfiguration()
+            .setWalCompactionEnabled(true)
+            .setDefaultDataRegionConfiguration(new DataRegionConfiguration()
+                .setName("persistence")
+                .setPersistenceEnabled(true)));
+
+        cfg.setCacheConfiguration(ccfgs);
+
+        cfg.setGridLogger(lsnLogger);
+
+        
cfg.setConsistentId(String.valueOf(getTestIgniteInstanceIndex(instanceName)));
+
+        return cfg;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTest() throws Exception {
+        cleanPersistenceDir();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids();
+
+        cleanPersistenceDir();
+    }
+
+    /** */
+    @Test
+    public void testAtomicCacheWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot("cache",
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 1, "cache", "grp"));
+
+        checkWarnMessageOnRestoreSnapshot("cache", null);
+    }
+
+    /** */
+    @Test
+    public void testAtomicCacheNoBackupsNoWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot(null,
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 0, "cache", "grp"));
+
+        checkWarnMessageOnRestoreSnapshot(null, null);
+    }
+
+    /** */
+    @Test
+    public void testMultipleCachesSingleAtomicWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot("cache0",
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 1, "cache0", "grp0"),
+            cacheConfiguration(CacheAtomicityMode.TRANSACTIONAL, 1, "cache1", 
"grp1"));
+
+        checkWarnMessageOnRestoreSnapshot("cache0", null);
+        checkWarnMessageOnRestoreSnapshot("cache0", F.asList("grp0"));
+        checkWarnMessageOnRestoreSnapshot(null, F.asList("grp1"));
+    }
+
+    /** */
+    @Test
+    public void testCacheGroupWithAtomicWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot("cache0",
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 1, "cache0", "grp"),
+            cacheConfiguration(CacheAtomicityMode.TRANSACTIONAL, 1, "cache1", 
"grp"));
+
+        checkWarnMessageOnRestoreSnapshot("cache0", null);
+    }
+
+    /** */
+    @Test
+    public void testMultipleAtomicCachesWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot("cache0, cache2",
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 1, "cache0", "grp"),
+            cacheConfiguration(CacheAtomicityMode.TRANSACTIONAL, 1, "cache1", 
"grp"),
+            cacheConfiguration(CacheAtomicityMode.ATOMIC, 1, "cache2", 
"grp1"));
+
+        checkWarnMessageOnRestoreSnapshot("cache[0,2], cache[0,2]", null);
+        checkWarnMessageOnRestoreSnapshot("cache0", F.asList("grp"));
+        checkWarnMessageOnRestoreSnapshot("cache2", F.asList("grp1"));
+    }
+
+    /** */
+    @Test
+    public void testTxCacheNoWarn() throws Exception {
+        checkWarnMessageOnCreateSnapshot(null,
+            cacheConfiguration(CacheAtomicityMode.TRANSACTIONAL, 1, "cache", 
"grp"));
+
+        checkWarnMessageOnRestoreSnapshot(null, null);
+    }
+
+    /** */
+    private void checkWarnMessageOnCreateSnapshot(
+        String warnAtomicCaches,
+        CacheConfiguration<Integer, Integer>... ccfgs
+    ) throws Exception {
+        this.ccfgs = ccfgs;
+
+        Ignite g = startGrids(3);
+
+        g.cluster().state(ClusterState.ACTIVE);
+
+        for (CacheConfiguration<Integer, Integer> c: ccfgs) {
+            for (int i = 0; i < 1_000; i++)
+                g.cache(c.getName()).put(i, i);
+        }
+
+        LogListener lsnr = warnLogListener(warnAtomicCaches, 0);  // Should 
warn only for incremental caches.

Review Comment:
    incremental?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to