You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "ekaterinadimitrova2 (via GitHub)" <gi...@apache.org> on 2023/02/09 21:04:23 UTC

[GitHub] [cassandra] ekaterinadimitrova2 commented on a diff in pull request #2143: [CASSANDRA-17979] Remove invalid handshake test assertion

ekaterinadimitrova2 commented on code in PR #2143:
URL: https://github.com/apache/cassandra/pull/2143#discussion_r1102022551


##########
test/unit/org/apache/cassandra/net/HandshakeTest.java:
##########
@@ -377,24 +378,35 @@ private void testOutboundFallbackOnSSLHandshakeFailure(SslFallbackConnectionType
             InetAddressAndPort endpoint = inbound.sockets().stream().map(s -> s.settings.bindAddress).findFirst().get();
             inbound.open();
 
-            // Open outbound connections, and wait until connection is established
+            // Open outbound connections, and wait until connection is established and the initial message is delivered
             OutboundConnection outboundConnection = initiateOutbound(endpoint, fromConnectionType, fromOptional);
-            waitForConnection(outboundConnection);
-            assertTrue(outboundConnection.isConnected());
-            assertFalse(outboundConnection.hasPending());
+            confirmDelivery(outboundConnection);
         }
         finally
         {
             inbound.close().await(10L, TimeUnit.SECONDS);
         }
     }
 
-    private void waitForConnection(OutboundConnection outboundConnection) throws InterruptedException
+    private final static Duration DELIVERY_WAIT_DURATION = Duration.ofSeconds(60);
+
+    private void confirmDelivery(OutboundConnection outboundConnection) throws InterruptedException
     {
-        long startTime = System.currentTimeMillis();
-        while (!outboundConnection.isConnected() && System.currentTimeMillis() - startTime < 60000)
+        Instant endTime = Instant.now().plus(DELIVERY_WAIT_DURATION);
+
+        while (!outboundConnection.isConnected() && Instant.now().isBefore(endTime))
         {
             Thread.sleep(1000);
         }
+
+        // Now that we're connected, we also want to ensure that the delivery thread succeeded
+        while (outboundConnection.hasPending() && Instant.now().isBefore(endTime)) {

Review Comment:
   `We might be waiting indefinitely`
   @jyothsnakonisa  Can you elaborate a bit, please? I suspect I might be missing something



-- 
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: pr-unsubscribe@cassandra.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org