This is an automated email from the ASF dual-hosted git repository. bross pushed a commit to branch feature/GEODE-7665 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-7665 by this push: new 6e7b949 GEODE-7846: Adding Stats for Partitioned Region Clear (#5391) 6e7b949 is described below commit 6e7b949d83407aca094546b96a3d74dc9f9343c4 Author: BenjaminPerryRoss <39068135+benjaminperryr...@users.noreply.github.com> AuthorDate: Wed Aug 19 14:40:09 2020 -0700 GEODE-7846: Adding Stats for Partitioned Region Clear (#5391) Added stats to CachePerfStats for PR Clear - Changed clears to 'regionClears' and 'bucketClears' to differentiate between the number of times the region was cleared and the number of times a bucket was cleared in a PartitionedRegion - Added Local and Total duration stats to record how long clear has been running for a specific region, as well as how long it was spent clearing any specific member --- .../cache/RegionClearStatsDistributedTest.java | 2 +- .../cache/PartitionedRegionClearDUnitTest.java | 66 ++++++++++++++++++++++ .../geode/internal/cache/AbstractRegionMap.java | 7 ++- .../geode/internal/cache/CachePerfStats.java | 59 ++++++++++++++++--- .../geode/internal/cache/PartitionedRegion.java | 8 +++ .../internal/cache/PartitionedRegionClear.java | 14 ++++- .../geode/internal/cache/RegionPerfStats.java | 12 +++- .../apache/geode/internal/cache/RegionStats.java | 4 +- .../geode/internal/cache/CachePerfStatsTest.java | 53 +++++++++++++---- 9 files changed, 200 insertions(+), 25 deletions(-) diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache/RegionClearStatsDistributedTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache/RegionClearStatsDistributedTest.java index 52a4ade..50cea82 100755 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache/RegionClearStatsDistributedTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache/RegionClearStatsDistributedTest.java @@ -169,7 +169,7 @@ public class RegionClearStatsDistributedTest implements Serializable { } private void validateClearCountStat() { - assertThat(cacheRule.getCache().getCachePerfStats().getClearCount()) + assertThat(cacheRule.getCache().getCachePerfStats().getRegionClearCount()) .isEqualTo(EXPECTED_CLEAR_COUNT_STAT_VALUE); } } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionClearDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionClearDUnitTest.java index a3b311c..b871926 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionClearDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionClearDUnitTest.java @@ -152,6 +152,34 @@ public class PartitionedRegionClearDUnitTest implements Serializable { dataStore3.invoke(() -> verifyRegionSize(false, expectedNum)); } + private void verifyDatastoreStats(MemberVM datastore, boolean isCoordinator) { + datastore.invoke(() -> { + PartitionedRegion region = (PartitionedRegion) getRegion(false); + long clearCount = 0L; + int bucketCount = region.getDataStore().getAllLocalBucketRegions().size(); + + for (BucketRegion bucket : region.getDataStore().getAllLocalBucketRegions()) { + if (clearCount == 0) { + clearCount = bucket.getCachePerfStats().getBucketClearCount(); + } + assertThat(bucket.getCachePerfStats().getBucketClearCount()).isEqualTo(bucketCount); + } + + CachePerfStats stats = region.getRegionCachePerfStats(); + + assertThat(stats.getRegionClearCount()).isEqualTo(1); + assertThat(stats.getPartitionedRegionClearLocalDuration()) + .isGreaterThan(0); + if (isCoordinator) { + assertThat(stats.getPartitionedRegionClearTotalDuration()) + .isGreaterThan(0); + } else { + assertThat(stats.getPartitionedRegionClearTotalDuration()) + .isEqualTo(0); + } + }); + } + private void verifyClientRegionSize(int expectedNum) { client1.invoke(() -> verifyRegionSize(true, expectedNum)); // TODO: notify register clients @@ -331,6 +359,44 @@ public class PartitionedRegionClearDUnitTest implements Serializable { } @Test + public void normalClearFromDataStoreUpdatesStats() { + configureServers(false, true); + client1.invoke(this::initClientCache); + client2.invoke(this::initClientCache); + + // Verify no clears have been recorded in stats + dataStore1.invoke(() -> { + PartitionedRegion region = (PartitionedRegion) getRegion(false); + + for (BucketRegion bucket : region.getDataStore().getAllLocalBucketRegions()) { + long clearCount = bucket.getCachePerfStats().getRegionClearCount(); + assertThat(clearCount).isEqualTo(0); + } + }); + + accessor.invoke(() -> feed(false)); + verifyServerRegionSize(NUM_ENTRIES); + dataStore1.invoke(() -> getRegion(false).clear()); + verifyServerRegionSize(0); + + // Verify the stats were properly updated for the bucket regions + verifyDatastoreStats(dataStore1, true); + verifyDatastoreStats(dataStore2, false); + verifyDatastoreStats(dataStore3, false); + + + // The accessor shouldn't increment the region clear count + accessor.invoke(() -> { + PartitionedRegion region = (PartitionedRegion) getRegion(false); + + assertThat(region.getRegionCachePerfStats()).isNull(); + assertThat(region.getCachePerfStats().getRegionClearCount()).isEqualTo(0); + assertThat(region.getCachePerfStats().getPartitionedRegionClearLocalDuration()).isEqualTo(0); + assertThat(region.getCachePerfStats().getPartitionedRegionClearTotalDuration()).isEqualTo(0); + }); + } + + @Test public void normalClearFromClient() { configureServers(true, false); client1.invoke(this::initClientCache); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java index f1f765e..dc26126 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java @@ -344,7 +344,12 @@ public abstract class AbstractRegionMap extends BaseRegionMap if (lr != null && !(lr instanceof HARegion)) { CachePerfStats stats = lr.getCachePerfStats(); if (stats != null) { - stats.incClearCount(); + if (lr.isUsedForPartitionedRegionBucket()) { + stats.incBucketClearCount(); + } else { + stats.incRegionClearCount(); + } + } } } 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 7371ec0..5851b3f 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 @@ -117,7 +117,11 @@ public class CachePerfStats { static final int indexUpdateInProgressId; static final int indexUpdateCompletedId; static final int indexUpdateTimeId; - static final int clearsId; + static final int bucketClearsId; + static final int regionClearsId; + static final int partitionedRegionClearLocalDurationId; + static final int partitionedRegionClearTotalDurationId; + private static final int indexInitializationInProgressId; private static final int indexInitializationCompletedId; private static final int indexInitializationTimeId; @@ -274,7 +278,14 @@ public class CachePerfStats { "Current number of regions configured for reliablity that are missing required roles with Limited access"; final String reliableRegionsMissingNoAccessDesc = "Current number of regions configured for reliablity that are missing required roles with No access"; - final String clearsDesc = "The total number of times a clear has been done on this cache."; + final String regionClearsDesc = + "The total number of times a clear has been done on this cache."; + final String bucketClearsDesc = + "The total number of times a clear has been done on this region and it's bucket regions"; + final String partitionedRegionClearLocalDurationDesc = + "The time in nanoseconds partitioned region clear has been running for the region on this member"; + final String partitionedRegionClearTotalDurationDesc = + "The time in nanoseconds partitioned region clear has been running for the region with this member as coordinator."; final String metaDataRefreshCountDesc = "Total number of times the meta data is refreshed due to hopping observed."; final String conflatedEventsDesc = @@ -442,7 +453,12 @@ public class CachePerfStats { f.createIntCounter("retries", "Number of times a concurrent destroy followed by a create has caused an entry operation to need to retry.", "operations"), - f.createLongCounter("clears", clearsDesc, "operations"), + f.createLongCounter("regionClears", regionClearsDesc, "operations"), + f.createLongCounter("bucketClears", bucketClearsDesc, "operations"), + f.createLongCounter("partitionedRegionClearLocalDuration", + partitionedRegionClearLocalDurationDesc, "nanoseconds"), + f.createLongCounter("partitionedRegionClearTotalDuration", + partitionedRegionClearTotalDurationDesc, "nanoseconds"), f.createIntGauge("diskTasksWaiting", "Current number of disk tasks (oplog compactions, asynchronous recoveries, etc) that are waiting for a thread to run the operation", "operations"), @@ -573,7 +589,10 @@ public class CachePerfStats { eventsQueuedId = type.nameToId("eventsQueued"); retriesId = type.nameToId("retries"); - clearsId = type.nameToId("clears"); + regionClearsId = type.nameToId("regionClears"); + bucketClearsId = type.nameToId("bucketClears"); + partitionedRegionClearLocalDurationId = type.nameToId("partitionedRegionClearLocalDuration"); + partitionedRegionClearTotalDurationId = type.nameToId("partitionedRegionClearTotalDuration"); diskTasksWaitingId = type.nameToId("diskTasksWaiting"); evictorJobsStartedId = type.nameToId("evictorJobsStarted"); @@ -1353,12 +1372,36 @@ public class CachePerfStats { }; } - public long getClearCount() { - return stats.getLong(clearsId); + public long getRegionClearCount() { + return stats.getLong(regionClearsId); + } + + public long getBucketClearCount() { + return stats.getLong(bucketClearsId); + } + + public long getPartitionedRegionClearLocalDuration() { + return stats.getLong(partitionedRegionClearLocalDurationId); + } + + public long getPartitionedRegionClearTotalDuration() { + return stats.getLong(partitionedRegionClearTotalDurationId); + } + + public void incRegionClearCount() { + stats.incLong(regionClearsId, 1L); + } + + public void incBucketClearCount() { + stats.incLong(bucketClearsId, 1L); + } + + public void incPartitionedRegionClearLocalDuration(long durationNanos) { + stats.incLong(partitionedRegionClearLocalDurationId, durationNanos); } - public void incClearCount() { - stats.incLong(clearsId, 1L); + public void incPartitionedRegionClearTotalDuration(long durationNanos) { + stats.incLong(partitionedRegionClearTotalDurationId, durationNanos); } public long getConflatedEventsCount() { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java index deb1c42..cc711dc 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java @@ -574,6 +574,14 @@ public class PartitionedRegion extends LocalRegion return this.partitionListeners; } + public CachePerfStats getRegionCachePerfStats() { + if (dataStore != null && dataStore.getAllLocalBucketRegions().size() > 0) { + BucketRegion bucket = dataStore.getAllLocalBucketRegions().iterator().next(); + return bucket.getCachePerfStats(); + } + return null; + } + /** * Return canonical representation for a bucket (for logging) * diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionClear.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionClear.java index 1c9d5b2..4796a17 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionClear.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionClear.java @@ -123,6 +123,7 @@ public class PartitionedRegionClear { public Set<Integer> clearRegionLocal(RegionEventImpl regionEvent) { Set<Integer> clearedBuckets = new HashSet<>(); + long clearStartTime = System.nanoTime(); setMembershipChange(false); // Synchronized to handle the requester departure. synchronized (lockForListenerAndClientNotification) { @@ -156,6 +157,11 @@ public class PartitionedRegionClear { doAfterClear(regionEvent); } finally { partitionedRegion.getDataStore().unlockBucketCreationForRegionClear(); + if (clearedBuckets.size() != 0 && partitionedRegion.getCachePerfStats() != null) { + partitionedRegion.getRegionCachePerfStats().incRegionClearCount(); + partitionedRegion.getRegionCachePerfStats() + .incPartitionedRegionClearLocalDuration(System.nanoTime() - clearStartTime); + } } } else { // Non data-store with client queue and listener @@ -327,10 +333,12 @@ public class PartitionedRegionClear { void doClear(RegionEventImpl regionEvent, boolean cacheWrite) { String lockName = CLEAR_OPERATION + partitionedRegion.getName(); + long clearStartTime = 0; try { // distributed lock to make sure only one clear op is in progress in the cluster. acquireDistributedClearLock(lockName); + clearStartTime = System.nanoTime(); // Force all primary buckets to be created before clear. assignAllPrimaryBuckets(); @@ -367,9 +375,13 @@ public class PartitionedRegionClear { releaseLockForClear(regionEvent); } } - } finally { releaseDistributedClearLock(lockName); + CachePerfStats stats = partitionedRegion.getRegionCachePerfStats(); + if (stats != null) { + partitionedRegion.getRegionCachePerfStats() + .incPartitionedRegionClearTotalDuration(System.nanoTime() - clearStartTime); + } } } 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 d3c9891..30d60bf 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 @@ -520,9 +520,15 @@ class RegionPerfStats extends CachePerfStats implements RegionStats { } @Override - public void incClearCount() { - stats.incLong(clearsId, 1L); - cachePerfStats.incClearCount(); + public void incRegionClearCount() { + stats.incLong(regionClearsId, 1L); + cachePerfStats.incRegionClearCount(); + } + + @Override + public void incBucketClearCount() { + stats.incLong(bucketClearsId, 1L); + cachePerfStats.incBucketClearCount(); } @Override diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java index 2fe6cc1..4c0e446 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java @@ -135,7 +135,9 @@ public interface RegionStats { void incEvictWorkTime(long delta); - void incClearCount(); + void incBucketClearCount(); + + void incRegionClearCount(); void incPRQueryRetries(); 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 ef96053..0f915a4 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 @@ -14,9 +14,9 @@ */ package org.apache.geode.internal.cache; +import static org.apache.geode.internal.cache.CachePerfStats.bucketClearsId; import static org.apache.geode.internal.cache.CachePerfStats.cacheListenerCallsCompletedId; import static org.apache.geode.internal.cache.CachePerfStats.cacheWriterCallsCompletedId; -import static org.apache.geode.internal.cache.CachePerfStats.clearsId; import static org.apache.geode.internal.cache.CachePerfStats.createsId; import static org.apache.geode.internal.cache.CachePerfStats.deltaFailedUpdatesId; import static org.apache.geode.internal.cache.CachePerfStats.deltaFullValuesRequestedId; @@ -38,10 +38,13 @@ import static org.apache.geode.internal.cache.CachePerfStats.loadsCompletedId; import static org.apache.geode.internal.cache.CachePerfStats.missesId; import static org.apache.geode.internal.cache.CachePerfStats.netloadsCompletedId; import static org.apache.geode.internal.cache.CachePerfStats.netsearchesCompletedId; +import static org.apache.geode.internal.cache.CachePerfStats.partitionedRegionClearLocalDurationId; +import static org.apache.geode.internal.cache.CachePerfStats.partitionedRegionClearTotalDurationId; import static org.apache.geode.internal.cache.CachePerfStats.putAllsId; import static org.apache.geode.internal.cache.CachePerfStats.putTimeId; import static org.apache.geode.internal.cache.CachePerfStats.putsId; import static org.apache.geode.internal.cache.CachePerfStats.queryExecutionsId; +import static org.apache.geode.internal.cache.CachePerfStats.regionClearsId; import static org.apache.geode.internal.cache.CachePerfStats.removeAllsId; import static org.apache.geode.internal.cache.CachePerfStats.retriesId; import static org.apache.geode.internal.cache.CachePerfStats.txCommitChangesId; @@ -415,28 +418,58 @@ public class CachePerfStatsTest { @Test public void getClearsDelegatesToStatistics() { - statistics.incLong(clearsId, Long.MAX_VALUE); + statistics.incLong(regionClearsId, Long.MAX_VALUE); - assertThat(cachePerfStats.getClearCount()).isEqualTo(Long.MAX_VALUE); + assertThat(cachePerfStats.getRegionClearCount()).isEqualTo(Long.MAX_VALUE); } @Test - public void incClearCountIncrementsClears() { - cachePerfStats.incClearCount(); + public void incRegionClearCountIncrementsClears() { + cachePerfStats.incRegionClearCount(); - assertThat(statistics.getLong(clearsId)).isEqualTo(1L); + assertThat(statistics.getLong(regionClearsId)).isEqualTo(1L); + } + + @Test + public void incBucketClearCountIncrementsClears() { + cachePerfStats.incBucketClearCount(); + + assertThat(statistics.getLong(bucketClearsId)).isEqualTo(1L); + } + + @Test + public void incPartitionedRegionClearLocalDurationIncrementsPartitionedRegionClearLocalDuration() { + cachePerfStats.incPartitionedRegionClearLocalDuration(100L); + + assertThat(statistics.getLong(partitionedRegionClearLocalDurationId)).isEqualTo(100L); + } + + @Test + public void incPartitionedRegionClearTotalDurationIncrementsPartitionedRegionClearTotalDuration() { + cachePerfStats.incPartitionedRegionClearTotalDuration(100L); + + assertThat(statistics.getLong(partitionedRegionClearTotalDurationId)).isEqualTo(100L); } /** * Characterization test: {@code clears} currently wraps to negative from max long value. */ @Test - public void clearsWrapsFromMaxLongToNegativeValue() { - statistics.incLong(clearsId, Long.MAX_VALUE); + public void regionClearsWrapsFromMaxLongToNegativeValue() { + statistics.incLong(regionClearsId, Long.MAX_VALUE); + + cachePerfStats.incRegionClearCount(); + + assertThat(cachePerfStats.getRegionClearCount()).isNegative(); + } + + @Test + public void bucketClearsWrapsFromMaxLongToNegativeValue() { + statistics.incLong(bucketClearsId, Long.MAX_VALUE); - cachePerfStats.incClearCount(); + cachePerfStats.incBucketClearCount(); - assertThat(cachePerfStats.getClearCount()).isNegative(); + assertThat(cachePerfStats.getBucketClearCount()).isNegative(); } @Test