kirklund commented on code in PR #7515:
URL: https://github.com/apache/geode/pull/7515#discussion_r857846935


##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -214,6 +215,56 @@ public void 
testTwoSendersWithSameIdShouldUseSameValueForEnforceThreadsConnectTo
 
   }
 
+
+  /**
+   * The aim of this test is verify that when several gateway receivers in a 
remote site share the
+   * same port and hostname-for-senders, the pings sent from the gateway 
senders reach the right
+   * gateway receiver and not just any of the receivers. Check that only one 
destination will be
+   * pinged.
+   */
+  @Test
+  public void 
testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceiver()
+      throws InterruptedException {
+    String senderId = "ln";
+    String regionName = "region-wan";
+    final int remoteLocPort = docker.getExternalPortForService("haproxy", 
20334);
+
+    int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+    VM vm1 = VM.getVM(1);

Review Comment:
   I notice that this is a dunit test in the `acceptanceTest` src set. It 
should probably be in `distributedTest` instead. The acceptance tests are all 
written using GfshRule without dunit.
   
   Test names should reflect the test types:
   
   Unit test: MyTest
   
   - Integration test: `MyIntegrationTest`
   - Distributed test: `MyDistributedTest`
   - Acceptance test: `MyAcceptanceTest`



##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -214,6 +215,56 @@ public void 
testTwoSendersWithSameIdShouldUseSameValueForEnforceThreadsConnectTo
 
   }
 
+
+  /**
+   * The aim of this test is verify that when several gateway receivers in a 
remote site share the
+   * same port and hostname-for-senders, the pings sent from the gateway 
senders reach the right
+   * gateway receiver and not just any of the receivers. Check that only one 
destination will be
+   * pinged.
+   */
+  @Test
+  public void 
testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceiver()
+      throws InterruptedException {
+    String senderId = "ln";
+    String regionName = "region-wan";
+    final int remoteLocPort = docker.getExternalPortForService("haproxy", 
20334);
+
+    int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+    VM vm1 = VM.getVM(1);
+    createCache(vm1, locPort);
+
+    // We use one dispatcher thread. With just one dispatcher thread, only one
+    // connection will be created by the sender towards one of the receivers 
and it will be
+    // monitored by the one ping thread for that remote receiver.
+    createGatewaySender(vm1, senderId, 2, true, 5,
+        1, GatewaySender.DEFAULT_ORDER_POLICY);
+
+    createPartitionedRegion(vm1, regionName, senderId, 0, 10);
+
+    int NUM_PUTS = 1;
+
+    putKeyValues(vm1, NUM_PUTS, regionName);
+
+    await().untilAsserted(() -> assertThat(getQueuedEvents(vm1, 
senderId)).isEqualTo(0));
+
+
+    // Wait longer than the value set in the receivers for
+    // maximum-time-between-pings: 10000 (see geode-starter-create.gfsh)
+    // to verify that connections are not closed
+    // by the receivers because each has received the pings timely.
+    int maxTimeBetweenPingsInReceiver = 15000;
+    Thread.sleep(maxTimeBetweenPingsInReceiver);
+
+    int senderPoolDisconnects = getSenderPoolDisconnects(vm1, senderId);
+    assertEquals(0, senderPoolDisconnects);
+
+    int poolEndPointSize = getPoolEndPointSize(vm1, senderId);
+    assertEquals(1, poolEndPointSize);

Review Comment:
   Please always use AssertJ assertions instead of JUnit assertions.



##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -214,6 +215,56 @@ public void 
testTwoSendersWithSameIdShouldUseSameValueForEnforceThreadsConnectTo
 
   }
 
+
+  /**
+   * The aim of this test is verify that when several gateway receivers in a 
remote site share the
+   * same port and hostname-for-senders, the pings sent from the gateway 
senders reach the right
+   * gateway receiver and not just any of the receivers. Check that only one 
destination will be
+   * pinged.
+   */
+  @Test
+  public void 
testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceiver()
+      throws InterruptedException {
+    String senderId = "ln";
+    String regionName = "region-wan";
+    final int remoteLocPort = docker.getExternalPortForService("haproxy", 
20334);
+
+    int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+    VM vm1 = VM.getVM(1);
+    createCache(vm1, locPort);
+
+    // We use one dispatcher thread. With just one dispatcher thread, only one
+    // connection will be created by the sender towards one of the receivers 
and it will be
+    // monitored by the one ping thread for that remote receiver.
+    createGatewaySender(vm1, senderId, 2, true, 5,
+        1, GatewaySender.DEFAULT_ORDER_POLICY);
+
+    createPartitionedRegion(vm1, regionName, senderId, 0, 10);
+
+    int NUM_PUTS = 1;
+
+    putKeyValues(vm1, NUM_PUTS, regionName);
+
+    await().untilAsserted(() -> assertThat(getQueuedEvents(vm1, 
senderId)).isEqualTo(0));
+
+
+    // Wait longer than the value set in the receivers for
+    // maximum-time-between-pings: 10000 (see geode-starter-create.gfsh)
+    // to verify that connections are not closed
+    // by the receivers because each has received the pings timely.
+    int maxTimeBetweenPingsInReceiver = 15000;
+    Thread.sleep(maxTimeBetweenPingsInReceiver);

Review Comment:
   `Thread.sleep` almost always results in flaky tests that fail 
intermittently. See if you can figure out a different way to test this behavior 
using `Awaitility` 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to