IGNITE-10033 Update Grid(Ignite)CacheDatabaseSharedManager according to 
accepted coding guidelines - Fixes #5156.

Signed-off-by: Dmitriy Govorukhin <dmitriy.govoruk...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/bdb22399
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/bdb22399
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/bdb22399

Branch: refs/heads/ignite-627
Commit: bdb22399699a8c0288730ca9a38f177e73878322
Parents: d6154e1
Author: Maxim Muzafarov <maxmu...@gmail.com>
Authored: Tue Oct 30 00:12:36 2018 +0300
Committer: Dmitriy Govorukhin <dmitriy.govoruk...@gmail.com>
Committed: Tue Oct 30 00:12:36 2018 +0300

----------------------------------------------------------------------
 .../GridCacheDatabaseSharedManager.java         | 135 ++++++-----------
 .../IgniteCacheDatabaseSharedManager.java       | 150 +++++++++++--------
 .../IgniteNoParrallelClusterIsAllowedTest.java  |   2 +-
 3 files changed, 134 insertions(+), 153 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/bdb22399/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
----------------------------------------------------------------------
diff --git 
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
 
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
index 4af1d8e..e09ad22 100755
--- 
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
+++ 
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
@@ -57,7 +57,6 @@ import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import javax.management.ObjectName;
 import org.apache.ignite.DataStorageMetrics;
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.IgniteException;
@@ -324,9 +323,6 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
     /** */
     private DataStorageMetricsImpl persStoreMetrics;
 
-    /** */
-    private ObjectName persistenceMetricsMbeanName;
-
     /** Counter for written checkpoint pages. Not null only if checkpoint is 
running. */
     private volatile AtomicInteger writtenPagesCntr = null;
 
@@ -438,7 +434,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
 
         addDataRegion(
             memCfg,
-            createDataRegionConfiguration(memCfg),
+            createMetastoreDataRegionConfig(memCfg),
             false
         );
 
@@ -451,7 +447,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
      * @param storageCfg Data storage configuration.
      * @return Data region configuration.
      */
