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);