This is an automated email from the ASF dual-hosted git repository.
mivanac pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 1b4b60c GEODE-7963: solution for faulty bucket metrics (#5000)
1b4b60c is described below
commit 1b4b60ca66867a995a593eb0727404e0d89ab9c9
Author: Mario Ivanac <48509724+miva...@users.noreply.github.com>
AuthorDate: Sun May 10 10:45:42 2020 +0200
GEODE-7963: solution for faulty bucket metrics (#5000)
* GEODE-7963: solution for faulty bucket metrics
* GEODE-7963: added test to reproduce fault
* GEODE-7963: added UT
* GEODE-7963: update after comments
* GEODE-7963: small updates
---
.../management/MemberMXBeanDistributedTest.java | 129 +++++++++++++++++++++
.../geode/internal/cache/GemFireCacheImpl.java | 3 +-
.../geode/internal/cache/InternalRegion.java | 4 +
.../apache/geode/internal/cache/LocalRegion.java | 10 ++
.../internal/cache/PRHARedundancyProvider.java | 5 +
.../geode/internal/cache/PartitionedRegion.java | 22 +++-
.../internal/cache/PartitionedRegionTest.java | 24 ++++
7 files changed, 195 insertions(+), 2 deletions(-)
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/MemberMXBeanDistributedTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/MemberMXBeanDistributedTest.java
new file mode 100644
index 0000000..55a82a2
--- /dev/null
+++
b/geode-core/src/distributedTest/java/org/apache/geode/management/MemberMXBeanDistributedTest.java
@@ -0,0 +1,129 @@
+/*
+ * 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.management;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import javax.management.ObjectName;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.partition.PartitionRegionHelper;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.GfshTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@Category({GfshTest.class})
+public class MemberMXBeanDistributedTest implements
+ Serializable {
+
+ private static MemberVM locator;
+ private static MemberVM server1;
+ private static MemberVM server2;
+ private static MemberVM server3;
+ private static MemberVM server4;
+
+ @ClassRule
+ public static ClusterStartupRule lsRule = new ClusterStartupRule();
+
+ @ClassRule
+ public static GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public TestName testName = new SerializableTestName();
+
+ @BeforeClass
+ public static void before() throws Exception {
+ locator = lsRule.startLocatorVM(0);
+ server1 = lsRule.startServerVM(1, "", locator.getPort());
+ server2 = lsRule.startServerVM(2, "", locator.getPort());
+ server3 = lsRule.startServerVM(3, "", locator.getPort());
+ server4 = lsRule.startServerVM(4, "", locator.getPort());
+
+ gfsh.connectAndVerify(locator);
+ }
+
+ @Test
+ public void testBucketCount() {
+ String regionName = "testCreateRegion";
+
+ gfsh.executeAndAssertThat("create region"
+ + " --name=" + regionName
+ + " --type=PARTITION_PERSISTENT"
+ + " --total-num-buckets=1000").statusIsSuccess();
+
+ server1.invoke(() -> createBuckets(regionName));
+ server2.invoke(() -> createBuckets(regionName));
+ server3.invoke(() -> createBuckets(regionName));
+ server4.invoke(() -> createBuckets(regionName));
+
+ await().untilAsserted(() -> {
+ final int sumOfBuckets = server1.invoke(() -> getBucketsInitialized()) +
+ server2.invoke(() -> getBucketsInitialized()) +
+ server3.invoke(() -> getBucketsInitialized()) +
+ server4.invoke(() -> getBucketsInitialized());
+ assertEquals("Expected bucket count is 1000, and actual count is " +
sumOfBuckets,
+ sumOfBuckets, 1000);
+ });
+
+ for (int i = 1; i < 4; i++) {
+ gfsh.executeAndAssertThat("create region"
+ + " --name=" + regionName + i
+ + " --type=PARTITION_PERSISTENT"
+ + " --total-num-buckets=1000"
+ + " --colocated-with=" + regionName).statusIsSuccess();
+ }
+
+ await().untilAsserted(() -> {
+ final int sumOfBuckets = server1.invoke(() -> getBucketsInitialized()) +
+ server2.invoke(() -> getBucketsInitialized()) +
+ server3.invoke(() -> getBucketsInitialized()) +
+ server4.invoke(() -> getBucketsInitialized());
+ assertEquals("Expected bucket count is 4000, and actual count is " +
sumOfBuckets,
+ sumOfBuckets, 4000);
+ });
+
+ }
+
+ private int getBucketsInitialized() {
+ Cache cache = ClusterStartupRule.getCache();
+
+ DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
+ ManagementService mgmtService =
ManagementService.getManagementService(cache);
+ ObjectName memberMBeanName = mgmtService.getMemberMBeanName(member);
+ MemberMXBean memberMXBean = mgmtService.getMBeanInstance(memberMBeanName,
MemberMXBean.class);
+
+ return memberMXBean.getTotalBucketCount();
+ }
+
+ private void createBuckets(String regionName) {
+ Cache cache = ClusterStartupRule.getCache();
+
PartitionRegionHelper.assignBucketsToPartitions(cache.getRegion(regionName));
+ }
+
+}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index caa2532..22a3572 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -3058,8 +3058,9 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
invokeRegionAfter(region);
// Putting the callback here to avoid creating RegionMBean in case of
Exception
- if (!region.isInternalRegion()) {
+ if (!region.isRegionCreateNotified() && !region.isInternalRegion()) {
system.handleResourceEvent(ResourceEvent.REGION_CREATE, region);
+ region.setRegionCreateNotified(true);
}
return cast(region);
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
index 097aace..4ae752b 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
@@ -459,4 +459,8 @@ public interface InternalRegion extends Region,
HasCachePerfStats, RegionEntryCo
* @return true if synchronization should be attempted
*/
boolean shouldSyncForCrashedMember(InternalDistributedMember id);
+
+ boolean isRegionCreateNotified();
+
+ void setRegionCreateNotified(boolean notified);
}
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 48aeefe..11b5c9b 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
@@ -11047,6 +11047,16 @@ public class LocalRegion extends AbstractRegion
implements LoaderHelperFactory,
KEYS, VALUES, ENTRIES
}
+ @Override
+ public boolean isRegionCreateNotified() {
+ return false;
+ }
+
+ @Override
+ public void setRegionCreateNotified(boolean notified) {
+ // do nothing
+ }
+
/**
* Used by {@link #foreachRegionEntry}.
*
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
index f18cb49..e07ab55 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
@@ -1657,6 +1657,11 @@ public class PRHARedundancyProvider {
for (ProxyBucketRegion proxyBucket : bucketsHostedLocally) {
proxyBucket.waitForPrimaryPersistentRecovery();
}
+
+ if (!partitionedRegion.isInternalRegion() &&
!bucketsNotHostedLocally.isEmpty()) {
+ partitionedRegion.notifyRegionCreated();
+ }
+
for (ProxyBucketRegion proxyBucket : bucketsNotHostedLocally) {
proxyBucket.recoverFromDiskRecursively();
}
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 9d7ad5e..d4bbf10 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
@@ -154,6 +154,7 @@ import
org.apache.geode.distributed.internal.OperationExecutors;
import org.apache.geode.distributed.internal.ProfileListener;
import org.apache.geode.distributed.internal.ReplyException;
import org.apache.geode.distributed.internal.ReplyProcessor21;
+import org.apache.geode.distributed.internal.ResourceEvent;
import org.apache.geode.distributed.internal.locks.DLockRemoteToken;
import org.apache.geode.distributed.internal.locks.DLockService;
import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
@@ -744,6 +745,8 @@ public class PartitionedRegion extends LocalRegion
private final PartitionedRegionRedundancyTracker redundancyTracker;
+ private boolean regionCreationNotified;
+
/**
* Constructor for a PartitionedRegion. This has an accessor (Region API)
functionality and
* contains a datastore for actual storage. An accessor can act as a local
cache by having a local
@@ -856,7 +859,7 @@ public class PartitionedRegion extends LocalRegion
this.isShadowPR = true;
this.parallelGatewaySender =
internalRegionArgs.getParallelGatewaySender();
}
-
+ this.regionCreationNotified = false;
/*
* Start persistent profile logging if we are a persistent region.
@@ -10188,4 +10191,21 @@ public class PartitionedRegion extends LocalRegion
public SenderIdMonitor getSenderIdMonitor() {
return senderIdMonitor;
}
+
+ @Override
+ public boolean isRegionCreateNotified() {
+ return this.regionCreationNotified;
+ }
+
+ @Override
+ public void setRegionCreateNotified(boolean notified) {
+ this.regionCreationNotified = notified;
+ };
+
+ void notifyRegionCreated() {
+ if (regionCreationNotified)
+ return;
+ this.getSystem().handleResourceEvent(ResourceEvent.REGION_CREATE, this);
+ this.regionCreationNotified = true;
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTest.java
index ae580e3..742db8a 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTest.java
@@ -622,6 +622,30 @@ public class PartitionedRegionTest {
assertThat(failures.contains(1)).isFalse();
}
+ @Test
+ public void testGetRegionCreateNotification() {
+ partitionedRegion = new PartitionedRegion("region",
attributesFactory.create(), null, cache,
+ mock(InternalRegionArguments.class), disabledClock(),
ColocationLoggerFactory.create());
+
+ assertThat(partitionedRegion.isRegionCreateNotified()).isFalse();
+
+ partitionedRegion.setRegionCreateNotified(true);
+
+ assertThat(partitionedRegion.isRegionCreateNotified()).isTrue();
+ }
+
+ @Test
+ public void testNotifyRegionCreated() {
+ partitionedRegion = new PartitionedRegion("region",
attributesFactory.create(), null, cache,
+ mock(InternalRegionArguments.class), disabledClock(),
ColocationLoggerFactory.create());
+
+ assertThat(partitionedRegion.isRegionCreateNotified()).isFalse();
+
+ partitionedRegion.notifyRegionCreated();
+
+ assertThat(partitionedRegion.isRegionCreateNotified()).isTrue();
+ }
+
private static <K> Set<K> asSet(K... values) {
Set<K> set = new HashSet<>();
Collections.addAll(set, values);
