You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by zs...@apache.org on 2022/12/19 10:44:21 UTC

[ignite] branch master updated: IGNITE-18377 Sql statistics become broken after node restart with enabled persistence - Fixes #10441.

This is an automated email from the ASF dual-hosted git repository.

zstan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git


The following commit(s) were added to refs/heads/master by this push:
     new 97639944bc1 IGNITE-18377 Sql statistics become broken after node restart with enabled persistence - Fixes #10441.
97639944bc1 is described below

commit 97639944bc19bba598e9ae2ea4651525f24b59a6
Author: zstan <st...@gmail.com>
AuthorDate: Mon Dec 19 13:35:48 2022 +0300

    IGNITE-18377 Sql statistics become broken after node restart with enabled persistence - Fixes #10441.
    
    Signed-off-by: zstan <st...@gmail.com>
---
 .../processors/cache/GridCacheAdapter.java         |  15 +--
 .../cache/IgniteCacheOffheapManager.java           |   4 +-
 .../cache/IgniteCacheOffheapManagerImpl.java       |   2 +-
 .../IgniteServiceConfigVariationsFullApiTest.java  |   9 +-
 .../junits/common/GridCommonAbstractTest.java      |   8 +-
 .../processors/query/h2/opt/GridH2Table.java       |  20 +++-
 .../processors/query/h2/opt/TableStatistics.java   |  18 ++--
 .../query/SqlQueriesTopologyMappingTest.java       | 101 ++++++++++++++++++++-
 8 files changed, 135 insertions(+), 42 deletions(-)

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java
index 0658d2cfccd..36378b008f8 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java
@@ -4162,17 +4162,12 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V
 
     /** {@inheritDoc} */
     @Override public long offHeapEntriesCount() {
-        try {
-            IgniteCacheOffheapManager mgr = ctx.offheap();
+        IgniteCacheOffheapManager mgr = ctx.offheap();
 
-            return mgr != null ? mgr.cacheEntriesCount(ctx.cacheId(),
-                true,
-                true,
-                ctx.affinity().affinityTopologyVersion()) : -1;
-        }
-        catch (IgniteCheckedException ignore) {
-            return 0;
-        }
+        return mgr != null ? mgr.cacheEntriesCount(ctx.cacheId(),
+            true,
+            true,
+            ctx.affinity().affinityTopologyVersion()) : -1;
     }
 
     /** {@inheritDoc} */
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManager.java
index db8823128e8..3c4c732d092 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManager.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManager.java
@@ -490,11 +490,9 @@ public interface IgniteCacheOffheapManager {
      * @param backup Backup entries flag.
      * @param topVer Topology version.
      * @return Entries count.
-     * @throws IgniteCheckedException If failed.
      */
     // TODO: MVCC>
-    public long cacheEntriesCount(int cacheId, boolean primary, boolean backup, AffinityTopologyVersion topVer)
-        throws IgniteCheckedException;
+    public long cacheEntriesCount(int cacheId, boolean primary, boolean backup, AffinityTopologyVersion topVer);
 
     /**
      * Store entries.
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java
index a26e9c6df14..b57a6aeb9b5 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java
@@ -358,7 +358,7 @@ public class IgniteCacheOffheapManagerImpl implements IgniteCacheOffheapManager
         boolean primary,
         boolean backup,
         AffinityTopologyVersion topVer
-    ) throws IgniteCheckedException {
+    ) {
         long cnt = 0;
 
         Iterator<CacheDataStore> it = cacheData(primary, backup, topVer);
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/service/IgniteServiceConfigVariationsFullApiTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/service/IgniteServiceConfigVariationsFullApiTest.java
index 3b6ae6fadee..b6760b42f05 100644
--- a/modules/core/src/test/java/org/apache/ignite/internal/processors/service/IgniteServiceConfigVariationsFullApiTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/service/IgniteServiceConfigVariationsFullApiTest.java
@@ -24,8 +24,6 @@ import java.io.ObjectOutput;
 import java.util.concurrent.ThreadLocalRandom;
 import javax.cache.configuration.Factory;
 import org.apache.ignite.IgniteCache;
-import org.apache.ignite.IgniteCheckedException;
-import org.apache.ignite.IgniteException;
 import org.apache.ignite.IgniteServices;
 import org.apache.ignite.internal.IgniteEx;
 import org.apache.ignite.internal.IgniteInterruptedCheckedException;
@@ -129,12 +127,7 @@ public class IgniteServiceConfigVariationsFullApiTest extends IgniteConfigVariat
             @Override public void run(IgniteServices services, String svcName, TestService svc) {
                 IgniteCache<Object, Object> cache = grid(testedNodeIdx).getOrCreateCache(CACHE_NAME);
 
-                try {
-                    services.deployKeyAffinitySingleton(svcName, (Service)svc, cache.getName(), primaryKey(cache));
-                }
-                catch (IgniteCheckedException e) {
-                    throw new IgniteException(e);
-                }
+                services.deployKeyAffinitySingleton(svcName, (Service)svc, cache.getName(), primaryKey(cache));
             }
         }));
     }
diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java
index 067b40438eb..c584b0e1b1b 100755
--- a/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java
@@ -1525,20 +1525,16 @@ public abstract class GridCommonAbstractTest extends GridAbstractTest {
     /**
      * @param cache Cache.
      * @return Collection of keys for which given cache is primary.
-     * @throws IgniteCheckedException If failed.
      */
-    protected Integer primaryKey(IgniteCache<?, ?> cache)
-        throws IgniteCheckedException {
+    protected Integer primaryKey(IgniteCache<?, ?> cache) {
         return primaryKeys(cache, 1, 1).get(0);
     }
 
     /**
      * @param cache Cache.
      * @return Keys for which given cache is backup.
-     * @throws IgniteCheckedException If failed.
      */
-    protected Integer backupKey(IgniteCache<?, ?> cache)
-        throws IgniteCheckedException {
+    protected Integer backupKey(IgniteCache<?, ?> cache) {
         return backupKeys(cache, 1, 1).get(0);
     }
 
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java
index 70bf2e7d843..f3564c5be8b 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java
@@ -35,8 +35,10 @@ import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.IgniteException;
 import org.apache.ignite.IgniteInterruptedException;
 import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cache.CacheMode;
 import org.apache.ignite.cache.CachePeekMode;
 import org.apache.ignite.cache.query.QueryRetryException;
+import org.apache.ignite.configuration.CacheConfiguration;
 import org.apache.ignite.internal.GridKernalContext;
 import org.apache.ignite.internal.processors.cache.GridCacheContext;
 import org.apache.ignite.internal.processors.cache.GridCacheContextInfo;
@@ -73,6 +75,7 @@ import org.h2.value.DataType;
 import org.jetbrains.annotations.Nullable;
 
 import static org.apache.ignite.cache.CacheMode.PARTITIONED;
+import static org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion.NONE;
 
 /**
  * H2 Table implementation.
@@ -1009,7 +1012,7 @@ public class GridH2Table extends TableBase {
 
         refreshStatsIfNeeded();
 
-        return tblStats.primaryRowCount();
+        return tblStats.localRowCount();
     }
 
     /**
@@ -1034,13 +1037,22 @@ public class GridH2Table extends TableBase {
 
         // Update stats if total table size changed significantly since the last stats update.
         if (needRefreshStats(statsTotalRowCnt, curTotalRowCnt) && cacheInfo.affinityNode()) {
-            long primaryRowCnt = cacheSize(CachePeekMode.PRIMARY);
-            long totalRowCnt = cacheSize(CachePeekMode.PRIMARY, CachePeekMode.BACKUP);
+            CacheConfiguration ccfg = cacheContext().config();
+
+            int backups = ccfg.getCacheMode() == CacheMode.REPLICATED ? 0 : cacheContext().config().getBackups();
+
+            // After restart of node with persistence and before affinity exchange - PRIMARY partitions are empty.
+            // Try to predict local row count take into account ideal distribution.
+            long localOwnerRowCnt = cacheSize(CachePeekMode.PRIMARY, CachePeekMode.BACKUP) / (backups + 1);
+
+            int owners = cacheContext().discovery().cacheNodes(cacheContext().name(), NONE).size();
+
+            long totalRowCnt = owners * localOwnerRowCnt;
 
             size.reset();
             size.add(totalRowCnt);
 
-            tblStats = new TableStatistics(totalRowCnt, primaryRowCnt);
+            tblStats = new TableStatistics(totalRowCnt, localOwnerRowCnt);
         }
     }
 
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/TableStatistics.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/TableStatistics.java
index 0d8422af553..af446779493 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/TableStatistics.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/TableStatistics.java
@@ -23,18 +23,18 @@ public class TableStatistics {
     /** Total table row count (including primary and backup partitions). */
     private final long totalRowCnt;
 
-    /** Primary parts row count. */
-    private final long primaryRowCnt;
+    /** Local node row count. */
+    private final long localRowCount;
 
     /**
      * @param totalRowCnt Total table row count (including primary and backup partitions).
-     * @param primaryRowCnt Primary parts row count.
+     * @param localRowCount Local node row count.
      */
-    public TableStatistics(long totalRowCnt, long primaryRowCnt) {
-        assert totalRowCnt >= 0 && primaryRowCnt >= 0 : "totalRowCnt=" + totalRowCnt + ", primaryRowCnt=" + primaryRowCnt;
+    public TableStatistics(long totalRowCnt, long localRowCount) {
+        assert totalRowCnt >= 0 && localRowCount >= 0 : "totalRowCnt=" + totalRowCnt + ", localRowCount=" + localRowCount;
 
         this.totalRowCnt = totalRowCnt;
-        this.primaryRowCnt = primaryRowCnt;
+        this.localRowCount = localRowCount;
     }
 
     /**
@@ -45,9 +45,9 @@ public class TableStatistics {
     }
 
     /**
-     * @return Primary parts row count.
+     * @return Local node row count.
      */
-    public long primaryRowCount() {
-        return primaryRowCnt;
+    public long localRowCount() {
+        return localRowCount;
     }
 }
diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueriesTopologyMappingTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueriesTopologyMappingTest.java
index 23756e8941b..a6bfceec0f8 100644
--- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueriesTopologyMappingTest.java
+++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/SqlQueriesTopologyMappingTest.java
@@ -22,34 +22,66 @@ import java.util.List;
 import org.apache.ignite.Ignite;
 import org.apache.ignite.IgniteCache;
 import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.affinity.rendezvous.RendezvousAffinityFunction;
 import org.apache.ignite.cache.query.SqlFieldsQuery;
 import org.apache.ignite.cluster.ClusterNode;
+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.IgniteEx;
 import org.apache.ignite.internal.TestRecordingCommunicationSpi;
 import org.apache.ignite.internal.processors.cache.distributed.dht.preloader.GridDhtPartitionSupplyMessage;
 import org.apache.ignite.internal.processors.cache.index.AbstractIndexingCommonTest;
+import org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing;
+import org.apache.ignite.internal.processors.query.h2.opt.GridH2Table;
+import org.apache.ignite.internal.processors.query.h2.opt.TableStatistics;
 import org.apache.ignite.internal.util.typedef.G;
 import org.apache.ignite.lang.IgniteBiPredicate;
 import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.testframework.GridTestUtils;
 import org.junit.Test;
 
 import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_IGNITE_INSTANCE_NAME;
 
 /** */
 public class SqlQueriesTopologyMappingTest extends AbstractIndexingCommonTest {
+    /** If {@code true} persistence will be enabled. */
+    private static boolean persistence;
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTest() throws Exception {
+        super.beforeTest();
+
+        persistence = false;
+
+        cleanPersistenceDir();
+    }
+
     /** {@inheritDoc} */
     @Override protected void afterTest() throws Exception {
         stopAllGrids();
 
         super.afterTest();
+
+        cleanPersistenceDir();
     }
 
     /** {@inheritDoc} */
     @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
-        return super.getConfiguration(igniteInstanceName)
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName)
             .setCommunicationSpi(new TestRecordingCommunicationSpi());
+
+        if (persistence) {
+            DataRegionConfiguration dataReg = new DataRegionConfiguration();
+            dataReg.setMaxSize(256 * 1024 * 1024);
+            dataReg.setPersistenceEnabled(true);
+
+            cfg.setDataStorageConfiguration(new DataStorageConfiguration().setDefaultDataRegionConfiguration(dataReg));
+        }
+
+        return cfg;
     }
 
     /** */
@@ -64,6 +96,18 @@ public class SqlQueriesTopologyMappingTest extends AbstractIndexingCommonTest {
         checkQueryWithRebalance(CacheMode.REPLICATED);
     }
 
+    /** */
+    @Test
+    public void testPartitionedQueryStatUpdateWithRebalance() throws Exception {
+        checkSqlStatWithRebalance(CacheMode.PARTITIONED);
+    }
+
+    /** */
+    @Test
+    public void testReplicatedQueryStatUpdateWithRebalance() throws Exception {
+        checkSqlStatWithRebalance(CacheMode.REPLICATED);
+    }
+
     /** */
     @Test
     public void testPartitionedQueryWithNodeFilter() throws Exception {
@@ -89,6 +133,7 @@ public class SqlQueriesTopologyMappingTest extends AbstractIndexingCommonTest {
         blockRebalanceSupplyMessages(ign0, DEFAULT_CACHE_NAME, getTestIgniteInstanceName(1));
 
         startGrid(1);
+
         startClientGrid(10);
 
         for (Ignite ign : G.allGrids()) {
@@ -113,6 +158,7 @@ public class SqlQueriesTopologyMappingTest extends AbstractIndexingCommonTest {
         cache.put(1, 2);
 
         startGrid(1);
+
         startClientGrid(10);
 
         for (Ignite ign : G.allGrids()) {
@@ -124,6 +170,59 @@ public class SqlQueriesTopologyMappingTest extends AbstractIndexingCommonTest {
         }
     }
 
+    /** Checks correctness of sql statistics updates after node restart and active reballance. */
+    private void checkSqlStatWithRebalance(CacheMode cacheMode) throws Exception {
+        persistence = true;
+
+        int partitions = 1024;
+
+        IgniteEx ign0 = (IgniteEx)startGridsMultiThreaded(2);
+
+        ign0.cluster().state(ClusterState.ACTIVE);
+
+        IgniteCache<Object, Object> cache =
+            ign0.createCache(new CacheConfiguration<>(DEFAULT_CACHE_NAME)
+                .setCacheMode(cacheMode)
+                .setAffinity(new RendezvousAffinityFunction(false, partitions))
+                .setBackups(1)
+                .setIndexedTypes(Integer.class, Integer.class));
+
+        for (int i = 0; i < partitions; ++i)
+            cache.put(i, i);
+
+        IgniteEx cli = startClientGrid(10);
+
+        IgniteH2Indexing idx = (IgniteH2Indexing)ign0.context().query().getIndexing();
+
+        GridH2Table tbl = idx.schemaManager().dataTable(DEFAULT_CACHE_NAME, "INTEGER");
+
+        double statUpdTreshold = GridTestUtils.getFieldValue(tbl, "STATS_UPDATE_THRESHOLD");
+
+        List<Integer> newKeys = backupKeys(cache, (int)(statUpdTreshold * partitions / 2 + 10), 2 * partitions);
+
+        stopGrid(1);
+
+        blockRebalanceSupplyMessages(ign0, DEFAULT_CACHE_NAME, getTestIgniteInstanceName(1));
+
+        for (int key : newKeys)
+            cache.put(key, key);
+
+        IgniteEx ign1 = startGrid(1);
+
+        // touch statistics
+        List<List<?>> res = cli.cache(DEFAULT_CACHE_NAME).query(new SqlFieldsQuery("SELECT COUNT(*) FROM Integer")).getAll();
+
+        assertEquals((long)partitions + newKeys.size(), res.get(0).get(0));
+
+        idx = (IgniteH2Indexing)ign1.context().query().getIndexing();
+
+        tbl = idx.schemaManager().dataTable(DEFAULT_CACHE_NAME, "INTEGER");
+
+        TableStatistics stat = GridTestUtils.getFieldValue(tbl, "tblStats");
+
+        assertFalse(stat.localRowCount() == 0);
+    }
+
     /** */
     private void blockRebalanceSupplyMessages(IgniteEx sndNode, String cacheName, String dstNodeName) {
         int grpId = sndNode.cachex(cacheName).context().groupId();