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:26 UTC
[geode] 13/18: Try keeping entry count accumulator in
RegionPerfStats
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 f8864a2b9c650261fbfb8e52a0e9076bce19bf42
Author: Dale Emery <de...@pivotal.io>
AuthorDate: Thu Jul 18 17:30:14 2019 -0700
Try keeping entry count accumulator in RegionPerfStats
Co-Authored-By: Mark Hanson <mh...@pivotal.io>
---
.../geode/internal/cache/CachePerfStats.java | 24 ++++++-------------
.../geode/internal/cache/RegionPerfStats.java | 10 ++++----
.../geode/internal/cache/RegionPerfStatsTest.java | 27 +++++++++++-----------
3 files changed, 26 insertions(+), 35 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 167c471..41dfe2b 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,7 +14,6 @@
*/
package org.apache.geode.internal.cache;
-import java.util.concurrent.atomic.AtomicLong;
import java.util.function.LongSupplier;
import org.apache.geode.StatisticDescriptor;
@@ -625,31 +624,26 @@ public class CachePerfStats {
private final LongSupplier clock;
- private final AtomicLong entryCount;
-
public CachePerfStats(StatisticsFactory factory) {
this(factory, "cachePerfStats");
}
@VisibleForTesting
public CachePerfStats(StatisticsFactory factory, LongSupplier clock) {
- this(factory, "cachePerfStats", clock, new AtomicLong());
+ this(factory, "cachePerfStats", clock);
}
public CachePerfStats(StatisticsFactory factory, String textId) {
- this(factory, textId, createClock(), new AtomicLong());
+ this(factory, textId, createClock());
}
- CachePerfStats(StatisticsFactory factory, String textId, LongSupplier clock,
- AtomicLong entryCount) {
- this(createStatistics(factory, textId), clock, entryCount);
+ CachePerfStats(StatisticsFactory factory, String textId, LongSupplier clock) {
+ this(createStatistics(factory, textId), clock);
}
- private CachePerfStats(Statistics stats, LongSupplier clock, AtomicLong entryCount) {
+ private CachePerfStats(Statistics stats, LongSupplier clock) {
this.stats = stats;
this.clock = clock;
- this.entryCount = entryCount;
- stats.setLongSupplier(entryGaugeId, entryCount::get);
}
private static Statistics createStatistics(StatisticsFactory factory, String textId) {
@@ -691,10 +685,6 @@ public class CachePerfStats {
return clock.getAsLong();
}
- protected AtomicLong entryCount() {
- return entryCount;
- }
-
public int getLoadsCompleted() {
return stats.getInt(loadsCompletedId);
}
@@ -1330,11 +1320,11 @@ public class CachePerfStats {
}
public void incEntryCount(int delta) {
- entryCount.addAndGet(delta);
+ stats.incLong(entryGaugeId, delta);
}
public long getEntries() {
- return entryCount.get();
+ return stats.getLong(entryGaugeId);
}
public void incRetries() {
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 13a54fc..016ee77 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
@@ -29,21 +29,22 @@ class RegionPerfStats extends CachePerfStats {
private final CachePerfStats cachePerfStats;
private final MeterRegistry meterRegistry;
private final Gauge entriesGauge;
+ private final AtomicLong entryCount;
RegionPerfStats(StatisticsFactory statisticsFactory, String textId, CachePerfStats cachePerfStats,
String regionName, MeterRegistry meterRegistry) {
- this(statisticsFactory, textId, createClock(), new AtomicLong(), cachePerfStats, regionName,
+ this(statisticsFactory, textId, createClock(), cachePerfStats, regionName,
meterRegistry);
}
@VisibleForTesting
RegionPerfStats(StatisticsFactory statisticsFactory, String textId, LongSupplier clock,
- AtomicLong entryCount, CachePerfStats cachePerfStats, String regionName,
+ CachePerfStats cachePerfStats, String regionName,
MeterRegistry meterRegistry) {
- super(statisticsFactory, textId, clock, entryCount);
+ super(statisticsFactory, textId, clock);
this.cachePerfStats = cachePerfStats;
this.meterRegistry = meterRegistry;
-
+ entryCount = new AtomicLong();
entriesGauge = Gauge.builder("member.region.entries", entryCount, AtomicLong::get)
.description("Current number of entries in the region.")
.baseUnit("entries")
@@ -488,6 +489,7 @@ class RegionPerfStats extends CachePerfStats {
@Override
public void incEntryCount(int delta) {
super.incEntryCount(delta);
+ entryCount.addAndGet(delta);
cachePerfStats.incEntryCount(delta);
}
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 d91ee2a..0067b5c 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
@@ -22,8 +22,6 @@ 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;
@@ -50,6 +48,7 @@ public class RegionPerfStatsTest {
private RegionPerfStats regionPerfStats;
private RegionPerfStats regionPerfStats2;
+ private Statistics statistics;
@Rule
public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(STRICT_STUBS);
@@ -59,8 +58,9 @@ public class RegionPerfStatsTest {
meterRegistry = new SimpleMeterRegistry();
cachePerfStats = mock(CachePerfStats.class);
statisticsFactory = mock(StatisticsFactory.class);
+ statistics = mock(Statistics.class);
- when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(mock(Statistics.class));
+ when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(statistics);
regionPerfStats =
new RegionPerfStats(statisticsFactory, TEXT_ID, cachePerfStats, REGION_NAME, meterRegistry);
@@ -104,18 +104,17 @@ public class RegionPerfStatsTest {
}
@Test
- public void entriesGauge_showsAccumulatedEntryCount() {
- regionPerfStats.incEntryCount(1);
+ public void incEntryCount_updatesEntryGaugeStatistic() {
regionPerfStats.incEntryCount(3);
- regionPerfStats.incEntryCount(-1);
- assertThat(regionPerfStats.getEntries()).isEqualTo(3);
+ verify(statistics).incLong(RegionPerfStats.entryGaugeId, 3);
+ }
- Gauge entriesGauge = meterRegistry
- .find("member.region.entries")
- .tag("region.name", REGION_NAME)
- .gauge();
- assertThat(entriesGauge.value()).isEqualTo(3);
+ @Test
+ public void getEntryCount_updatesEntryGaugeStatistic() {
+ when(statistics.getLong(RegionPerfStats.entryGaugeId)).thenReturn(431L);
+
+ assertThat(regionPerfStats.getEntries()).isEqualTo(431);
}
@Test
@@ -134,8 +133,8 @@ public class RegionPerfStatsTest {
@Test
public void incEntryCount_doesNotUpdateMeterForOtherRegion() {
regionPerfStats2 =
- new RegionPerfStats(statisticsFactory, "region2", () -> 0, new AtomicLong(), cachePerfStats,
- "region2", meterRegistry);
+ new RegionPerfStats(statisticsFactory, "region2", () -> 0, cachePerfStats, "region2",
+ meterRegistry);
regionPerfStats.incEntryCount(1);
regionPerfStats.incEntryCount(3);