[GitHub] [geode] kirklund commented on a change in pull request #7144: GEODE-9853: get all members hosting bucket

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

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



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java
##########
@@ -351,6 +351,23 @@ void removeBucket() {
     setHosting(false);
   }
 
+  /**
+   * Return all members hosting this bucket
+   *
+   * @return members hosting bucket
+   */
+  public Set<InternalDistributedMember> getAllMembers() {

Review comment:
       `getAllMembers` is kind of ambiguous. I recommend something more explicit such as `getAllHostingMembers`.

##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderOperationsDUnitTest.java
##########
@@ -1185,6 +1189,89 @@ public void testParallelGWSenderUpdateFiltersWhilePuttingOnOneDispatcThread() th
 
   }
 
+  /**
+   * Put entries in region after gateway sander is stopped. Count number of PQRM messages sent.
+   */
+  @Test
+  public void testDroppedEventsSignalizationToSecundaryQueueWhileSenderStopped() throws Exception {
+    Integer[] locatorPorts = createLNAndNYLocators();
+    Integer lnPort = locatorPorts[0];
+    Integer nyPort = locatorPorts[1];

Review comment:
       It's better to use primitives such as `int` unless you really need wrappers like `Integer`. Long, long ago, these tests needed to use wrappers but that's no longer true.

##########
File path: geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderOperationsDUnitTest.java
##########
@@ -22,6 +22,7 @@
 import static org.apache.geode.test.dunit.IgnoredException.addIgnoredException;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.junit.Assert.assertEquals;

Review comment:
       Please stick to AssertJ and use `assertThat(a).isEqualTo(b)` instead.




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