Simplify calculating delay after config failures

When there are several subscribers to a config and 1 JRTConfiguRequester
the counter for failures might quickly increase so backoff based
on these numbers will not be useful. Simplify by using booleans
for failures instead. Max delay for configured subscriptions will
now be 30 seconds instead of 150 seconds (which is way too much)

1 files changed, 44 insertions, 77 deletions

diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
index 1b56e9290b2..658404f1772 100644
--- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
@@ -1,9 +1,9 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.subscription.impl;
 
 import com.yahoo.config.subscription.ConfigSourceSet;
-import com.yahoo.foo.SimpletypesConfig;
 import com.yahoo.config.subscription.ConfigSubscriber;
+import com.yahoo.foo.SimpletypesConfig;
 import com.yahoo.jrt.Request;
 import com.yahoo.vespa.config.ConfigKey;
 import com.yahoo.vespa.config.ConnectionPool;
@@ -17,11 +17,11 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Random;
 
-import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -36,99 +36,68 @@ public class JRTConfigRequesterTest {
         TimingValues timingValues = new TimingValues(defaultTimingValues, random);
 
         // transientFailures and fatalFailures are not set until after delay has been calculated,
-        // so 0 is the case for the first failure
-        int transientFailures = 0;
-        int fatalFailures = 0;
+        // so false is the case for the first failure
+        boolean transientFailures = false;
+        boolean fatalFailures = false;
         boolean configured = false;
 
         // First time failure, not configured
         long delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is(timingValues.getUnconfiguredDelay()));
-        transientFailures = 5;
+        assertEquals(timingValues.getUnconfiguredDelay(), delay);
+        transientFailures = true;
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is((transientFailures + 1) * timingValues.getUnconfiguredDelay()));
-        transientFailures = 0;
+        assertEquals(timingValues.getUnconfiguredDelay() * 2, delay);
+        transientFailures = false;
 
 
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.FATAL,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
-        assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
-        assertThat(delay, is(5462L));
+        assertTrue("delay=" + delay, delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
+        assertTrue("delay=" + delay,delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
+        assertEquals(5462, delay);
 
         // First time failure, configured
         configured = true;
-
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is(timingValues.getConfiguredErrorDelay()));
+        assertEquals(timingValues.getConfiguredErrorDelay(), delay);
 
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.FATAL,
                 transientFailures, fatalFailures, timingValues, configured);
         assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
         assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay());
-        assertThat(delay, is(5663L));
+        assertEquals(5663, delay);
 
 
         // nth time failure, not configured
-        fatalFailures = 1;
+        fatalFailures = true;
         configured = false;
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is(timingValues.getUnconfiguredDelay()));
+        assertEquals(timingValues.getUnconfiguredDelay(), delay);
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.FATAL,
                 transientFailures, fatalFailures, timingValues, configured);
         final long l = timingValues.getFixedDelay() + timingValues.getUnconfiguredDelay();
         assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l);
         assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l);
-        assertThat(delay, is(5377L));
+        assertEquals(5377, delay);
 
 
         // nth time failure, configured
-        fatalFailures = 1;
+        fatalFailures = true;
+        transientFailures = true;
         configured = true;
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
                 transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is(timingValues.getConfiguredErrorDelay()));
+        assertEquals(timingValues.getConfiguredErrorDelay() * 2, delay);
         delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.FATAL,
                 transientFailures, fatalFailures, timingValues, configured);
         final long l1 = timingValues.getFixedDelay() + timingValues.getConfiguredErrorDelay();
         assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l1);
         assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l1);
-        assertThat(delay, is(20851L));
-
-
-        // 1 more than max delay multiplier time failure, configured
-        fatalFailures = timingValues.getMaxDelayMultiplier() + 1;
-        configured = true;
-        delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
-                transientFailures, fatalFailures, timingValues, configured);
-        assertThat(delay, is(timingValues.getConfiguredErrorDelay()));
-        assertTrue(delay < timingValues.getMaxDelayMultiplier() * timingValues.getConfiguredErrorDelay());
-        delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.FATAL,
-                transientFailures, fatalFailures, timingValues, configured);
-        final long l2 = timingValues.getFixedDelay() + timingValues.getMaxDelayMultiplier() * timingValues.getConfiguredErrorDelay();
-        assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l2);
-        assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l2);
-        assertThat(delay, is(163520L));
-    }
-
-    @Test
-    public void testDelay() {
-        TimingValues timingValues = new TimingValues();
-
-        // transientFailures and fatalFailures are not set until after delay has been calculated,
-        // so 0 is the case for the first failure
-        int transientFailures = 0;
-        int fatalFailures = 0;
-
-        // First time failure, configured
-        long delay = JRTConfigRequester.calculateFailedRequestDelay(ErrorType.TRANSIENT,
-                transientFailures, fatalFailures, timingValues, true);
-        assertThat(delay, is(timingValues.getConfiguredErrorDelay()));
-        assertThat(delay, is((transientFailures + 1) * timingValues.getConfiguredErrorDelay()));
+        assertEquals(20851L, delay);
     }
 
     @Test
