[GitHub] [geode] agingade opened a new pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade opened a new pull request #5208:
URL: https://github.com/apache/geode/pull/5208


   Thank you for submitting a contribution to Apache Geode.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [Y] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
   
   - [Y] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
   
   - [Y] Is your initial contribution a single, squashed commit?
   
   - [Y] Does `gradlew build` run cleanly?
   
   - [Y] Have you written or updated unit tests to verify your changes?
   
   - [ NA] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   
   ### Note:
   Please ensure that once the PR is submitted, check Concourse for build issues and
   submit an update to your PR as soon as possible. If you need help, please send an
   email to dev@geode.apache.org.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] agingade merged pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade merged pull request #5208:
URL: https://github.com/apache/geode/pull/5208


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] agingade commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436215854



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();
+    for (int i = 0; i < numBuckets; i++) {
+      BucketRegion bucketRegion = mock(BucketRegion.class);
+      when(bucketRegion.getBucketAdvisor()).thenReturn(bucketAdvisor);
+      when(bucketRegion.size()).thenReturn(1);
+      when(bucketRegion.getId()).thenReturn(i);
+      bucketRegions.add(bucketRegion);
+    }
+
+    when(partitionedRegionDataStore.getAllLocalBucketRegions()).thenReturn(bucketRegions);
+    when(partitionedRegionDataStore.getAllLocalPrimaryBucketRegions()).thenReturn(bucketRegions);
+
+    return bucketRegions;
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsTrueWhenLocked() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(true);
+    partitionedRegionClear.obtainClearLockLocal(internalDistributedMember);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isTrue();
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsFalseWhenMemberNotInTheSystemRequestsLock() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(false);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isFalse();
+  }
+
+  @Test
+  public void acquireDistributedClearLockGetsDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.acquireDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).lock(lockName, -1, -1);
+  }
+
+  @Test
+  public void releaseDistributedClearLockReleasesDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.releaseDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).unlock(lockName);
+  }
+
+  @Test
+  public void obtainLockForClearGetsLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.obtainLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).obtainClearLockLocal(internalDistributedMember);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void releaseLockForClearReleasesLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.releaseLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).releaseClearLockLocal();
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void clearRegionClearsLocalAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+    RegionVersionVector regionVersionVector = mock(RegionVersionVector.class);
+
+    spyPartitionedRegionClear.clearRegion(regionEvent, false, regionVersionVector);
+
+    verify(spyPartitionedRegionClear, times(1)).clearRegionLocal(regionEvent);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterFindingAllPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterRetryForPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(false).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(1)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryThrowsPartitionedRegionPartialClearException() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+    when(retryTimer.overMaximum()).thenReturn(true);
+
+    Throwable thrown = catchThrowable(() -> partitionedRegionClear.waitForPrimary(retryTimer));
+
+    assertThat(thrown)
+        .isInstanceOf(PartitionedRegionPartialClearException.class)
+        .hasMessage(
+            "Unable to find primary bucket region during clear operation for region: prRegion");
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void clearRegionLocalCallsClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+
+    List bucketsCleared = partitionedRegionClear.clearRegionLocal(regionEvent);
+
+    assertThat(bucketsCleared.size()).isEqualTo(buckets.size());
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).clear();
+    }
+  }
+
+  @Test
+  public void clearRegionLocalRetriesClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    when(spyPartitionedRegionClear.getMembershipChange()).thenReturn(true).thenReturn(false);
+
+    List bucketsCleared = spyPartitionedRegionClear.clearRegionLocal(regionEvent);
+
+    int expectedClears = buckets.size() * 2; /* clear is called twice on each bucket */
+    assertThat(bucketsCleared.size()).isEqualTo(expectedClears);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(2)).clear();
+    }
+  }
+
+  @Test
+  public void doAfterClearCallsNotifyClientsWhenClientHaveInterests() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasAnyClientsInterested()).thenReturn(true);
+    FilterProfile filterProfile = mock(FilterProfile.class);
+    FilterRoutingInfo filterRoutingInfo = mock(FilterRoutingInfo.class);
+    when(filterProfile.getFilterRoutingInfoPart1(regionEvent, FilterProfile.NO_PROFILES,
+        Collections.emptySet())).thenReturn(filterRoutingInfo);
+    when(filterProfile.getFilterRoutingInfoPart2(filterRoutingInfo, regionEvent)).thenReturn(
+        filterRoutingInfo);
+    when(partitionedRegion.getFilterProfile()).thenReturn(filterProfile);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(regionEvent, times(1)).setLocalFilterInfo(any());
+    verify(partitionedRegion, times(1)).notifyBridgeClients(regionEvent);
+  }
+
+  @Test
+  public void doAfterClearDispatchesListenerEvents() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasListener()).thenReturn(true);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(partitionedRegion, times(1)).dispatchListenerEvent(
+        EnumListenerEvent.AFTER_REGION_CLEAR, regionEvent);
+  }
+
+  @Test
+  public void obtainClearLockLocalGetsLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isSameAs(member);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void obtainClearLockLocalDoesNotGetLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(false);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalReleasesLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+    partitionedRegionClear.lockForListenerAndClientNotification.setLocked(member);
+
+    partitionedRegionClear.releaseClearLockLocal();
+
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalDoesNotReleaseLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+
+    partitionedRegionClear.obtainClearLockLocal(member);

