You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/05/10 01:15:07 UTC

[GitHub] [geode] gesterzhou opened a new pull request, #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

gesterzhou opened a new pull request, #7670:
URL: https://github.com/apache/geode/pull/7670

   <!-- 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:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   - [ ] Does `gradlew build` run cleanly?
   
   - [ ] Have you written or updated unit tests to verify your changes?
   
   - [ ] 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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] dschneider-pivotal commented on a diff in pull request #7670: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
dschneider-pivotal commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r874251073


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();

Review Comment:
   no need to allocate a real HashSet for foundIds if "isPersistentRegion". For persistent regions this could just be the canonical Collections.emptySet().



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1100,6 +1107,13 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         }
       }
     }
+    if (!departedMemberSet.isEmpty() && foundIds.size() > 0) {

Review Comment:
   instead of size() > 0 use !isEmpty()
   also I don't see any need to check departedMemberSet here if you make the other changes. 



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    Set<VersionSource> departedMemberSet = receivedRVV.getDepartedMembersSet();
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !departedMemberSet.isEmpty()) {

Review Comment:
   since foundIds is only added to if '!isPersistentRegion' it seems like this OR should be "|| (!isPersistentRegion && !departedMemberSet.isEmpty())" .
   Otherwise we go into this "if" for a persistentRegion if departedMemberSet is not empty.



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    Set<VersionSource> departedMemberSet = receivedRVV.getDepartedMembersSet();
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !departedMemberSet.isEmpty()) {
       // only search for unfinished keys when localRVV has something newer

Review Comment:
   this comment needs to be updated since you now also do the search for non-persistent with departedMembers



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1077,7 +1081,10 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         if (id == null) {
           id = myId;
         }
-        if (!remoteRVV.contains(id, stamp.getRegionVersion())) {
+        if (!isPersistentRegion) {
+          foundIds.add(id);
+        }
+        if (isPersistentRegion && !remoteRVV.contains(id, stamp.getRegionVersion())) {

Review Comment:
   instead of testing "isPersistentRegion" in this "if" just add an "else" before the "if".



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r875381346


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1100,6 +1107,13 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         }
       }
     }
+    if (!departedMemberSet.isEmpty() && foundIds.size() > 0) {

Review Comment:
   !departedMemberSet.isEmpty means this is non persistent region and there's something to remove to RVV's member map. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869774427


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1077,7 +1081,8 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         if (id == null) {
           id = myId;
         }
-        if (!remoteRVV.contains(id, stamp.getRegionVersion())) {
+        foundIds.add(id);

Review Comment:
   it won't hurt to add id into foundIds. I will keep it the same as GII provider part. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] nabarunnag commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
nabarunnag commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869687159


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
+    foundIds.add(region.getVersionMember());
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !remoteRVV.getDepartedMembersSet().isEmpty()) {

Review Comment:
   It will hit the condition that Jianxia mentions below and the id may be added
   



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
+    foundIds.add(region.getVersionMember());
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !remoteRVV.getDepartedMembersSet().isEmpty()) {

Review Comment:
   Previously this blocked is accessed by only persistent region. But now it can be accessed by non-persistent region when the second condition is true.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r875385351


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    Set<VersionSource> departedMemberSet = receivedRVV.getDepartedMembersSet();
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !departedMemberSet.isEmpty()) {

Review Comment:
   departedMemberSet will always be empty for persistentRegion



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou merged pull request #7670: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou merged PR #7670:
URL: https://github.com/apache/geode/pull/7670


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] jchen21 commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
jchen21 commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869675129


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1077,7 +1081,8 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         if (id == null) {
           id = myId;
         }
-        if (!remoteRVV.contains(id, stamp.getRegionVersion())) {
+        foundIds.add(id);

Review Comment:
   This will add `id` to `foundIds`, even for the persistent case. However, according to the description of GEODE-10290, it is for the non-persistent case:
   >In non-persistent but concurrent-check enabled case, members departed will be marked. They should be removed from RVV during GII to prevent memberToVersion in RVV grows bigger and bigger.
   
   So I am not sure if we should add a `if` check here. e.g. `if (!isPersistentRegion)` .



##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1100,6 +1105,12 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         }
       }
     }
+    if (foundIds.size() > 0) {

Review Comment:
   `foundIds` always has at least one element. Because of line 1067 `foundIds.add(region.getVersionMember());`.  So this `if ` check is not necessary. 
   
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869774427


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1077,7 +1081,8 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         if (id == null) {
           id = myId;
         }
-        if (!remoteRVV.contains(id, stamp.getRegionVersion())) {
+        foundIds.add(id);

Review Comment:
   fixed



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869774525


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1100,6 +1105,12 @@ protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
         }
       }
     }
+    if (foundIds.size() > 0) {

Review Comment:
   fixed.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] gesterzhou commented on a diff in pull request #7670: [Don't review]: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
gesterzhou commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869775615