-    private DataRegionConfiguration 
createDataRegionConfiguration(DataStorageConfiguration storageCfg) {
+    private DataRegionConfiguration 
createMetastoreDataRegionConfig(DataStorageConfiguration storageCfg) {
         DataRegionConfiguration cfg = new DataRegionConfiguration();
 
         cfg.setName(METASTORE_DATA_REGION_NAME);
@@ -667,7 +663,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
         try {
             DataStorageConfiguration memCfg = 
cctx.kernalContext().config().getDataStorageConfiguration();
 
-            DataRegionConfiguration plcCfg = 
createDataRegionConfiguration(memCfg);
+            DataRegionConfiguration plcCfg = 
createMetastoreDataRegionConfig(memCfg);
 
             File allocPath = buildAllocPath(plcCfg);
 
@@ -733,10 +729,15 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
 
         snapshotMgr = cctx.snapshot();
 
-        if (!cctx.kernalContext().clientNode()) {
-            initDataBase();
-
-            registrateMetricsMBean();
+        if (!cctx.kernalContext().clientNode() && 
persistenceCfg.getCheckpointThreads() > 1) {
+            asyncRunner = new IgniteThreadPoolExecutor(
+                CHECKPOINT_RUNNER_THREAD_PREFIX,
+                cctx.igniteInstanceName(),
+                persistenceCfg.getCheckpointThreads(),
+                persistenceCfg.getCheckpointThreads(),
+                30_000,
+                new LinkedBlockingQueue<>()
+            );
         }
 
         if (checkpointer == null)
@@ -759,61 +760,17 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
         stopping = false;
     }
 
-    /**
-     *
-     */
-    private void initDataBase() {
-        if (persistenceCfg.getCheckpointThreads() > 1)
-            asyncRunner = new IgniteThreadPoolExecutor(
-                CHECKPOINT_RUNNER_THREAD_PREFIX,
-                cctx.igniteInstanceName(),
-                persistenceCfg.getCheckpointThreads(),
-                persistenceCfg.getCheckpointThreads(),
-                30_000,
-                new LinkedBlockingQueue<Runnable>()
-            );
-    }
-
-    /**
-     * Try to register Metrics MBean.
-     *
-     * @throws IgniteCheckedException If failed.
-     */
-    private void registrateMetricsMBean() throws IgniteCheckedException {
-        if (U.IGNITE_MBEANS_DISABLED)
-            return;
-
-        try {
-            persistenceMetricsMbeanName = U.registerMBean(
-                cctx.kernalContext().config().getMBeanServer(),
-                cctx.kernalContext().igniteInstanceName(),
-                MBEAN_GROUP,
-                MBEAN_NAME,
-                persStoreMetrics,
-                DataStorageMetricsMXBean.class);
-        }
-        catch (Throwable e) {
-            throw new IgniteCheckedException("Failed to register " + 
MBEAN_NAME + " MBean.", e);
-        }
-    }
-
-    /**
-     * Unregister metrics MBean.
-     */
-    private void unRegistrateMetricsMBean() {
-        if (persistenceMetricsMbeanName == null)
-            return;
-
-        assert !U.IGNITE_MBEANS_DISABLED;
-
-        try {
-            
cctx.kernalContext().config().getMBeanServer().unregisterMBean(persistenceMetricsMbeanName);
-
-            persistenceMetricsMbeanName = null;
-        }
-        catch (Throwable e) {
-            U.error(log, "Failed to unregister " + MBEAN_NAME + " MBean.", e);
-        }
+    /** {@inheritDoc} */
+    @Override protected void registerMetricsMBeans(IgniteConfiguration cfg) {
+        super.registerMetricsMBeans(cfg);
+
+        registerMetricsMBean(
+            cctx.kernalContext().config(),
+            MBEAN_GROUP,
+            MBEAN_NAME,
+            persStoreMetrics,
+            DataStorageMetricsMXBean.class
+        );
     }
 
     /** {@inheritDoc} */
@@ -926,8 +883,8 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                 cacheGrps);
 
             if (tailWalPtr == null && 
!status.endPtr.equals(CheckpointStatus.NULL_PTR)) {
-                throw new StorageException("Restore wal pointer = " + 
tailWalPtr + ", while status.endPtr = " +
-                    status.endPtr + ". Can't restore memory - critical part of 
WAL archive is missing.");
+                throw new StorageException("The memory cannot be restored. The 
critical part of WAL archive is missing " +
+                    "[tailWalPtr=" + tailWalPtr + ", endPtr=" + status.endPtr 
+ ']');
             }
 
             nodeStart(tailWalPtr);
@@ -1053,7 +1010,11 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
 
         super.onKernalStop0(cancel);
 
-        unRegistrateMetricsMBean();
+        unregisterMetricsMBean(
+            cctx.gridConfig(),
+            MBEAN_GROUP,
+            MBEAN_NAME
+        );
     }
 
     /** {@inheritDoc} */
@@ -1228,8 +1189,9 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                 plc = 
PageMemoryImpl.ThrottlingPolicy.valueOf(throttlingPolicyOverride.toUpperCase());
             }
             catch (IllegalArgumentException e) {
-                log.error("Incorrect value of 
IGNITE_OVERRIDE_WRITE_THROTTLING_ENABLED property: " +
-                    throttlingPolicyOverride + ". Default throttling policy " 
+ plc + " will be used.");
+                log.error("Incorrect value of 
IGNITE_OVERRIDE_WRITE_THROTTLING_ENABLED property. " +
+                    "The default throttling policy will be used [plc=" + 
throttlingPolicyOverride +
+                    ", defaultPlc=" + plc + ']');
             }
         }
         return plc;
@@ -1241,8 +1203,8 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
         if (!regCfg.isPersistenceEnabled())
             super.checkRegionEvictionProperties(regCfg, dbCfg);
         else if (regCfg.getPageEvictionMode() != 
DataPageEvictionMode.DISABLED) {
-            U.warn(log, "Page eviction mode set for [" + regCfg.getName() + "] 
data will have no effect" +
-                " because the oldest pages are evicted automatically if Ignite 
persistence is enabled.");
+            U.warn(log, "Page eviction mode will have no effect because the 
oldest pages are evicted automatically " +
+                "if Ignite persistence is enabled: " + regCfg.getName());
         }
     }
 
@@ -2240,7 +2202,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                     "[cpStatus=" + status + ", lastRead=" + lastReadPtr + "]");
 
             log.info("Finished applying memory changes [changesApplied=" + 
applied +
-                ", time=" + (U.currentTimeMillis() - start) + "ms]");
+                ", time=" + (U.currentTimeMillis() - start) + " ms]");
 
             if (applied > 0)
                 finalizeCheckpointOnRecovery(status.cpStartTs, 
status.cpStartId, status.startPtr);
@@ -2319,7 +2281,8 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                                             if (cacheCtx != null)
                                                 applyUpdate(cacheCtx, 
dataEntry);
                                             else if (log != null)