Review comment:
       Good catch. Thanks.
   Updated the test.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] DonalEvans commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

DonalEvans commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436210405



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();
+    for (int i = 0; i < numBuckets; i++) {
+      BucketRegion bucketRegion = mock(BucketRegion.class);
+      when(bucketRegion.getBucketAdvisor()).thenReturn(bucketAdvisor);
+      when(bucketRegion.size()).thenReturn(1);
+      when(bucketRegion.getId()).thenReturn(i);
+      bucketRegions.add(bucketRegion);
+    }
+
+    when(partitionedRegionDataStore.getAllLocalBucketRegions()).thenReturn(bucketRegions);
+    when(partitionedRegionDataStore.getAllLocalPrimaryBucketRegions()).thenReturn(bucketRegions);
+
+    return bucketRegions;
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsTrueWhenLocked() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(true);
+    partitionedRegionClear.obtainClearLockLocal(internalDistributedMember);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isTrue();
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsFalseWhenMemberNotInTheSystemRequestsLock() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(false);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isFalse();
+  }
+
+  @Test
+  public void acquireDistributedClearLockGetsDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.acquireDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).lock(lockName, -1, -1);
+  }
+
+  @Test
+  public void releaseDistributedClearLockReleasesDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.releaseDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).unlock(lockName);
+  }
+
+  @Test
+  public void obtainLockForClearGetsLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.obtainLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).obtainClearLockLocal(internalDistributedMember);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void releaseLockForClearReleasesLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.releaseLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).releaseClearLockLocal();
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void clearRegionClearsLocalAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+    RegionVersionVector regionVersionVector = mock(RegionVersionVector.class);
+
+    spyPartitionedRegionClear.clearRegion(regionEvent, false, regionVersionVector);
+
+    verify(spyPartitionedRegionClear, times(1)).clearRegionLocal(regionEvent);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterFindingAllPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterRetryForPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(false).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(1)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryThrowsPartitionedRegionPartialClearException() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+    when(retryTimer.overMaximum()).thenReturn(true);
+
+    Throwable thrown = catchThrowable(() -> partitionedRegionClear.waitForPrimary(retryTimer));
+
+    assertThat(thrown)
+        .isInstanceOf(PartitionedRegionPartialClearException.class)
+        .hasMessage(
+            "Unable to find primary bucket region during clear operation for region: prRegion");
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void clearRegionLocalCallsClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+
+    List bucketsCleared = partitionedRegionClear.clearRegionLocal(regionEvent);
+
+    assertThat(bucketsCleared.size()).isEqualTo(buckets.size());
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).clear();
+    }
+  }
+
+  @Test
+  public void clearRegionLocalRetriesClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    when(spyPartitionedRegionClear.getMembershipChange()).thenReturn(true).thenReturn(false);
+
+    List bucketsCleared = spyPartitionedRegionClear.clearRegionLocal(regionEvent);
+
+    int expectedClears = buckets.size() * 2; /* clear is called twice on each bucket */
+    assertThat(bucketsCleared.size()).isEqualTo(expectedClears);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(2)).clear();
+    }
+  }
+
+  @Test
+  public void doAfterClearCallsNotifyClientsWhenClientHaveInterests() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasAnyClientsInterested()).thenReturn(true);
+    FilterProfile filterProfile = mock(FilterProfile.class);
+    FilterRoutingInfo filterRoutingInfo = mock(FilterRoutingInfo.class);
+    when(filterProfile.getFilterRoutingInfoPart1(regionEvent, FilterProfile.NO_PROFILES,
+        Collections.emptySet())).thenReturn(filterRoutingInfo);
+    when(filterProfile.getFilterRoutingInfoPart2(filterRoutingInfo, regionEvent)).thenReturn(
+        filterRoutingInfo);
+    when(partitionedRegion.getFilterProfile()).thenReturn(filterProfile);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(regionEvent, times(1)).setLocalFilterInfo(any());
+    verify(partitionedRegion, times(1)).notifyBridgeClients(regionEvent);
+  }
+
+  @Test
+  public void doAfterClearDispatchesListenerEvents() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasListener()).thenReturn(true);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(partitionedRegion, times(1)).dispatchListenerEvent(
+        EnumListenerEvent.AFTER_REGION_CLEAR, regionEvent);
+  }
+
+  @Test
+  public void obtainClearLockLocalGetsLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isSameAs(member);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void obtainClearLockLocalDoesNotGetLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(false);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalReleasesLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+    partitionedRegionClear.lockForListenerAndClientNotification.setLocked(member);
+
+    partitionedRegionClear.releaseClearLockLocal();
+
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalDoesNotReleaseLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+
+    partitionedRegionClear.obtainClearLockLocal(member);