##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1052,19 +1052,23 @@ protected RegionVersionVector getRVVFromProvider(final ClusterDistributionManage
   /**
    * Compare the received RVV with local RVV and return a set of keys for unfinished operations.
    *
-   * @param remoteRVV RVV from provider
+   * @param remoteRVV RVV from provider to be filled with unfinished operations
    * @param localRVV RVV recovered from disk
+   * @param receivedRVV original RVV from provider to remove departed members
    * @return set for keys of unfinished operations.
    */
   protected Set<Object> processReceivedRVV(RegionVersionVector remoteRVV,
-      RegionVersionVector localRVV) {
+      RegionVersionVector localRVV, RegionVersionVector receivedRVV) {
     if (remoteRVV == null) {
       return null;
     }
     // calculate keys for unfinished ops
+    Set<VersionSource> foundIds = new HashSet<>();
+    foundIds.add(region.getVersionMember());
     HashSet<Object> keys = new HashSet<>();
-    if (region.getDataPolicy().withPersistence()
-        && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV)) {
+    boolean isPersistentRegion = region.getDataPolicy().withPersistence();
+    if ((isPersistentRegion && localRVV.isNewerThanOrCanFillExceptionsFor(remoteRVV))
+        || !remoteRVV.getDepartedMembersSet().isEmpty()) {

Review Comment:
   fixed. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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


[GitHub] [geode] jchen21 commented on a diff in pull request #7670: GEODE-10290: GII requester should remove departed members

Posted by GitBox <gi...@apache.org>.
jchen21 commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r876224536


##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionRestartRebalanceDUnitTest.java:
##########
@@ -0,0 +1,133 @@
+/*
+ * 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.apache.geode.test.dunit.VM.getVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.Serializable;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.control.RebalanceOperation;
+import org.apache.geode.cache.control.RebalanceResults;
+import org.apache.geode.internal.cache.versions.VersionSource;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+
+public class PartitionedRegionRestartRebalanceDUnitTest implements Serializable {
+  private static final int REDUNDANT_COPIES = 2;
+  private static final int TOTAL_NUM_BUCKETS = 12;
+  private static final Logger logger = LogManager.getLogger();
+
+  private String REGION_NAME = getClass().getSimpleName();;
+  private VM[] datastores;
+
+  @Rule
+  public DistributedRule distributedRule = new DistributedRule();
+
+  @Rule
+  public CacheRule cacheRule = new CacheRule();
+
+  @Before
+  public void setUp() throws Exception {
+    datastores = new VM[4];
+    for (int i = 0; i < datastores.length; i++) {
+      datastores[i] = getVM(i);
+      datastores[i].invoke(() -> cacheRule.createCache());
+      datastores[i].invoke(() -> createRegion());
+    }
+    datastores[0].invoke(() -> feedData());
+  }
+
+  private void createRegion() {
+    PartitionAttributesFactory<String, Integer> paf = new PartitionAttributesFactory();
+    paf.setRedundantCopies(REDUNDANT_COPIES);
+    paf.setTotalNumBuckets(TOTAL_NUM_BUCKETS);
+
+    RegionFactory<String, Integer> rf = cacheRule.getCache().createRegionFactory();
+    rf.setDataPolicy(DataPolicy.PARTITION);
+    rf.setPartitionAttributes(paf.create());
+    LocalRegion region = (LocalRegion) rf.create(REGION_NAME);
+  }
+
+  private void feedData() throws InterruptedException {
+    PartitionedRegion pr = (PartitionedRegion) cacheRule.getCache().getRegion(REGION_NAME);
+    for (int i = 0; i < TOTAL_NUM_BUCKETS * 2; i++) {
+      pr.put(i, "VALUE-" + i);
+      if (i < TOTAL_NUM_BUCKETS) {
+        pr.destroy(i);
+      }
+    }
+    cacheRule.getCache().getTombstoneService().forceBatchExpirationForTests(TOTAL_NUM_BUCKETS);
+  }
+
+  private void rebalance() throws InterruptedException {
+    RebalanceOperation op =
+        cacheRule.getCache().getResourceManager().createRebalanceFactory().start();
+    RebalanceResults results = op.getResults();
+    logger.info("Rebalance total time is " + results.getTotalTime());
+  }
+
+  private void verify() {
+    PartitionedRegion pr = (PartitionedRegion) cacheRule.getCache().getRegion(REGION_NAME);
+    for (BucketRegion br : pr.getDataStore().getAllLocalBucketRegions()) {
+      Set<VersionSource> departedMemberSet = br.getVersionVector().getDepartedMembersSet();
+      for (Object key : br.getRegionKeysForIteration()) {
+        RegionEntry entry = br.getRegionEntry(key);
+        departedMemberSet.remove(entry.getVersionStamp().getMemberID());
+        if (departedMemberSet.isEmpty()) {
+          break;
+        }
+      }
+      Map map = br.getVersionVector().getMemberToVersion();
+      for (Object key : br.getVersionVector().getMemberToVersion().keySet()) {
+        logger.info(br.getFullPath() + ":" + key + ":"
+            + br.getVersionVector().getMemberToVersion().get(key));
+      }
+      // The test proved that departedMemberSet is not growing
+      assertThat(departedMemberSet.size()).isLessThanOrEqualTo(datastores.length);

Review Comment:
   `departedMemberSet` is a set, which does not allow duplicate element. The maximal number of departed member is `datastores`' length. Therefore `departedMemberSet` is always less than or equal to `datastores.length`. Better find other assertion I think.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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