You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by dh...@apache.org on 2019/07/22 22:09:21 UTC
[geode] 08/18: Reorganize CachePerfStats/RegionPerfStats
constructors
This is an automated email from the ASF dual-hosted git repository.
dhemery pushed a commit to branch GEODE-7001-region-entry-count
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 31354fcdd62d06e5aed6f5c2a05eaeff85571689
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Wed Jul 17 16:19:07 2019 -0700
Reorganize CachePerfStats/RegionPerfStats constructors
---
.../geode/internal/cache/CachePerfStats.java | 76 ++++++++++--------
.../geode/internal/cache/DummyCachePerfStats.java | 4 +
.../apache/geode/internal/cache/LocalRegion.java | 7 +-
.../internal/cache/PartitionedRegionDataStore.java | 2 +-
.../internal/cache/PartitionedRegionHelper.java | 2 +-
.../geode/internal/cache/RegionPerfStats.java | 27 ++++---
.../internal/cache/wan/AbstractGatewaySender.java | 3 +-
.../geode/internal/statistics/CallbackSampler.java | 2 +-
.../geode/internal/statistics/StatisticsImpl.java | 11 +--
.../internal/statistics/SuppliableStatistics.java | 35 +++++++++
.../management/internal/FederatingManager.java | 3 +-
.../geode/management/internal/LocalManager.java | 3 +-
.../geode/internal/cache/CachePerfStatsTest.java | 51 ++++++++++--
.../geode/internal/cache/RegionPerfStatsTest.java | 90 ++++++++++++++--------
.../internal/statistics/CallbackSamplerTest.java | 4 +-
.../internal/statistics/SimpleStatistics.java | 70 -----------------
.../internal/statistics/StatisticsImplTest.java | 12 +--
.../bean/stats/MemberLevelStatsTest.java | 4 +-
18 files changed, 229 insertions(+), 177 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
index aedbd81..167c471 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.internal.cache;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.function.LongSupplier;
import org.apache.geode.StatisticDescriptor;
@@ -624,33 +625,42 @@ public class CachePerfStats {
private final LongSupplier clock;
- /**
- * Created specially for bug 39348. Should not be invoked in any other case.
- */
- public CachePerfStats() {
- this(null);
- }
+ private final AtomicLong entryCount;
- /**
- * Creates a new <code>CachePerfStats</code> and registers itself with the given statistics
- * factory.
- */
public CachePerfStats(StatisticsFactory factory) {
- this(factory, "cachePerfStats", enableClockStats ? NanoTimer::getTime : () -> 0);
+ this(factory, "cachePerfStats");
}
- /**
- * Creates a new <code>CachePerfStats</code> and registers itself with the given statistics
- * factory.
- */
- public CachePerfStats(StatisticsFactory factory, String regionName) {
- this(factory, "RegionStats-" + regionName, enableClockStats ? NanoTimer::getTime : () -> 0);
+ @VisibleForTesting
+ public CachePerfStats(StatisticsFactory factory, LongSupplier clock) {
+ this(factory, "cachePerfStats", clock, new AtomicLong());
}
- @VisibleForTesting
- public CachePerfStats(StatisticsFactory factory, String textId, LongSupplier clock) {
- stats = factory == null ? null : factory.createAtomicStatistics(type, textId);
+ public CachePerfStats(StatisticsFactory factory, String textId) {
+ this(factory, textId, createClock(), new AtomicLong());
+ }
+
+ CachePerfStats(StatisticsFactory factory, String textId, LongSupplier clock,
+ AtomicLong entryCount) {
+ this(createStatistics(factory, textId), clock, entryCount);
+ }
+
+ private CachePerfStats(Statistics stats, LongSupplier clock, AtomicLong entryCount) {
+ this.stats = stats;
this.clock = clock;
+ this.entryCount = entryCount;
+ stats.setLongSupplier(entryGaugeId, entryCount::get);
+ }
+
+ private static Statistics createStatistics(StatisticsFactory factory, String textId) {
+ if (factory == null) {
+ return null;
+ }
+ return factory.createAtomicStatistics(type, textId);
+ }
+
+ private static LongSupplier createClock() {
+ return enableClockStats ? NanoTimer::getTime : () -> 0;
}
/**
@@ -668,10 +678,23 @@ public class CachePerfStats {
return type;
}
+ /**
+ * Returns the Statistics instance that stores the cache perf stats.
+ *
+ * @since GemFire 3.5
+ */
+ public Statistics getStats() {
+ return stats;
+ }
+
protected long getTime() {
return clock.getAsLong();
}
+ protected AtomicLong entryCount() {
+ return entryCount;
+ }
+
public int getLoadsCompleted() {
return stats.getInt(loadsCompletedId);
}
@@ -1307,11 +1330,11 @@ public class CachePerfStats {
}
public void incEntryCount(int delta) {
- stats.incLong(entryGaugeId, delta);
+ entryCount.addAndGet(delta);
}
public long getEntries() {
- return stats.getLong(entryGaugeId);
+ return entryCount.get();
}
public void incRetries() {
@@ -1363,15 +1386,6 @@ public class CachePerfStats {
}
/**
- * Returns the Statistics instance that stores the cache perf stats.
- *
- * @since GemFire 3.5
- */
- public Statistics getStats() {
- return stats;
- }
-
- /**
* Returns a helper object so that the event pool can record its stats to the proper cache perf
* stats.
*
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DummyCachePerfStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DummyCachePerfStats.java
index 1a574e5..fa704c2 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DummyCachePerfStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DummyCachePerfStats.java
@@ -22,6 +22,10 @@ import org.apache.geode.distributed.internal.PoolStatHelper;
*/
public class DummyCachePerfStats extends CachePerfStats {
+ DummyCachePerfStats() {
+ super(null);
+ }
+
@Override
public int getLoadsCompleted() {
return 0;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 992dab3..46db44e 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -597,9 +597,10 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
cachePerfStats = cache.getCachePerfStats();
} else {
hasOwnStats = true;
- cachePerfStats = new RegionPerfStats(
- cache.getInternalDistributedSystem().getStatisticsManager(), cache.getCachePerfStats(),
- regionName, cache.getMeterRegistry());
+ cachePerfStats =
+ new RegionPerfStats(cache.getInternalDistributedSystem().getStatisticsManager(),
+ "RegionStats-" + regionName, cache.getCachePerfStats(), regionName,
+ cache.getMeterRegistry());
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
index c0a14c0..346e4d0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
@@ -204,7 +204,7 @@ public class PartitionedRegionDataStore implements HasCachePerfStats {
// this.bucketStats = new CachePerfStats(pr.getSystem(), "partition-" + pr.getName());
this.bucketStats =
new RegionPerfStats(pr.getCache().getInternalDistributedSystem().getStatisticsManager(),
- pr.getCachePerfStats(), "partition-" + pr.getName(),
+ "RegionStats-partition-" + pr.getName(), pr.getCachePerfStats(), pr.getName(),
pr.getCache().getMeterRegistry());
this.keysOfInterest = new ConcurrentHashMap();
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionHelper.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionHelper.java
index 23eda2d..b83c4fa 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionHelper.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionHelper.java
@@ -268,7 +268,7 @@ public class PartitionedRegionHelper {
final HasCachePerfStats prMetaStatsHolder = new HasCachePerfStats() {
@Override
public CachePerfStats getCachePerfStats() {
- return new CachePerfStats(cache.getDistributedSystem(), "partitionMetaData");
+ return new CachePerfStats(cache.getDistributedSystem(), "RegionStats-partitionMetaData");
}
};
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
index d36f012..13a54fc 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.internal.cache;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.function.LongSupplier;
import io.micrometer.core.instrument.Gauge;
@@ -29,29 +30,31 @@ class RegionPerfStats extends CachePerfStats {
private final MeterRegistry meterRegistry;
private final Gauge entriesGauge;
- RegionPerfStats(StatisticsFactory statisticsFactory, CachePerfStats cachePerfStats,
+ RegionPerfStats(StatisticsFactory statisticsFactory, String textId, CachePerfStats cachePerfStats,
String regionName, MeterRegistry meterRegistry) {
- this(statisticsFactory, cachePerfStats, regionName, meterRegistry,
- enableClockStats ? NanoTimer::getTime : () -> 0);
+ this(statisticsFactory, textId, createClock(), new AtomicLong(), cachePerfStats, regionName,
+ meterRegistry);
}
@VisibleForTesting
- RegionPerfStats(StatisticsFactory statisticsFactory, CachePerfStats cachePerfStats,
- String regionName, MeterRegistry meterRegistry, LongSupplier clock) {
- super(statisticsFactory, "RegionStats-" + regionName, clock);
+ RegionPerfStats(StatisticsFactory statisticsFactory, String textId, LongSupplier clock,
+ AtomicLong entryCount, CachePerfStats cachePerfStats, String regionName,
+ MeterRegistry meterRegistry) {
+ super(statisticsFactory, textId, clock, entryCount);
this.cachePerfStats = cachePerfStats;
this.meterRegistry = meterRegistry;
- entriesGauge = Gauge.builder("member.region.entries", stats,
- value -> {
- long longValue = value.getLong(entryGaugeId);
- return (double) longValue;
- })
+ entriesGauge = Gauge.builder("member.region.entries", entryCount, AtomicLong::get)
.description("Current number of entries in the region.")
.baseUnit("entries")
+ .tag("region.name", regionName)
.register(meterRegistry);
}
+ private static LongSupplier createClock() {
+ return enableClockStats ? NanoTimer::getTime : () -> 0;
+ }
+
@Override
protected void close() {
meterRegistry.remove(entriesGauge);
@@ -484,7 +487,7 @@ class RegionPerfStats extends CachePerfStats {
@Override
public void incEntryCount(int delta) {
- stats.incLong(entryGaugeId, delta);
+ super.incEntryCount(delta);
cachePerfStats.incEntryCount(delta);
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
index 8563e8d..3c4411f 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
@@ -1256,7 +1256,8 @@ public abstract class AbstractGatewaySender implements InternalGatewaySender, Di
final HasCachePerfStats statsHolder = new HasCachePerfStats() {
@Override
public CachePerfStats getCachePerfStats() {
- return new CachePerfStats(cache.getDistributedSystem(), META_DATA_REGION_NAME);
+ return new CachePerfStats(cache.getDistributedSystem(),
+ "RegionStats-" + META_DATA_REGION_NAME);
}
};
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/CallbackSampler.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/CallbackSampler.java
index f50c30e..d5ad2f0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/CallbackSampler.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/CallbackSampler.java
@@ -64,7 +64,7 @@ public class CallbackSampler {
try {
for (Statistics stats : statisticsManager.getStatsList()) {
StatisticsImpl statistics = (StatisticsImpl) stats;
- errors += statistics.invokeSuppliers();
+ errors += statistics.updateSuppliedValues();
suppliers += statistics.getSupplierCount();
}
} catch (VirtualMachineError e) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsImpl.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsImpl.java
index c8a8551..125296a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsImpl.java
@@ -40,7 +40,7 @@ import org.apache.geode.internal.util.concurrent.CopyOnWriteHashMap;
*
* @since GemFire 3.0
*/
-public abstract class StatisticsImpl implements Statistics {
+public abstract class StatisticsImpl implements SuppliableStatistics {
private static final Logger logger = LogService.getLogger();
@@ -504,13 +504,8 @@ public abstract class StatisticsImpl implements Statistics {
// nothing needed in this impl.
}
- /**
- * Invoke sample suppliers to retrieve the current value for the suppler controlled sets and
- * update the stats to reflect the supplied values.
- *
- * @return the number of callback errors that occurred while sampling stats
- */
- int invokeSuppliers() {
+ @Override
+ public int updateSuppliedValues() {
int errors = 0;
for (Map.Entry<Integer, IntSupplier> entry : intSuppliers.entrySet()) {
try {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/SuppliableStatistics.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/SuppliableStatistics.java
new file mode 100644
index 0000000..0f01907
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/SuppliableStatistics.java
@@ -0,0 +1,35 @@
+/*
+ * 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.geode.internal.statistics;
+
+import org.apache.geode.Statistics;
+
+public interface SuppliableStatistics extends Statistics {
+
+ /**
+ * Cast {@code Statistics} to {@code SuppliableStatistics}.
+ */
+ static SuppliableStatistics toSuppliableStatistics(Statistics statistics) {
+ return (SuppliableStatistics) statistics;
+ }
+
+ /**
+ * Invoke sample suppliers to retrieve the current value for the supplier controlled sets and
+ * update the stats to reflect the supplied values.
+ *
+ * @return the number of callback errors that occurred while sampling stats
+ */
+ int updateSuppliedValues();
+}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
index 0fdc80d..12e70ba 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
@@ -374,7 +374,8 @@ public class FederatingManager extends Manager {
// Create anonymous stats holder for Management Regions
HasCachePerfStats monitoringRegionStats =
- () -> new CachePerfStats(cache.getDistributedSystem(), "managementRegionStats");
+ () -> new CachePerfStats(cache.getDistributedSystem(),
+ "RegionStats-managementRegionStats");
internalRegionArguments.setCachePerfStatsHolder(monitoringRegionStats);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
index 38c3821..bb0cc86 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
@@ -118,7 +118,8 @@ public class LocalManager extends Manager {
final HasCachePerfStats monitoringRegionStats = new HasCachePerfStats() {
@Override
public CachePerfStats getCachePerfStats() {
- return new CachePerfStats(cache.getDistributedSystem(), "managementRegionStats");
+ return new CachePerfStats(cache.getDistributedSystem(),
+ "RegionStats-managementRegionStats");
}
};
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/CachePerfStatsTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/CachePerfStatsTest.java
index 4ae2c01..f1c7800 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/CachePerfStatsTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/CachePerfStatsTest.java
@@ -26,6 +26,7 @@ import static org.apache.geode.internal.cache.CachePerfStats.deltaUpdatesId;
import static org.apache.geode.internal.cache.CachePerfStats.deltasPreparedId;
import static org.apache.geode.internal.cache.CachePerfStats.deltasSentId;
import static org.apache.geode.internal.cache.CachePerfStats.destroysId;
+import static org.apache.geode.internal.cache.CachePerfStats.entryGaugeId;
import static org.apache.geode.internal.cache.CachePerfStats.evictorJobsCompletedId;
import static org.apache.geode.internal.cache.CachePerfStats.evictorJobsStartedId;
import static org.apache.geode.internal.cache.CachePerfStats.getInitialImagesCompletedId;
@@ -53,18 +54,17 @@ import static org.apache.geode.internal.cache.CachePerfStats.updatesId;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import org.apache.geode.Statistics;
import org.apache.geode.StatisticsFactory;
import org.apache.geode.StatisticsType;
import org.apache.geode.internal.statistics.StatisticsManager;
import org.apache.geode.internal.statistics.StripedStatisticsImpl;
+import org.apache.geode.internal.statistics.SuppliableStatistics;
/**
* Unit tests for {@link CachePerfStats}.
@@ -74,7 +74,7 @@ public class CachePerfStatsTest {
private static final String TEXT_ID = "cachePerfStats";
private static final long CLOCK_TIME = 10;
- private Statistics statistics;
+ private SuppliableStatistics statistics;
private CachePerfStats cachePerfStats;
@Before
@@ -84,13 +84,13 @@ public class CachePerfStatsTest {
StatisticsManager statisticsManager = mock(StatisticsManager.class);
StatisticsFactory statisticsFactory = mock(StatisticsFactory.class);
- statistics = spy(new StripedStatisticsImpl(statisticsType, TEXT_ID, 1, 1, statisticsManager));
+ statistics = new StripedStatisticsImpl(statisticsType, TEXT_ID, 1, 1, statisticsManager);
when(statisticsFactory.createAtomicStatistics(eq(statisticsType), eq(TEXT_ID)))
.thenReturn(statistics);
CachePerfStats.enableClockStats = true;
- cachePerfStats = new CachePerfStats(statisticsFactory, TEXT_ID, () -> CLOCK_TIME);
+ cachePerfStats = new CachePerfStats(statisticsFactory, () -> CLOCK_TIME);
}
@After
@@ -1125,4 +1125,45 @@ public class CachePerfStatsTest {
assertThat(cachePerfStats.getDeltaFullValuesRequested()).isNegative();
}
+
+ @Test
+ public void incEntryCount_whenDeltaIsPositive_increasesTheEntryCount() {
+ cachePerfStats.incEntryCount(2);
+
+ assertThat(cachePerfStats.getEntries()).isEqualTo(2);
+ }
+
+ @Test
+ public void incEntryCount_whenDeltaIsNegative_decreasesTheEntryCount() {
+ cachePerfStats.incEntryCount(-2);
+
+ assertThat(cachePerfStats.getEntries()).isEqualTo(-2);
+ }
+
+ @Test
+ public void getEntriesReturnsZeroByDefault() {
+ long defaultValue = cachePerfStats.getEntries();
+
+ assertThat(defaultValue).isZero();
+ }
+
+ @Test
+ public void entryGaugeIdStatIsSuppliedByIncEntryCount() {
+ int delta = 2;
+
+ cachePerfStats.incEntryCount(delta);
+
+ statistics.updateSuppliedValues();
+
+ assertThat(statistics.getLong(entryGaugeId)).isEqualTo(delta);
+ }
+
+ @Test
+ public void entryGaugeIdStatIsSuppliedZeroByDefault() {
+ statistics.updateSuppliedValues();
+
+ long defaultValue = statistics.getLong(entryGaugeId);
+
+ assertThat(defaultValue).isZero();
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
index 33b6472..d91ee2a 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
@@ -14,14 +14,16 @@
*/
package org.apache.geode.internal.cache;
-import static org.apache.geode.internal.cache.CachePerfStats.entryGaugeId;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.quality.Strictness.STRICT_STUBS;
+import java.util.concurrent.atomic.AtomicLong;
+
import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.Meter;
import io.micrometer.core.instrument.MeterRegistry;
@@ -31,7 +33,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
-import org.mockito.ArgumentCaptor;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
@@ -40,14 +41,15 @@ import org.apache.geode.StatisticsFactory;
public class RegionPerfStatsTest {
- private static final String REGION_NAME = "regionName";
+ private static final String REGION_NAME = "region1";
+ private static final String TEXT_ID = "textId";
private MeterRegistry meterRegistry;
private StatisticsFactory statisticsFactory;
- private Statistics statistics;
private CachePerfStats cachePerfStats;
private RegionPerfStats regionPerfStats;
+ private RegionPerfStats regionPerfStats2;
@Rule
public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(STRICT_STUBS);
@@ -55,14 +57,13 @@ public class RegionPerfStatsTest {
@Before
public void setUp() {
meterRegistry = new SimpleMeterRegistry();
- statisticsFactory = mock(StatisticsFactory.class);
cachePerfStats = mock(CachePerfStats.class);
- statistics = mock(Statistics.class);
+ statisticsFactory = mock(StatisticsFactory.class);
- when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(statistics);
+ when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(mock(Statistics.class));
regionPerfStats =
- new RegionPerfStats(statisticsFactory, cachePerfStats, REGION_NAME, meterRegistry, () -> 0);
+ new RegionPerfStats(statisticsFactory, TEXT_ID, cachePerfStats, REGION_NAME, meterRegistry);
}
@After
@@ -70,58 +71,81 @@ public class RegionPerfStatsTest {
if (regionPerfStats != null) {
regionPerfStats.close();
}
+ if (regionPerfStats2 != null) {
+ regionPerfStats2.close();
+ }
}
@Test
- public void textIdIsRegionStatsHyphenRegionName() {
- ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
- verify(statisticsFactory).createAtomicStatistics(any(), captor.capture());
+ public void createsStatisticsUsingTextId() {
+ StatisticsFactory statisticsFactory = mock(StatisticsFactory.class);
+ when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(mock(Statistics.class));
- assertThat(captor.getValue()).isEqualTo("RegionStats-" + REGION_NAME);
+ regionPerfStats =
+ new RegionPerfStats(statisticsFactory, TEXT_ID, cachePerfStats, REGION_NAME, meterRegistry);
+
+ verify(statisticsFactory).createAtomicStatistics(any(), eq(TEXT_ID));
}
@Test
- public void incEntryCount_whenDeltaIsPositive_increasesEntryGaugeStat() {
- regionPerfStats.incEntryCount(1);
+ public void incEntryCount_incrementsCachePerfStatsEntryCount() {
+ regionPerfStats.incEntryCount(2);
- verify(statistics).incLong(entryGaugeId, 1);
+ verify(cachePerfStats).incEntryCount(2);
}
@Test
- public void incEntryCount_whenDeltaIsNegative_decreasesEntryGaugeStat() {
- regionPerfStats.incEntryCount(-1);
-
- verify(statistics).incLong(entryGaugeId, -1);
+ public void regionPerfStatsConstruction_createsEntriesGauge_taggedWithRegionName() {
+ Gauge entriesGauge = meterRegistry
+ .find("member.region.entries")
+ .tag("region.name", REGION_NAME)
+ .gauge();
+ assertThat(entriesGauge).isNotNull();
}
@Test
- public void getEntries_getsValueFromEntryGaugeStat() {
- long statValue = 22;
- when(statistics.getLong(entryGaugeId)).thenReturn(statValue);
+ public void entriesGauge_showsAccumulatedEntryCount() {
+ regionPerfStats.incEntryCount(1);
+ regionPerfStats.incEntryCount(3);
+ regionPerfStats.incEntryCount(-1);
- regionPerfStats =
- new RegionPerfStats(statisticsFactory, cachePerfStats, REGION_NAME, meterRegistry, () -> 0);
+ assertThat(regionPerfStats.getEntries()).isEqualTo(3);
- assertThat(regionPerfStats.getEntries()).isEqualTo(statValue);
+ Gauge entriesGauge = meterRegistry
+ .find("member.region.entries")
+ .tag("region.name", REGION_NAME)
+ .gauge();
+ assertThat(entriesGauge.value()).isEqualTo(3);
}
@Test
- public void regionPerfStatsConstruction_createsEntriesGauge() {
+ public void incEntryCount_updatesMeterForThisRegion() {
+ regionPerfStats.incEntryCount(1);
+ regionPerfStats.incEntryCount(3);
+ regionPerfStats.incEntryCount(-1);
+
Gauge entriesGauge = meterRegistry
.find("member.region.entries")
+ .tag("region.name", REGION_NAME)
.gauge();
- assertThat(entriesGauge).isNotNull();
+ assertThat(entriesGauge.value()).isEqualTo(3);
}
@Test
- public void entriesGauge_getsValueFromEntryGaugeStat() {
- long statValue = 22;
- when(statistics.getLong(entryGaugeId)).thenReturn(statValue);
+ public void incEntryCount_doesNotUpdateMeterForOtherRegion() {
+ regionPerfStats2 =
+ new RegionPerfStats(statisticsFactory, "region2", () -> 0, new AtomicLong(), cachePerfStats,
+ "region2", meterRegistry);
- regionPerfStats =
- new RegionPerfStats(statisticsFactory, cachePerfStats, REGION_NAME, meterRegistry, () -> 0);
+ regionPerfStats.incEntryCount(1);
+ regionPerfStats.incEntryCount(3);
+ regionPerfStats.incEntryCount(-1);
- assertThat(regionPerfStats.getEntries()).isEqualTo(statValue);
+ Gauge region2EntriesGauge = meterRegistry
+ .find("member.region.entries")
+ .tag("region.name", "region2")
+ .gauge();
+ assertThat(region2EntriesGauge.value()).isZero();
}
@Test
diff --git a/geode-core/src/test/java/org/apache/geode/internal/statistics/CallbackSamplerTest.java b/geode-core/src/test/java/org/apache/geode/internal/statistics/CallbackSamplerTest.java
index 4cfcd52..a392f1f 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/statistics/CallbackSamplerTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/statistics/CallbackSamplerTest.java
@@ -61,8 +61,8 @@ public class CallbackSamplerTest {
StatisticsImpl stats1 = mock(StatisticsImpl.class);
StatisticsImpl stats2 = mock(StatisticsImpl.class);
- when(stats1.invokeSuppliers()).thenReturn(3);
- when(stats2.invokeSuppliers()).thenReturn(2);
+ when(stats1.updateSuppliedValues()).thenReturn(3);
+ when(stats2.updateSuppliedValues()).thenReturn(2);
when(stats1.getSupplierCount()).thenReturn(7);
when(stats2.getSupplierCount()).thenReturn(8);
when(statisticsManager.getStatsList()).thenReturn(Arrays.asList(stats1, stats2));
diff --git a/geode-core/src/test/java/org/apache/geode/internal/statistics/SimpleStatistics.java b/geode-core/src/test/java/org/apache/geode/internal/statistics/SimpleStatistics.java
deleted file mode 100644
index 96e05f3..0000000
--- a/geode-core/src/test/java/org/apache/geode/internal/statistics/SimpleStatistics.java
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- * 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.geode.internal.statistics;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import org.apache.geode.StatisticsType;
-
-public class SimpleStatistics extends StatisticsImpl {
-
- private final Map<Number, Number> values = new HashMap<>();
-
- public SimpleStatistics(StatisticsType type, String textId, long numericId, long uniqueId,
- int osStatFlags, StatisticsManager statisticsManager) {
- super(type, textId, numericId, uniqueId, osStatFlags, statisticsManager);
- }
-
- public SimpleStatistics(StatisticsType type, String textId, long numericId, long uniqueId,
- int osStatFlags, StatisticsManager statisticsManager, StatisticsLogger statisticsLogger) {
- super(type, textId, numericId, uniqueId, osStatFlags, statisticsManager, statisticsLogger);
- }
-
- @Override
- public boolean isAtomic() {
- return false;
- }
-
- @Override
- protected void _setLong(int offset, long value) {
- values.put(offset, value);
- }
-
- @Override
- protected void _setDouble(int offset, double value) {
- values.put(offset, value);
- }
-
- @Override
- protected long _getLong(int offset) {
- return (long) values.get(offset);
- }
-
- @Override
- protected double _getDouble(int offset) {
- return (double) values.get(offset);
- }
-
- @Override
- protected void _incLong(int offset, long delta) {
- values.put(offset, (long) values.get(delta) + 1);
- }
-
- @Override
- protected void _incDouble(int offset, double delta) {
- values.put(offset, (double) values.get(delta) + 1);
- }
-}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/statistics/StatisticsImplTest.java b/geode-core/src/test/java/org/apache/geode/internal/statistics/StatisticsImplTest.java
index 35c43cb..0d3f9c3 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/statistics/StatisticsImplTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/statistics/StatisticsImplTest.java
@@ -69,7 +69,7 @@ public class StatisticsImplTest {
IntSupplier intSupplier = mock(IntSupplier.class);
when(intSupplier.getAsInt()).thenReturn(23);
statistics.setIntSupplier(4, intSupplier);
- assertThat(statistics.invokeSuppliers()).isEqualTo(0);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(0);
verify(intSupplier).getAsInt();
assertThat(statistics.getInt(4)).isEqualTo(23);
@@ -80,7 +80,7 @@ public class StatisticsImplTest {
LongSupplier longSupplier = mock(LongSupplier.class);
when(longSupplier.getAsLong()).thenReturn(23L);
statistics.setLongSupplier(4, longSupplier);
- assertThat(statistics.invokeSuppliers()).isEqualTo(0);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(0);
verify(longSupplier).getAsLong();
assertThat(statistics.getLong(4)).isEqualTo(23L);
@@ -91,7 +91,7 @@ public class StatisticsImplTest {
DoubleSupplier doubleSupplier = mock(DoubleSupplier.class);
when(doubleSupplier.getAsDouble()).thenReturn(23.3);
statistics.setDoubleSupplier(4, doubleSupplier);
- assertThat(statistics.invokeSuppliers()).isEqualTo(0);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(0);
verify(doubleSupplier).getAsDouble();
assertThat(statistics.getDouble(4)).isEqualTo(23.3);
@@ -109,7 +109,7 @@ public class StatisticsImplTest {
IntSupplier throwingSupplier = mock(IntSupplier.class);
when(throwingSupplier.getAsInt()).thenThrow(NullPointerException.class);
statistics.setIntSupplier(4, throwingSupplier);
- assertThat(statistics.invokeSuppliers()).isEqualTo(1);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(1);
verify(throwingSupplier).getAsInt();
}
@@ -123,13 +123,13 @@ public class StatisticsImplTest {
IntSupplier throwingSupplier = mock(IntSupplier.class);
when(throwingSupplier.getAsInt()).thenThrow(NullPointerException.class);
statistics.setIntSupplier(4, throwingSupplier);
- assertThat(statistics.invokeSuppliers()).isEqualTo(1);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(1);
// String message, Object p0, Object p1, Object p2
verify(statisticsLogger).logWarning(anyString(), isNull(), anyInt(),
isA(NullPointerException.class));
- assertThat(statistics.invokeSuppliers()).isEqualTo(1);
+ assertThat(statistics.updateSuppliedValues()).isEqualTo(1);
// Make sure the logger isn't invoked again
verify(statisticsLogger).logWarning(anyString(), isNull(), anyInt(),
diff --git a/geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsTest.java b/geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsTest.java
index b04aad1..21e166d 100644
--- a/geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.management.bean.stats;
+import static org.apache.geode.internal.statistics.SuppliableStatistics.toSuppliableStatistics;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -86,7 +87,7 @@ public class MemberLevelStatsTest {
statSampler.start();
- cachePerfStats = new CachePerfStats(statisticsManager, "cachePerfStats", clockTime::get);
+ cachePerfStats = new CachePerfStats(statisticsManager, clockTime::get);
funcServiceStats = new FunctionServiceStats(statisticsManager, "FunctionExecution",
clockTime::get);
distributionStats =
@@ -340,6 +341,7 @@ public class MemberLevelStatsTest {
}
private void sampleStats() {
+ toSuppliableStatistics(cachePerfStats.getStats()).updateSuppliedValues();
statSampler.getSampleCollector().sample(clockTime.get());
}