diff options
Diffstat (limited to 'config')
3 files changed, 76 insertions, 123 deletions
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java index efb93c6aed2..6878397a403 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java @@ -44,8 +44,8 @@ public class JRTConfigRequester implements RequestWaiter { private static final JRTManagedConnectionPools managedPool = new JRTManagedConnectionPools(); private static final int TRACELEVEL = 6; private final TimingValues timingValues; - private int fatalFailures = 0; // independent of transientFailures - private int transientFailures = 0; // independent of fatalFailures + private boolean fatalFailures = false; + private boolean transientFailures = false; private final ScheduledThreadPoolExecutor scheduler; private Instant noApplicationWarningLogged = Instant.MIN; private static final Duration delayBetweenWarnings = Duration.ofSeconds(60); @@ -173,19 +173,21 @@ public class JRTConfigRequester implements RequestWaiter { } } - static long calculateFailedRequestDelay(ErrorType errorCode, int transientFailures, int fatalFailures, + static long calculateFailedRequestDelay(ErrorType errorType, boolean transientFailures, boolean fatalFailures, TimingValues timingValues, boolean configured) { - long delay; - if (configured) - delay = timingValues.getConfiguredErrorDelay(); - else - delay = timingValues.getUnconfiguredDelay(); - - if (errorCode == ErrorType.TRANSIENT) { - delay = delay * Math.min((transientFailures + 1), timingValues.getMaxDelayMultiplier()); - } else { - delay = timingValues.getFixedDelay() + (delay * Math.min(fatalFailures, timingValues.getMaxDelayMultiplier())); - delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); + long delay = (configured ? timingValues.getConfiguredErrorDelay(): timingValues.getUnconfiguredDelay()); + + switch (errorType) { + case TRANSIENT: + if (transientFailures) + delay = delay * 2; + break; + case FATAL: + delay = timingValues.getFixedDelay() + (delay * (fatalFailures ? 1 : 0)); + delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); + break; + default: + throw new IllegalArgumentException("Unknown error type " + errorType); } return delay; } @@ -194,7 +196,7 @@ public class JRTConfigRequester implements RequestWaiter { JRTConfigSubscription<ConfigInstance> sub, long delay, Connection connection) { - transientFailures++; + transientFailures = true; log.log(INFO, "Connection to " + connection.getAddress() + " failed or timed out, clients will keep existing config, will keep trying."); if (sub.getState() != ConfigSubscription.State.OPEN) return; @@ -217,7 +219,7 @@ public class JRTConfigRequester implements RequestWaiter { private void handleFatallyFailed(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, long delay) { if (sub.getState() != ConfigSubscription.State.OPEN) return; - fatalFailures++; + fatalFailures = true; // The logging depends on whether we are configured or not. Level logLevel = sub.getConfigState().getConfig() == null ? Level.FINE : Level.INFO; String logMessage = "Request for config " + jrtReq.getShortDescription() + "' failed with error code " + @@ -230,9 +232,8 @@ public class JRTConfigRequester implements RequestWaiter { private void handleOKRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, Connection connection) { - // Reset counters pertaining to error handling here - fatalFailures = 0; - transientFailures = 0; + fatalFailures = false; + transientFailures = false; noApplicationWarningLogged = Instant.MIN; sub.setLastCallBackOKTS(Instant.now()); log.log(FINE, () -> "OK response received in handleOkRequest: " + jrtReq); @@ -302,11 +303,11 @@ public class JRTConfigRequester implements RequestWaiter { } } - int getTransientFailures() { + boolean getTransientFailures() { return transientFailures; } - int getFatalFailures() { + boolean getFatalFailures() { return fatalFailures; } diff --git a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java index 5d5967e56c4..fb7ef03e918 100644 --- a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java +++ b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java @@ -1,4 +1,4 @@ -// 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.vespa.config; import java.util.Random; @@ -21,7 +21,6 @@ public class TimingValues { private long fixedDelay = 5000; private long unconfiguredDelay = 1000; private long configuredErrorDelay = 15000; - private int maxDelayMultiplier = 10; private final Random rand; public TimingValues() { @@ -38,8 +37,7 @@ public class TimingValues { long subscribeTimeout, long unconfiguredDelay, long configuredErrorDelay, - long fixedDelay, - int maxDelayMultiplier) { + long fixedDelay) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; this.initialTimeout = initialTimeout; @@ -47,7 +45,6 @@ public class TimingValues { this.unconfiguredDelay = unconfiguredDelay; this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; - this.maxDelayMultiplier = maxDelayMultiplier; this.rand = new Random(System.currentTimeMillis()); } @@ -58,7 +55,6 @@ public class TimingValues { long unconfiguredDelay, long configuredErrorDelay, long fixedDelay, - int maxDelayMultiplier, Random rand) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; @@ -67,7 +63,6 @@ public class TimingValues { this.unconfiguredDelay = unconfiguredDelay; this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; - this.maxDelayMultiplier = maxDelayMultiplier; this.rand = rand; } @@ -79,7 +74,6 @@ public class TimingValues { tv.unconfiguredDelay, tv.configuredErrorDelay, tv.fixedDelay, - tv.maxDelayMultiplier, random); } @@ -154,16 +148,6 @@ public class TimingValues { } /** - * Returns maximum multiplier to use when calculating delay (the delay is multiplied by the number of - * failed requests, unless that number is this maximum multiplier). - * - * @return timeout in milliseconds. - */ - public int getMaxDelayMultiplier() { - return maxDelayMultiplier; - } - - /** * Returns fixed delay that is used when retrying getting config no matter if it was a success or an error * and independent of number of retries. * @@ -187,13 +171,14 @@ public class TimingValues { @Override public String toString() { return "TimingValues [successTimeout=" + successTimeout - + ", errorTimeout=" + errorTimeout + ", initialTimeout=" - + initialTimeout + ", subscribeTimeout=" + subscribeTimeout - + ", configuredErrorTimeout=" + configuredErrorTimeout - + ", fixedDelay=" + fixedDelay + ", unconfiguredDelay=" - + unconfiguredDelay + ", configuredErrorDelay=" - + configuredErrorDelay + ", maxDelayMultiplier=" - + maxDelayMultiplier + ", rand=" + rand + "]"; + + ", errorTimeout=" + errorTimeout + + ", initialTimeout=" + initialTimeout + + ", subscribeTimeout=" + subscribeTimeout + + ", configuredErrorTimeout=" + configuredErrorTimeout + + ", fixedDelay=" + fixedDelay + + ", unconfiguredDelay=" + unconfiguredDelay + + ", configuredErrorDelay=" + configuredErrorDelay + + ", rand=" + rand + "]"; } 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 { |