Review comment:
       I think this should be `releaseClearLockLocal()` instead?

##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();
+    for (int i = 0; i < numBuckets; i++) {
+      BucketRegion bucketRegion = mock(BucketRegion.class);
+      when(bucketRegion.getBucketAdvisor()).thenReturn(bucketAdvisor);
+      when(bucketRegion.size()).thenReturn(1);
+      when(bucketRegion.getId()).thenReturn(i);
+      bucketRegions.add(bucketRegion);
+    }
+
+    when(partitionedRegionDataStore.getAllLocalBucketRegions()).thenReturn(bucketRegions);
+    when(partitionedRegionDataStore.getAllLocalPrimaryBucketRegions()).thenReturn(bucketRegions);
+
+    return bucketRegions;
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsTrueWhenLocked() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(true);
+    partitionedRegionClear.obtainClearLockLocal(internalDistributedMember);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isTrue();
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsFalseWhenMemberNotInTheSystemRequestsLock() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(false);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isFalse();
+  }
+
+  @Test
+  public void acquireDistributedClearLockGetsDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.acquireDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).lock(lockName, -1, -1);
+  }
+
+  @Test
+  public void releaseDistributedClearLockReleasesDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.releaseDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).unlock(lockName);
+  }
+
+  @Test
+  public void obtainLockForClearGetsLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.obtainLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).obtainClearLockLocal(internalDistributedMember);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void releaseLockForClearReleasesLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.releaseLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).releaseClearLockLocal();
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void clearRegionClearsLocalAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+    RegionVersionVector regionVersionVector = mock(RegionVersionVector.class);
+
+    spyPartitionedRegionClear.clearRegion(regionEvent, false, regionVersionVector);
+
+    verify(spyPartitionedRegionClear, times(1)).clearRegionLocal(regionEvent);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterFindingAllPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterRetryForPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(false).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(1)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryThrowsPartitionedRegionPartialClearException() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+    when(retryTimer.overMaximum()).thenReturn(true);
+
+    Throwable thrown = catchThrowable(() -> partitionedRegionClear.waitForPrimary(retryTimer));
+
+    assertThat(thrown)
+        .isInstanceOf(PartitionedRegionPartialClearException.class)
+        .hasMessage(
+            "Unable to find primary bucket region during clear operation for region: prRegion");
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void clearRegionLocalCallsClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+
+    List bucketsCleared = partitionedRegionClear.clearRegionLocal(regionEvent);
+
+    assertThat(bucketsCleared.size()).isEqualTo(buckets.size());
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).clear();
+    }
+  }
+
+  @Test
+  public void clearRegionLocalRetriesClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    when(spyPartitionedRegionClear.getMembershipChange()).thenReturn(true).thenReturn(false);
+
+    List bucketsCleared = spyPartitionedRegionClear.clearRegionLocal(regionEvent);
+
+    int expectedClears = buckets.size() * 2; /* clear is called twice on each bucket */
+    assertThat(bucketsCleared.size()).isEqualTo(expectedClears);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(2)).clear();
+    }
+  }
+
+  @Test
+  public void doAfterClearCallsNotifyClientsWhenClientHaveInterests() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasAnyClientsInterested()).thenReturn(true);
+    FilterProfile filterProfile = mock(FilterProfile.class);
+    FilterRoutingInfo filterRoutingInfo = mock(FilterRoutingInfo.class);
+    when(filterProfile.getFilterRoutingInfoPart1(regionEvent, FilterProfile.NO_PROFILES,
+        Collections.emptySet())).thenReturn(filterRoutingInfo);
+    when(filterProfile.getFilterRoutingInfoPart2(filterRoutingInfo, regionEvent)).thenReturn(
+        filterRoutingInfo);
+    when(partitionedRegion.getFilterProfile()).thenReturn(filterProfile);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(regionEvent, times(1)).setLocalFilterInfo(any());
+    verify(partitionedRegion, times(1)).notifyBridgeClients(regionEvent);
+  }
+
+  @Test
+  public void doAfterClearDispatchesListenerEvents() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasListener()).thenReturn(true);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(partitionedRegion, times(1)).dispatchListenerEvent(
+        EnumListenerEvent.AFTER_REGION_CLEAR, regionEvent);
+  }
+
+  @Test
+  public void obtainClearLockLocalGetsLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isSameAs(member);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void obtainClearLockLocalDoesNotGetLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(false);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalReleasesLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+    partitionedRegionClear.lockForListenerAndClientNotification.setLocked(member);
+
+    partitionedRegionClear.releaseClearLockLocal();
+
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalDoesNotReleaseLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void sendPartitionedRegionClearMessageSendsClearMessageToPRNodes() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> prRoot = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(prRoot);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    RegionAdvisor regionAdvisor = mock(RegionAdvisor.class);
+    Set<InternalDistributedMember> prNodes = new HashSet<>();
+    prNodes.add(member);
+    Node node = mock(Node.class);
+    when(node.getMemberId()).thenReturn(member);
+    Set<Node> configNodes = new HashSet<>();
+    configNodes.add(node);

