[GitHub] [geode] bschuchardt commented on a change in pull request #5236: GEODE-8241: Locator observes locator-wait-time

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

bschuchardt commented on a change in pull request #5236:
URL: https://github.com/apache/geode/pull/5236#discussion_r438832696



##########
File path: geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/MembershipIntegrationTest.java
##########
@@ -172,6 +179,91 @@ public void secondMembershipCanJoinUsingTheSecondLocatorToStart()
     stop(locator2, locator1);
   }
 
+  @Test
+  public void secondMembershipPausesForLocatorWaitTime()
+      throws IOException, MemberStartupException {
+
+    /*
+     * Start a locator for the coordinator (membership) so we have a port for it.
+     *
+     * Its locator-wait-time is set to 0 so it eventually (soon after membership is started) forms a
+     * distributed system and becomes a coordinator.
+     */
+
+    final MembershipLocator<MemberIdentifier> coordinatorLocator = createLocator(0);
+    coordinatorLocator.start();
+    final int coordinatorLocatorPort = coordinatorLocator.getPort();
+
+    final Membership<MemberIdentifier> coordinatorMembership =
+        createMembership(coordinatorLocator, coordinatorLocatorPort);
+
+    /*
+     * We have not even started the membership yet — connection attempts will certainly fail until
+     * we do. This is a bit like the locator (host) not being present in DNS (yet).
+     */
+
+    /*
+     * Start a second locator and membership trying to join via the coordinator (membership) that
+     * hasn't yet started behind the port.
+     *
+     * Set its locator-wait-time so it'll not become a coordinator right away, allowing time for the
+     * other member to start and become a coordinator.
+     *
+     * Calculate the locator-wait-time to be greater than the minimum wait time for connecting to a
+     * locator.
+     */
+
+    final MembershipLocator<MemberIdentifier> lateJoiningLocator = createLocator(0);
+    lateJoiningLocator.start();
+    final int lateJoiningLocatorPort = lateJoiningLocator.getPort();
+
+    final int[] lateJoiningMembershipLocatorPorts =
+        new int[] {coordinatorLocatorPort, lateJoiningLocatorPort};
+
+    final Duration minimumJoinWaitTime = Duration
+        .ofMillis(2_000) // expected amount of sleep time per loop in GMSJoinLeave.join()
+        .multipliedBy(lateJoiningMembershipLocatorPorts.length * 2); // expected number of loops
+    final int locatorWaitTime = (int) (3 * minimumJoinWaitTime.getSeconds());
+
+    final MembershipConfig lateJoiningMembershipConfig =
+        createMembershipConfig(true, locatorWaitTime, lateJoiningMembershipLocatorPorts);
+    final Membership<MemberIdentifier> lateJoiningMembership =
+        createMembership(lateJoiningMembershipConfig, lateJoiningLocator);
+
+    CompletableFuture<Void> lateJoiningMembershipStartup = executorServiceRule.runAsync(() -> {
+      try {
+        start(lateJoiningMembership);
+      } catch (MemberStartupException e) {
+        e.printStackTrace();

Review comment:
       There are a couple of these auto-generated try/catch statements in the test.  If they're failure conditions shouldn't the test fail?  If they're not, what good are these stack traces?




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