@@ -139,10 +108,10 @@ public class JRTConfigRequesterTest {
                 ErrorCode.ILLEGAL_DEF_MD5, ErrorCode.ILLEGAL_CONFIG_MD5, ErrorCode.ILLEGAL_TIMEOUT, ErrorCode.INTERNAL_ERROR,
                 9999); // unknown should also be fatal
         for (Integer i : transientErrors) {
-            assertThat(ErrorType.getErrorType(i), is(ErrorType.TRANSIENT));
+            assertEquals(ErrorType.TRANSIENT, ErrorType.getErrorType(i));
         }
         for (Integer i : fatalErrors) {
-            assertThat(ErrorType.getErrorType(i), is(ErrorType.FATAL));
+            assertEquals(ErrorType.FATAL, ErrorType.getErrorType(i));
         }
     }
 
@@ -154,16 +123,16 @@ public class JRTConfigRequesterTest {
 
         final MockConnection connection = new MockConnection();
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
-        assertThat(requester.getConnectionPool(), is(connection));
+        assertEquals(requester.getConnectionPool(), connection);
         requester.request(sub);
         final Request request = connection.getRequest();
         assertNotNull(request);
-        assertThat(connection.getNumberOfRequests(), is(1));
+        assertEquals(1, connection.getNumberOfRequests());
         JRTServerConfigRequestV3 receivedRequest = JRTServerConfigRequestV3.createFromRequest(request);
         assertTrue(receivedRequest.validateParameters());
-        assertThat(receivedRequest.getTimeout(), is(timingValues.getSubscribeTimeout()));
-        assertThat(requester.getFatalFailures(), is(0));
-        assertThat(requester.getTransientFailures(), is(0));
+        assertEquals(timingValues.getSubscribeTimeout(), receivedRequest.getTimeout());
+        assertFalse(requester.getFatalFailures());
+        assertFalse(requester.getTransientFailures());
     }
 
     @Test
@@ -175,8 +144,8 @@ public class JRTConfigRequesterTest {
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
         requester.request(createSubscription(subscriber, timingValues));
         waitUntilResponse(connection);
-        assertThat(requester.getFatalFailures(), is(1));
-        assertThat(requester.getTransientFailures(), is(0));
+        assertTrue(requester.getFatalFailures());
+        assertFalse(requester.getTransientFailures());;
     }
 
     @Test
@@ -190,8 +159,8 @@ public class JRTConfigRequesterTest {
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
         requester.request(sub);
         waitUntilResponse(connection);
-        assertThat(requester.getFatalFailures(), is(1));
-        assertThat(requester.getTransientFailures(), is(0));
+        assertTrue(requester.getFatalFailures());
+        assertFalse(requester.getTransientFailures());;
     }
 
     @Test
@@ -203,8 +172,8 @@ public class JRTConfigRequesterTest {
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
         requester.request(createSubscription(subscriber, timingValues));
         waitUntilResponse(connection);
-        assertThat(requester.getFatalFailures(), is(0));
-        assertThat(requester.getTransientFailures(), is(1));
+        assertFalse(requester.getFatalFailures());
+        assertTrue(requester.getTransientFailures());
     }
 
     @Test
@@ -218,8 +187,8 @@ public class JRTConfigRequesterTest {
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
         requester.request(sub);
         waitUntilResponse(connection);
-        assertThat(requester.getFatalFailures(), is(0));
-        assertThat(requester.getTransientFailures(), is(1));
+        assertFalse(requester.getFatalFailures());
+        assertTrue(requester.getTransientFailures());
     }
 
     @Test
@@ -231,12 +200,11 @@ public class JRTConfigRequesterTest {
 
         final MockConnection connection = new MockConnection(new ErrorResponseHandler(ErrorCode.UNKNOWN_DEFINITION));
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
-        assertThat(requester.getConnectionPool(), is(connection));
+        assertEquals(requester.getConnectionPool(), connection);
         requester.request(sub);
         waitUntilResponse(connection);
-        assertThat(requester.getFatalFailures(), is(1));
-        assertThat(requester.getTransientFailures(), is(0));
-        // TODO Check that no further request was sent?
+        assertTrue(requester.getFatalFailures());
+        assertFalse(requester.getTransientFailures());;
     }
 
     @Test
@@ -249,14 +217,14 @@ public class JRTConfigRequesterTest {
         final MockConnection connection = new MockConnection(new MockConnection.OKResponseHandler());
         JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
         requester.request(sub);
-        assertThat(connection.getNumberOfRequests(), is(1));
+        assertEquals(1, connection.getNumberOfRequests());
         // Check that no further request was sent?
         try {
             Thread.sleep(timingValues.getFixedDelay()*2);
         } catch (InterruptedException e) {
             e.printStackTrace();
         }
-        assertThat(connection.getNumberOfRequests(), is(1));
+        assertEquals(1, connection.getNumberOfRequests());
     }
 
     @Test
@@ -308,8 +276,7 @@ public class JRTConfigRequesterTest {
             2000,  // subscribeTimeout
             250,   // unconfiguredDelay
             500,   // configuredErrorDelay
-            250,   // fixedDelay
-            5);    // maxDelayMultiplier
+            250);   // fixedDelay
     }
 
     private static class ErrorResponseHandler extends MockConnection.OKResponseHandler {