Review comment:
       Small suggestion; you can instead use `Set<InternalDistributedMember> prNodes = Collections.singleton(member);` and `Set<Node> configNodes = Collections.singleton(node);` to condense the code a little.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] kirklund commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

kirklund commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436075685



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -10269,4 +10269,8 @@ boolean hasAnyClientsInterested() {
     return (getRegionAdvisor().hasPRServerWithInterest()
         || getRegionAdvisor().hasPRServerWithCQs());
   }
+
+  boolean isTransactionDistributed() {

Review comment:
       I hate the idea of adding methods like this to Regions. It has nothing to do with Region. Maybe Transaction or TransactionManager would be a better place for this method.

##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();

Review comment:
       Let's move away from the Jason Penney quirks. We should be declaring vars as the interface instead of the impl:
   ```
   Set<BucketRegion> bucketRegions = new HashSet<>();
   ```

##########
File path: geode-core/src/distributedTest/java/org/apache/geode/cache/ReplicateCacheListenerDistributedTest.java
##########
@@ -179,7 +180,13 @@ public void afterRegionDestroyIsInvokedInEveryMember() {
 
     region.destroyRegion();
 
-    assertThat(sharedCountersRule.getTotal(REGION_DESTROY)).isEqualTo(expectedRegionDestroys());
+    if (region instanceof PartitionedRegion) {

Review comment:
       You typically want to avoid if-else blocks like this, and separate it into two different tests. One for PR and one for Replicate.
   
   The test class is ReplicateCacheListenerDistributedTest. I can't find any PartitionedRegions being created in this test. Does this belong in PRCacheListenerDistributedTest instead?

##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();

Review comment:
       We should be declaring vars as the interface instead of the impl:
   ```
   Set<BucketRegion> bucketRegions = new HashSet<>();
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] agingade commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r435605934



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,594 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+  int numBuckets = 2;
+  PartitionedRegionClear partitionedRegionClear;
+  DistributionManager distributionManager;
+  PartitionedRegion partitionedRegion;

Review comment:
       Addressed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] agingade commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436215906



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();
+    for (int i = 0; i < numBuckets; i++) {
+      BucketRegion bucketRegion = mock(BucketRegion.class);
+      when(bucketRegion.getBucketAdvisor()).thenReturn(bucketAdvisor);
+      when(bucketRegion.size()).thenReturn(1);
+      when(bucketRegion.getId()).thenReturn(i);
+      bucketRegions.add(bucketRegion);
+    }
+
+    when(partitionedRegionDataStore.getAllLocalBucketRegions()).thenReturn(bucketRegions);
+    when(partitionedRegionDataStore.getAllLocalPrimaryBucketRegions()).thenReturn(bucketRegions);
+
+    return bucketRegions;
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsTrueWhenLocked() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(true);
+    partitionedRegionClear.obtainClearLockLocal(internalDistributedMember);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isTrue();
+  }
+
+  @Test
+  public void isLockedForListenerAndClientNotificationReturnsFalseWhenMemberNotInTheSystemRequestsLock() {
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(internalDistributedMember)).thenReturn(false);
+
+    assertThat(partitionedRegionClear.isLockedForListenerAndClientNotification()).isFalse();
+  }
+
+  @Test
+  public void acquireDistributedClearLockGetsDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.acquireDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).lock(lockName, -1, -1);
+  }
+
+  @Test
+  public void releaseDistributedClearLockReleasesDistributedLock() {
+    DistributedLockService distributedLockService = mock(DistributedLockService.class);
+    String lockName = PartitionedRegionClear.CLEAR_OPERATION + partitionedRegion.getName();
+    when(partitionedRegion.getPartitionedRegionLockService()).thenReturn(distributedLockService);
+
+    partitionedRegionClear.releaseDistributedClearLock(lockName);
+
+    verify(distributedLockService, times(1)).unlock(lockName);
+  }
+
+  @Test
+  public void obtainLockForClearGetsLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.obtainLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).obtainClearLockLocal(internalDistributedMember);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_LOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void releaseLockForClearReleasesLocalLockAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+
+    spyPartitionedRegionClear.releaseLockForClear(regionEvent);
+
+    verify(spyPartitionedRegionClear, times(1)).releaseClearLockLocal();
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_UNLOCK_FOR_PR_CLEAR);
+  }
+
+  @Test
+  public void clearRegionClearsLocalAndSendsMessageForRemote() throws Exception {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> region = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(region);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    doReturn(Collections.EMPTY_LIST).when(spyPartitionedRegionClear)
+        .attemptToSendPartitionedRegionClearMessage(regionEvent,
+            PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+    InternalDistributedMember internalDistributedMember = mock(InternalDistributedMember.class);
+    when(distributionManager.getId()).thenReturn(internalDistributedMember);
+    RegionVersionVector regionVersionVector = mock(RegionVersionVector.class);
+
+    spyPartitionedRegionClear.clearRegion(regionEvent, false, regionVersionVector);
+
+    verify(spyPartitionedRegionClear, times(1)).clearRegionLocal(regionEvent);
+    verify(spyPartitionedRegionClear, times(1)).sendPartitionedRegionClearMessage(regionEvent,
+        PartitionedRegionClearMessage.OperationType.OP_PR_CLEAR);
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterFindingAllPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryReturnsAfterRetryForPrimary() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(false).thenReturn(true);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+
+    partitionedRegionClear.waitForPrimary(retryTimer);
+
+    verify(retryTimer, times(1)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void waitForPrimaryThrowsPartitionedRegionPartialClearException() {
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegion.RetryTimeKeeper retryTimer = mock(PartitionedRegion.RetryTimeKeeper.class);
+    when(retryTimer.overMaximum()).thenReturn(true);
+
+    Throwable thrown = catchThrowable(() -> partitionedRegionClear.waitForPrimary(retryTimer));
+
+    assertThat(thrown)
+        .isInstanceOf(PartitionedRegionPartialClearException.class)
+        .hasMessage(
+            "Unable to find primary bucket region during clear operation for region: prRegion");
+    verify(retryTimer, times(0)).waitForBucketsRecovery();
+  }
+
+  @Test
+  public void clearRegionLocalCallsClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+
+    List bucketsCleared = partitionedRegionClear.clearRegionLocal(regionEvent);
+
+    assertThat(bucketsCleared.size()).isEqualTo(buckets.size());
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).clear();
+    }
+  }
+
+  @Test
+  public void clearRegionLocalRetriesClearOnLocalPrimaryBucketRegions() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    doNothing().when(partitionedRegionDataStore).lockBucketCreationForRegionClear();
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    PartitionedRegionClear spyPartitionedRegionClear = spy(partitionedRegionClear);
+    when(spyPartitionedRegionClear.getMembershipChange()).thenReturn(true).thenReturn(false);
+
+    List bucketsCleared = spyPartitionedRegionClear.clearRegionLocal(regionEvent);
+
+    int expectedClears = buckets.size() * 2; /* clear is called twice on each bucket */
+    assertThat(bucketsCleared.size()).isEqualTo(expectedClears);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(2)).clear();
+    }
+  }
+
+  @Test
+  public void doAfterClearCallsNotifyClientsWhenClientHaveInterests() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasAnyClientsInterested()).thenReturn(true);
+    FilterProfile filterProfile = mock(FilterProfile.class);
+    FilterRoutingInfo filterRoutingInfo = mock(FilterRoutingInfo.class);
+    when(filterProfile.getFilterRoutingInfoPart1(regionEvent, FilterProfile.NO_PROFILES,
+        Collections.emptySet())).thenReturn(filterRoutingInfo);
+    when(filterProfile.getFilterRoutingInfoPart2(filterRoutingInfo, regionEvent)).thenReturn(
+        filterRoutingInfo);
+    when(partitionedRegion.getFilterProfile()).thenReturn(filterProfile);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(regionEvent, times(1)).setLocalFilterInfo(any());
+    verify(partitionedRegion, times(1)).notifyBridgeClients(regionEvent);
+  }
+
+  @Test
+  public void doAfterClearDispatchesListenerEvents() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(partitionedRegion.hasListener()).thenReturn(true);
+
+    partitionedRegionClear.doAfterClear(regionEvent);
+
+    verify(partitionedRegion, times(1)).dispatchListenerEvent(
+        EnumListenerEvent.AFTER_REGION_CLEAR, regionEvent);
+  }
+
+  @Test
+  public void obtainClearLockLocalGetsLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isSameAs(member);
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void obtainClearLockLocalDoesNotGetLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(false);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).lockLocallyForClear(partitionedRegion.getDistributionManager(),
+          partitionedRegion.getMyId(), null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalReleasesLockOnPrimaryBuckets() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    when(distributionManager.isCurrentMember(member)).thenReturn(true);
+    partitionedRegionClear.lockForListenerAndClientNotification.setLocked(member);
+
+    partitionedRegionClear.releaseClearLockLocal();
+
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(1)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void releaseClearLockLocalDoesNotReleaseLocksOnPrimaryBucketsWhenMemberIsNotCurrent() {
+    BucketAdvisor bucketAdvisor = mock(BucketAdvisor.class);
+    when(bucketAdvisor.hasPrimary()).thenReturn(true);
+    PartitionedRegionDataStore partitionedRegionDataStore = mock(PartitionedRegionDataStore.class);
+    Set<BucketRegion> buckets = setupBucketRegions(partitionedRegionDataStore, bucketAdvisor);
+    when(partitionedRegion.getDataStore()).thenReturn(partitionedRegionDataStore);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+
+    partitionedRegionClear.obtainClearLockLocal(member);
+
+    assertThat(partitionedRegionClear.lockForListenerAndClientNotification.getLockRequester())
+        .isNull();
+    for (BucketRegion bucketRegion : buckets) {
+      verify(bucketRegion, times(0)).releaseLockLocallyForClear(null);
+    }
+  }
+
+  @Test
+  public void sendPartitionedRegionClearMessageSendsClearMessageToPRNodes() {
+    RegionEventImpl regionEvent = mock(RegionEventImpl.class);
+    when(regionEvent.clone()).thenReturn(mock(RegionEventImpl.class));
+    Region<String, PartitionRegionConfig> prRoot = mock(Region.class);
+    when(partitionedRegion.getPRRoot()).thenReturn(prRoot);
+    InternalDistributedMember member = mock(InternalDistributedMember.class);
+    RegionAdvisor regionAdvisor = mock(RegionAdvisor.class);
+    Set<InternalDistributedMember> prNodes = new HashSet<>();
+    prNodes.add(member);
+    Node node = mock(Node.class);
+    when(node.getMemberId()).thenReturn(member);
+    Set<Node> configNodes = new HashSet<>();
+    configNodes.add(node);

Review comment:
       Good suggestion. Looks little tidy.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] agingade commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

agingade commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436214984



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -10269,4 +10269,8 @@ boolean hasAnyClientsInterested() {
     return (getRegionAdvisor().hasPRServerWithInterest()
         || getRegionAdvisor().hasPRServerWithCQs());
   }
+
+  boolean isTransactionDistributed() {

Review comment:
       Removed the change.

##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+  private PartitionedRegionClear partitionedRegionClear;
+  private DistributionManager distributionManager;
+  private PartitionedRegion partitionedRegion;
+
+  @Before
+  public void setUp() {
+
+    partitionedRegion = mock(PartitionedRegion.class);
+    distributionManager = mock(DistributionManager.class);
+
+    when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+    when(partitionedRegion.getName()).thenReturn("prRegion");
+
+    partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+  }
+
+  private Set<BucketRegion> setupBucketRegions(
+      PartitionedRegionDataStore partitionedRegionDataStore,
+      BucketAdvisor bucketAdvisor) {
+    final int numBuckets = 2;
+    HashSet<BucketRegion> bucketRegions = new HashSet<>();

Review comment:
       Modified in the latest checkin

##########
File path: geode-core/src/distributedTest/java/org/apache/geode/cache/ReplicateCacheListenerDistributedTest.java
##########
@@ -179,7 +180,13 @@ public void afterRegionDestroyIsInvokedInEveryMember() {
 
     region.destroyRegion();
 
-    assertThat(sharedCountersRule.getTotal(REGION_DESTROY)).isEqualTo(expectedRegionDestroys());
+    if (region instanceof PartitionedRegion) {

Review comment:
       Separated the PRCacheListenerDistributedTest




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] mhansonp commented on a change in pull request #5208: GEODE-8173: Add unit test (coverage) for PartitionedRegionClear class.

[GitBox <gi...@apache.org>]

mhansonp commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r435527266



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,594 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+  int numBuckets = 2;
+  PartitionedRegionClear partitionedRegionClear;
+  DistributionManager distributionManager;
+  PartitionedRegion partitionedRegion;

Review comment:
       Probably should use proper types for the Regions and Sets etc rather than raw types.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org