From 5dead63f82597b1d86c0be98fad743ae01dd30ae Mon Sep 17 00:00:00 2001
From: Bela Ban <belaban@yahoo.com>
Date: Mon, 20 Feb 2017 11:16:21 +0100
Subject: [PATCH] UNICAST3_Test now fails if CONN_CLOSE_TIMEOUT is small (e.g.
 1000)

---
 .../org/jgroups/protocols/UNICAST3_Test.java  | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tests/junit-functional/org/jgroups/protocols/UNICAST3_Test.java b/tests/junit-functional/org/jgroups/protocols/UNICAST3_Test.java
index 6c2c90f784..a7e49dd71e 100644
--- a/tests/junit-functional/org/jgroups/protocols/UNICAST3_Test.java
+++ b/tests/junit-functional/org/jgroups/protocols/UNICAST3_Test.java
@@ -22,6 +22,7 @@ public class UNICAST3_Test {
     protected UNICAST3           uni_a, uni_b;
     protected DropUnicastAck     drop_ack=new DropUnicastAck((short)499);
     protected static final short UNICAST3_ID=ClassConfigurator.getProtocolId(UNICAST3.class);
+    protected static final int   CONN_CLOSE_TIMEOUT=60_000; // change to a low value (e.g. 1000) to make this test fail
 
     @BeforeMethod protected void setup() throws Exception {
         a=create("A").connect(getClass().getSimpleName());
@@ -36,6 +37,21 @@ public class UNICAST3_Test {
     @AfterMethod protected void destroy() {Util.close(b, a);}
 
 
+    /**
+     - A and B exchanging unicast messages
+     - Seqno 499 is sent from A to B
+     - B adds it to its table for A, and sends the ACK, then delivers the message
+     - The ack for 499 from B to A is dropped
+     - B excludes A (but A doesn't exclude B) and removes A's table
+         * This happens only if conn_close_timeout is small (default: 10s)
+         * If conn_close_timeout == 0, connections will not be removed
+     - A retransmits 499 to B
+     - B receives A:499, but asks for the first seqno
+     - A has its highest seqno acked at 498, so resends 499 with first==true
+     - B creates a receiver window for A at 499, receives 499 and delivers it (again)
+
+     The issue is fixed by setting CONN_CLOSE_TIMEOUT to a highher value, or to 0
+     */
     public void testDuplicateMessageDelivery() throws Exception {
         b.setReceiver(receiver);
 
@@ -52,12 +68,13 @@ public void testDuplicateMessageDelivery() throws Exception {
 
         // remove A's receive window in B:
         System.out.printf("-- closing the receive-window for %s:\n", a_addr);
+        // e.g. caused by an asymmetric network split: B excludes A, but not vice versa
         uni_b.closeReceiveConnection(a_addr);
 
         uni_a.setLevel("trace");
         uni_b.setLevel("trace");
 
-        uni_b.setValue("conn_close_timeout", 1000);
+        uni_b.setValue("conn_close_timeout", CONN_CLOSE_TIMEOUT);
         // wait until B closes the receive window for A:
         for(int i=0; i < 10; i++) {
             if(uni_b.getNumReceiveConnections() == 0)
@@ -65,7 +82,7 @@ public void testDuplicateMessageDelivery() throws Exception {
             Util.sleep(500);
         }
 
-        assert uni_b.getNumReceiveConnections() == 0;
+        // assert uni_b.getNumReceiveConnections() > 0;
 
         // remove the DropUnicastAck protocol:
         System.out.printf("-- removing the %s protocol\n", DropUnicastAck.class.getSimpleName());
@@ -77,7 +94,7 @@ public void testDuplicateMessageDelivery() throws Exception {
             Util.sleep(100);
         }
         System.out.printf("B: received %d messages from A\n", receiver.count);
-        assert receiver.count == 500;
+        assert receiver.count == 499 : String.format("received %d messages, but should only have received 499", receiver.count);
     }