-                                                log.warning("Cache (cacheId=" 
+ cacheId + ") is not started, can't apply updates.");
+                                                log.warning("Cache is not 
started. Updates cannot be applied " +
+                                                    "[cacheId=" + cacheId + 
']');
                                         }
                                         finally {
                                             checkpointReadUnlock();
@@ -2491,7 +2454,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
 
         if (log.isInfoEnabled())
             log.info("Finished applying WAL changes [updatesApplied=" + 
applied +
-                ", time=" + (U.currentTimeMillis() - start) + "ms]");
+                ", time=" + (U.currentTimeMillis() - start) + " ms]");
     }
 
     /**
@@ -3208,7 +3171,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
             }
             finally {
                 if (err == null && !(stopping && isCancelled))
-                    err = new IllegalStateException("Thread " + name() + " is 
terminated unexpectedly");
+                    err = new IllegalStateException("Thread is terminated 
unexpectedly: " + name());
 
                 if (err instanceof OutOfMemoryError)
                     cctx.kernalContext().failure().process(new 
FailureContext(CRITICAL_ERROR, err));
@@ -3839,7 +3802,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                 }
                 catch (IgniteException e) {
                     U.error(log, "Failed to wait for snapshot operation 
initialization: " +
-                        curr.snapshotOperation + "]", e);
+                        curr.snapshotOperation, e);
                 }
             }
 
@@ -4518,9 +4481,9 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                         if (content == null)
                             content = readContent();
 
-                        log.warning("Failed to acquire file lock (local 
nodeId:" + ctx.localNodeId()
-                            + ", already locked by " + content + "), will try 
again in 1s: "
-                            + file.getAbsolutePath());
+                        log.warning("Failed to acquire file lock. Will try 
again in 1s " +
+                            "[nodeId=" + ctx.localNodeId() + ", holder=" + 
content +
+                            ", path=" + file.getAbsolutePath() + ']');
                     }
 
                     U.sleep(1000);
@@ -4529,8 +4492,8 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                 if (content == null)
                     content = readContent();
 
-                failMsg = "Failed to acquire file lock during " + 
(lockWaitTimeMillis / 1000) +
-                    " sec, (locked by " + content + "): " + 
file.getAbsolutePath();
+                failMsg = "Failed to acquire file lock [holder=" + content + 
", time=" + (lockWaitTimeMillis / 1000) +
+                    " sec, path=" + file.getAbsolutePath() + ']';
             }
             catch (Exception e) {
                 throw new IgniteCheckedException(e);
@@ -4816,7 +4779,7 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
                         return null;
                 }
 
-                log.error("Catch error during restore state, throwsCRCError=" 
+ throwsCRCError, e);
+                log.error("There is an error during restore state 
[throwsCRCError=" + throwsCRCError + ']', e);
 
                 throw e;
             }
@@ -4907,8 +4870,8 @@ public class GridCacheDatabaseSharedManager extends 
IgniteCacheDatabaseSharedMan
          * @return Flag indicates need throws CRC exception or not.
          */
         @Override public boolean throwsCRCError() {
-            log.info("Throws CRC error check, needApplyBinaryUpdates=" + 
needApplyBinaryUpdates +
-                ", lastArchivedSegment=" + lastArchivedSegment + ", lastRead=" 
+ lastRead);
+            log.info("Throws CRC error check [needApplyBinaryUpdates=" + 
needApplyBinaryUpdates +
+                ", lastArchivedSegment=" + lastArchivedSegment + ", lastRead=" 
+ lastRead + ']');
 
             if (needApplyBinaryUpdates)
                 return true;

http://git-wip-us.apache.org/repos/asf/ignite/blob/bdb22399/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java
----------------------------------------------------------------------
diff --git 
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java
 
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java
index 589b495..21bd454 100644
--- 
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java
+++ 
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java
@@ -94,6 +94,9 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
     private final boolean reuseMemory = 
IgniteSystemProperties.getBoolean(IGNITE_REUSE_MEMORY_ON_DEACTIVATE);
 
     /** */
+    private static final String MBEAN_GROUP_NAME = "DataRegionMetrics";
+
+    /** */
     protected volatile Map<String, DataRegion> dataRegionMap;
 
     /** */
@@ -140,44 +143,88 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
     }
 
     /**
-     * Registers MBeans for all DataRegionMetrics configured in this instance.
-     */
-    private void registerMetricsMBeans() {
-        if(U.IGNITE_MBEANS_DISABLED)
+     * @param cfg Ignite configuration.
+     * @param groupName Name of group.
+     * @param dataRegionName Metrics MBean name.
+     * @param impl Metrics implementation.
+     * @param clazz Metrics class type.
+     */
+    protected <T> void registerMetricsMBean(
+        IgniteConfiguration cfg,
+        String groupName,
+        String dataRegionName,
+        T impl,
+        Class<T> clazz
+    ) {
+        if (U.IGNITE_MBEANS_DISABLED)
             return;
 
-        IgniteConfiguration cfg = cctx.gridConfig();
-
-        for (DataRegionMetrics memMetrics : memMetricsMap.values()) {
-            DataRegionConfiguration memPlcCfg = 
dataRegionMap.get(memMetrics.getName()).config();
-
-            registerMetricsMBean((DataRegionMetricsImpl)memMetrics, memPlcCfg, 
cfg);
+        try {
+            U.registerMBean(
+                cfg.getMBeanServer(),
+                cfg.getIgniteInstanceName(),
+                groupName,
+                dataRegionName,
+                impl,
+                clazz);
+        }
+        catch (Throwable e) {
+            U.error(log, "Failed to register MBean with name: " + 
dataRegionName, e);
         }
     }
 
     /**
-     * @param memMetrics Memory metrics.
-     * @param dataRegionCfg Data region configuration.
      * @param cfg Ignite configuration.
+     * @param groupName Name of group.
+     * @param name Name of MBean.
      */
-    private void registerMetricsMBean(
-        DataRegionMetricsImpl memMetrics,
-        DataRegionConfiguration dataRegionCfg,
-        IgniteConfiguration cfg
+    protected void unregisterMetricsMBean(
+        IgniteConfiguration cfg,
+        String groupName,
+        String name
     ) {
-        assert !U.IGNITE_MBEANS_DISABLED;
+        if (U.IGNITE_MBEANS_DISABLED)
+            return;
+
+        assert cfg != null;
 
         try {
-            U.registerMBean(
-                cfg.getMBeanServer(),
-                cfg.getIgniteInstanceName(),
-                "DataRegionMetrics",
-                dataRegionCfg.getName(),
-                new DataRegionMetricsMXBeanImpl(memMetrics, dataRegionCfg),
-                DataRegionMetricsMXBean.class);
+            cfg.getMBeanServer().unregisterMBean(
+                U.makeMBeanName(
+                    cfg.getIgniteInstanceName(),
+                    groupName,
+                    name
+                ));
+        }
+        catch (InstanceNotFoundException ignored) {
+            // We tried to unregister a non-existing MBean, not a big deal.
         }
         catch (Throwable e) {
-            U.error(log, "Failed to register MBean for DataRegionMetrics with 
name: '" + memMetrics.getName() + "'", e);
+            U.error(log, "Failed to unregister MBean for memory metrics: " + 
name, e);
+        }
+    }
+
+    /**
+     * Registers MBeans for all DataRegionMetrics configured in this instance.
+     *
+     * @param cfg Ignite configuration.
+     */
+    protected void registerMetricsMBeans(IgniteConfiguration cfg) {
+        if (U.IGNITE_MBEANS_DISABLED)
+            return;
+
+        assert cfg != null;
+
+        for (DataRegionMetrics memMetrics : memMetricsMap.values()) {
+            DataRegionConfiguration memPlcCfg = 
dataRegionMap.get(memMetrics.getName()).config();
+
+            registerMetricsMBean(
+                cfg,
+                MBEAN_GROUP_NAME,
+                memPlcCfg.getName(),
+                new 
DataRegionMetricsMXBeanImpl((DataRegionMetricsImpl)memMetrics, memPlcCfg),
+                DataRegionMetricsMXBean.class
+            );
         }
     }
 
@@ -220,17 +267,6 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
     }
 
     /**
-     *
-     */
-    private void startMemoryPolicies() {
-        for (DataRegion memPlc : dataRegionMap.values()) {
-            memPlc.pageMemory().start();
-
-            memPlc.evictionTracker().start();
-        }
-    }
-
-    /**
      * @param memCfg Database config.
      * @throws IgniteCheckedException If failed to initialize swap path.
      */
@@ -320,7 +356,7 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
             dfltDataRegion = memPlc;
         else if (dataRegionName.equals(DFLT_DATA_REG_DEFAULT_NAME))
             U.warn(log, "Data Region with name 'default' isn't used as a 
default. " +
-                    "Please check Memory Policies configuration.");
+                    "Please, check Data Region configuration.");
     }
 
     /**
@@ -731,32 +767,6 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
         onDeActivate(true);
     }
 
-    /**
-     * Unregister MBean.
-     * @param name Name of mbean.
-     */
-    private void unregisterMBean(String name) {
-        if(U.IGNITE_MBEANS_DISABLED)
-            return;
-
-        IgniteConfiguration cfg = cctx.gridConfig();
-
-        try {
-            cfg.getMBeanServer().unregisterMBean(
-                U.makeMBeanName(
-                    cfg.getIgniteInstanceName(),
-                    "DataRegionMetrics", name
-                    ));
-        }
-        catch (InstanceNotFoundException ignored) {
-            // We tried to unregister a non-existing MBean, not a big deal.
-        }
-        catch (Throwable e) {
-            U.error(log, "Failed to unregister MBean for memory metrics: " +
-                name, e);
-        }
-    }
-
     /** {@inheritDoc} */
     @Override public boolean checkpointLockIsHeldByThread() {
         return true;
@@ -1197,9 +1207,13 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
 
         assert cfg != null;
 
-        registerMetricsMBeans();
+        registerMetricsMBeans(cctx.gridConfig());
 
-        startMemoryPolicies();
+        for (DataRegion memPlc : dataRegionMap.values()) {
+            memPlc.pageMemory().start();
+
+            memPlc.evictionTracker().start();
+        }
 
         initPageMemoryDataStructures(cfg);
 
@@ -1226,7 +1240,11 @@ public class IgniteCacheDatabaseSharedManager extends 
GridCacheSharedManagerAdap
 
                 memPlc.evictionTracker().stop();
 
-                unregisterMBean(memPlc.memoryMetrics().getName());
+                unregisterMetricsMBean(
+                    cctx.gridConfig(),
+                    MBEAN_GROUP_NAME,
+                    memPlc.memoryMetrics().getName()
+                );
             }
 
             dataRegionMap.clear();

http://git-wip-us.apache.org/repos/asf/ignite/blob/bdb22399/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/standbycluster/IgniteNoParrallelClusterIsAllowedTest.java
----------------------------------------------------------------------
diff --git 
a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/standbycluster/IgniteNoParrallelClusterIsAllowedTest.java
 
b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/standbycluster/IgniteNoParrallelClusterIsAllowedTest.java
index 5c986ee..7180d7b 100644
--- 
a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/standbycluster/IgniteNoParrallelClusterIsAllowedTest.java
+++ 
b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/standbycluster/IgniteNoParrallelClusterIsAllowedTest.java
@@ -76,7 +76,7 @@ public class IgniteNoParrallelClusterIsAllowedTest extends 
IgniteChangeGlobalSta
             while (true) {
                 String message = e.getMessage();
 
-                if (message.contains("Failed to acquire file lock during"))
+                if (message.contains("Failed to acquire file lock ["))
                     break;
 
                 if (e.getCause() != null)

Reply via email to