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)
