diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-10-05 20:12:56 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-05 20:12:56 +0200 |
commit | 5d24ada77e0f61eca8b0c66e13de02c868ce9147 (patch) | |
tree | 1d38432bd228f8f4a195d5e30b841047d3dbfede | |
parent | 81df9a8dfa793a068961b53883869653f91de343 (diff) | |
parent | 060961ce51b699ca575a1a51fe87787b8fedf79b (diff) |
Merge pull request #19422 from vespa-engine/hmusum/config-subscription-refactoring-part-1
Hmusum/config subscription refactoring part 1 [run-systemtest]
5 files changed, 111 insertions, 255 deletions
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index 71f1571b9c8..2e8685887c6 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.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 Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; import com.yahoo.concurrent.DaemonThreadFactory; @@ -11,6 +11,7 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; import com.yahoo.vespa.config.ConfigCacheKey; +import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; @@ -37,8 +38,8 @@ import static java.util.concurrent.TimeUnit.SECONDS; */ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { - private final static Logger log = Logger.getLogger(RpcConfigSourceClient.class.getName()); - private static final double timingValuesRatio = 0.8; + private static final Logger log = Logger.getLogger(RpcConfigSourceClient.class.getName()); + private static final TimingValues timingValues = createTimingValues(); private final Supervisor supervisor = new Supervisor(new Transport("config-source-client")); @@ -47,7 +48,6 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { private final Map<ConfigCacheKey, Subscriber> activeSubscribers = new ConcurrentHashMap<>(); private final MemoryCache memoryCache; private final DelayedResponses delayedResponses; - private final static TimingValues timingValues; private final ScheduledExecutorService nextConfigScheduler = Executors.newScheduledThreadPool(1, new DaemonThreadFactory("next config")); private final ScheduledFuture<?> nextConfigFuture; @@ -57,16 +57,6 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { Executors.newScheduledThreadPool(1, new DaemonThreadFactory("delayed responses")); private final ScheduledFuture<?> delayedResponsesFuture; - static { - // Proxy should time out before clients upon subscription. - TimingValues tv = new TimingValues(); - tv.setUnconfiguredDelay((long)(tv.getUnconfiguredDelay()* timingValuesRatio)). - setConfiguredErrorDelay((long)(tv.getConfiguredErrorDelay()* timingValuesRatio)). - setSubscribeTimeout((long)(tv.getSubscribeTimeout()* timingValuesRatio)). - setConfiguredErrorTimeout(-1); // Never cache errors - timingValues = tv; - } - RpcConfigSourceClient(RpcServer rpcServer, ConfigSourceSet configSourceSet, MemoryCache memoryCache) { this.rpcServer = rpcServer; this.configSourceSet = configSourceSet; @@ -74,35 +64,31 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { this.delayedResponses = new DelayedResponses(); checkConfigSources(); nextConfigFuture = nextConfigScheduler.scheduleAtFixedRate(this, 0, 10, MILLISECONDS); - requester = JRTConfigRequester.create(configSourceSet, timingValues); + this.requester = JRTConfigRequester.create(configSourceSet, timingValues); DelayedResponseHandler command = new DelayedResponseHandler(delayedResponses, memoryCache, rpcServer); - delayedResponsesFuture = delayedResponsesScheduler.scheduleAtFixedRate(command, 5, 1, SECONDS); + this.delayedResponsesFuture = delayedResponsesScheduler.scheduleAtFixedRate(command, 5, 1, SECONDS); } /** * Checks if config sources are available */ private void checkConfigSources() { - if (configSourceSet == null || configSourceSet.getSources() == null || configSourceSet.getSources().size() == 0) { - log.log(Level.WARNING, "No config sources defined, could not check connection"); - } else { - Request req = new Request("ping"); - for (String configSource : configSourceSet.getSources()) { - Spec spec = new Spec(configSource); - Target target = supervisor.connect(spec); - target.invokeSync(req, 30.0); - if (target.isValid()) { - log.log(Level.FINE, () -> "Created connection to config source at " + spec.toString()); - return; - } else { - log.log(Level.INFO, "Could not connect to config source at " + spec.toString()); - } - target.close(); - } - String extra = ""; - log.log(Level.INFO, "Could not connect to any config source in set " + configSourceSet.toString() + - ", please make sure config server(s) are running. " + extra); + if (configSourceSet == null || configSourceSet.getSources() == null || configSourceSet.getSources().size() == 0) + throw new IllegalArgumentException("No config sources defined, could not check connection"); + + Request req = new Request("ping"); + for (String configSource : configSourceSet.getSources()) { + Spec spec = new Spec(configSource); + Target target = supervisor.connect(spec); + target.invokeSync(req, 30.0); + if (target.isValid()) + return; + + log.log(Level.INFO, "Could not connect to config source at " + spec.toString()); + target.close(); } + log.log(Level.INFO, "Could not connect to any config source in set " + configSourceSet.toString() + + ", please make sure config server(s) are running."); } /** @@ -126,7 +112,7 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { DelayedResponse delayedResponse = new DelayedResponse(request); delayedResponses.add(delayedResponse); - final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); + ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); RawConfig cachedConfig = memoryCache.get(configCacheKey); boolean needToGetConfig = true; @@ -219,40 +205,41 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { } /** - * This method will be called when a response with changed config is received from upstream - * (content or generation has changed) or the server timeout has elapsed. + * Updates subscribers with new config. This method will be called when a response with changed config is + * received from upstream (content or generation has changed) or the server timeout has elapsed. * * @param config new config */ public void updateSubscribers(RawConfig config) { - log.log(Level.FINE, () -> "Config updated for " + config.getKey() + "," + config.getGeneration()); + ConfigKey<?> key = config.getKey(); + long generation = config.getGeneration(); + log.log(Level.FINE, () -> "Config updated for " + key + "," + generation); DelayQueue<DelayedResponse> responseDelayQueue = delayedResponses.responses(); + if (responseDelayQueue.size() == 0) return; + + log.log(Level.FINE, () -> "Delayed response queue has " + responseDelayQueue.size() + " elements"); log.log(Level.FINEST, () -> "Delayed response queue: " + responseDelayQueue); - if (responseDelayQueue.size() == 0) { - log.log(Level.FINE, () -> "There exists no matching element on delayed response queue for " + config.getKey()); - return; - } else { - log.log(Level.FINE, () -> "Delayed response queue has " + responseDelayQueue.size() + " elements"); - } boolean found = false; for (DelayedResponse response : responseDelayQueue.toArray(new DelayedResponse[0])) { JRTServerConfigRequest request = response.getRequest(); - if (request.getConfigKey().equals(config.getKey()) + if (request.getConfigKey().equals(key) // Generation 0 is special, used when returning empty sentinel config - && (config.getGeneration() >= request.getRequestGeneration() || config.getGeneration() == 0)) { + && (generation >= request.getRequestGeneration() || generation == 0)) { if (delayedResponses.remove(response)) { found = true; - log.log(Level.FINE, () -> "Call returnOkResponse for " + config.getKey() + "," + config.getGeneration()); + log.log(Level.FINE, () -> "Call returnOkResponse for " + key + "," + generation); + if (config.getPayload().getData().getByteLength() == 0) + log.log(Level.WARNING, () -> "Call returnOkResponse for " + key + "," + generation + " with empty config"); rpcServer.returnOkResponse(request, config); } else { - log.log(Level.INFO, "Could not remove " + config.getKey() + " from delayedResponses queue, already removed"); + log.log(Level.INFO, "Could not remove " + key + " from delayedResponses queue, already removed"); } } } if (!found) { - log.log(Level.FINE, () -> "Found no recipient for " + config.getKey() + " in delayed response queue"); + log.log(Level.FINE, () -> "Found no recipient for " + key + " in delayed response queue"); } - log.log(Level.FINE, () -> "Finished updating config for " + config.getKey() + "," + config.getGeneration()); + log.log(Level.FINE, () -> "Finished updating config for " + key + "," + generation); } @Override @@ -268,4 +255,14 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { updateSubscribers(newConfig); } + private static TimingValues createTimingValues() { + // Proxy should time out before clients upon subscription. + double timingValuesRatio = 0.8; + TimingValues tv = new TimingValues(); + tv.setFixedDelay((long) (tv.getFixedDelay() * timingValuesRatio)). + setSubscribeTimeout((long) (tv.getSubscribeTimeout() * timingValuesRatio)). + setConfiguredErrorTimeout(-1); // Never cache errors + return tv; + } + } 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 853bc4dfc00..7713d509f69 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 @@ -1,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. 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.ConfigInstance; @@ -9,7 +9,6 @@ import com.yahoo.jrt.RequestWaiter; import com.yahoo.vespa.config.Connection; import com.yahoo.vespa.config.ConnectionPool; import com.yahoo.vespa.config.ErrorCode; -import com.yahoo.vespa.config.ErrorType; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTClientConfigRequest; import com.yahoo.vespa.config.protocol.JRTConfigRequestFactory; @@ -43,17 +42,20 @@ public class JRTConfigRequester implements RequestWaiter { public static final ConfigSourceSet defaultSourceSet = ConfigSourceSet.createDefault(); private static final JRTManagedConnectionPools managedPool = new JRTManagedConnectionPools(); private static final int TRACELEVEL = 6; - private final TimingValues timingValues; - private boolean fatalFailures = false; - private final ScheduledThreadPoolExecutor scheduler; - private Instant noApplicationWarningLogged = Instant.MIN; private static final Duration delayBetweenWarnings = Duration.ofSeconds(60); - private final ConnectionPool connectionPool; - private final ConfigSourceSet configSourceSet; static final float randomFraction = 0.2f; /* Time to be added to server timeout to create client timeout. This is the time allowed for the server to respond after serverTimeout has elapsed. */ private static final Double additionalTimeForClientTimeout = 10.0; + private final TimingValues timingValues; + private final ScheduledThreadPoolExecutor scheduler; + + private final ConnectionPool connectionPool; + private final ConfigSourceSet configSourceSet; + + private Instant noApplicationWarningLogged = Instant.MIN; + private int failures = 0; + /** * Returns a new requester * @@ -129,15 +131,13 @@ public class JRTConfigRequester implements RequestWaiter { Trace trace = jrtReq.getResponseTrace(); trace.trace(TRACELEVEL, "JRTConfigRequester.doHandle()"); log.log(FINEST, () -> trace.toString()); - if (validResponse) { + if (validResponse) handleOKRequest(jrtReq, sub); - } else { - logWhenErrorResponse(jrtReq, connection); + else handleFailedRequest(jrtReq, sub, connection); - } } - private void logWhenErrorResponse(JRTClientConfigRequest jrtReq, Connection connection) { + private void logError(JRTClientConfigRequest jrtReq, Connection connection) { switch (jrtReq.errorCode()) { case com.yahoo.jrt.ErrorCode.CONNECTION: log.log(FINE, () -> "Request callback failed: " + jrtReq.errorMessage() + @@ -160,77 +160,36 @@ public class JRTConfigRequester implements RequestWaiter { } private void handleFailedRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, Connection connection) { - final boolean configured = (sub.getConfigState().getConfig() != null); - if (configured) { - // The subscription object has an "old" config, which is all we have to offer back now - log.log(INFO, "Failure of config subscription, clients will keep existing config until resolved: " + sub); - } - ErrorType errorType = ErrorType.getErrorType(jrtReq.errorCode()); + logError(jrtReq, connection); + + // The subscription object has an "old" config, which is all we have to offer back now + log.log(INFO, "Failure of config subscription tp " + connection.getAddress() + + ", clients will keep existing config until resolved: " + sub); connectionPool.setError(connection, jrtReq.errorCode()); - long delay = calculateFailedRequestDelay(errorType, fatalFailures, timingValues, configured); - if (errorType == ErrorType.TRANSIENT) { - handleTransientlyFailed(jrtReq, sub, delay, connection); - } else { - handleFatallyFailed(jrtReq, sub, delay); - } + failures++; + long delay = calculateFailedRequestDelay(failures, timingValues); + // The logging depends on whether we are configured or not. + Level logLevel = sub.getConfigState().getConfig() == null ? Level.FINE : Level.INFO; + log.log(logLevel, () -> "Request for config " + jrtReq.getShortDescription() + "' failed with error code " + + jrtReq.errorCode() + " (" + jrtReq.errorMessage() + "), scheduling new request " + + " in " + delay + " ms"); + scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); } - static long calculateFailedRequestDelay(ErrorType errorType, - boolean fatalFailures, - TimingValues timingValues, - boolean configured) { - long delay = configured ? timingValues.getConfiguredErrorDelay() : timingValues.getUnconfiguredDelay(); + static long calculateFailedRequestDelay(int failures, TimingValues timingValues) { + long delay = timingValues.getFixedDelay() * (long)Math.pow(2, failures); + delay = Math.min(60_000, delay); + delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); - switch (errorType) { - case TRANSIENT: - delay = timingValues.getRandomTransientDelay(delay); - break; - case FATAL: - delay = timingValues.getFixedDelay() + (fatalFailures ? delay : 0); - delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); - break; - default: - throw new IllegalArgumentException("Unknown error type " + errorType); - } return delay; } - private void handleTransientlyFailed(JRTClientConfigRequest jrtReq, - JRTConfigSubscription<ConfigInstance> sub, - long delay, - Connection connection) { - fatalFailures = false; - log.log(INFO, "Connection to " + connection.getAddress() + - " failed or timed out, clients will keep existing config, will keep trying."); - scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); - } - private long calculateErrorTimeout() { return timingValues.getPlusMinusFractionRandom(timingValues.getErrorTimeout(), randomFraction); } - /** - * This handles a fatal error both in the case that the subscriber is configured and not. - * The difference is in the delay (passed from outside) and the log level used for - * error message. - * - * @param jrtReq a JRT config request - * @param sub a config subscription - * @param delay delay before sending a new request - */ - private void handleFatallyFailed(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, long delay) { - 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 " + - jrtReq.errorCode() + " (" + jrtReq.errorMessage() + "), scheduling new connect " + - " in " + delay + " ms"; - log.log(logLevel, logMessage); - scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); - } - private void handleOKRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub) { - fatalFailures = false; + failures = 0; noApplicationWarningLogged = Instant.MIN; sub.setLastCallBackOKTS(Instant.now()); log.log(FINE, () -> "OK response received in handleOkRequest: " + jrtReq); @@ -303,9 +262,7 @@ public class JRTConfigRequester implements RequestWaiter { } } - boolean getFatalFailures() { - return fatalFailures; - } + int getFailures() { return failures; } // TODO: Should be package private, used in integrationtest.rb in system tests public ConnectionPool getConnectionPool() { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index e83fc7aefc5..d8551c37f41 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.config.protocol.Payload; import java.time.Duration; import java.time.Instant; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -39,7 +40,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc * The queue containing either nothing or the one (newest) request that has got callback from JRT, * but has not yet been handled. */ - private LinkedBlockingQueue<JRTClientConfigRequest> reqQueue = new LinkedBlockingQueue<>(); + private BlockingQueue<JRTClientConfigRequest> reqQueue = new LinkedBlockingQueue<>(); private ConfigSourceSet sources; public JRTConfigSubscription(ConfigKey<T> key, ConfigSubscriber subscriber, ConfigSource source, TimingValues timingValues) { @@ -134,9 +135,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc return configInstance; } - LinkedBlockingQueue<JRTClientConfigRequest> getReqQueue() { - return reqQueue; - } + BlockingQueue<JRTClientConfigRequest> getReqQueue() { return reqQueue; } @Override public boolean subscribe(long timeout) { 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 235928a7d0b..56f85845aa0 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 Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. 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; @@ -19,8 +19,6 @@ public class TimingValues { private long configuredErrorTimeout = -1; // Don't ever timeout (and do not use error response) when we are already configured private long fixedDelay = 5000; - private long unconfiguredDelay = 1000; - private long configuredErrorDelay = 15000; private final Random rand; public TimingValues() { @@ -35,15 +33,11 @@ public class TimingValues { long errorTimeout, long initialTimeout, long subscribeTimeout, - long unconfiguredDelay, - long configuredErrorDelay, long fixedDelay) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; this.initialTimeout = initialTimeout; this.subscribeTimeout = subscribeTimeout; - this.unconfiguredDelay = unconfiguredDelay; - this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; this.rand = new Random(System.currentTimeMillis()); } @@ -52,16 +46,12 @@ public class TimingValues { long errorTimeout, long initialTimeout, long subscribeTimeout, - long unconfiguredDelay, - long configuredErrorDelay, long fixedDelay, Random rand) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; this.initialTimeout = initialTimeout; this.subscribeTimeout = subscribeTimeout; - this.unconfiguredDelay = unconfiguredDelay; - this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; this.rand = rand; } @@ -71,8 +61,6 @@ public class TimingValues { tv.errorTimeout, tv.initialTimeout, tv.subscribeTimeout, - tv.unconfiguredDelay, - tv.configuredErrorDelay, tv.fixedDelay, random); } @@ -115,39 +103,6 @@ public class TimingValues { } /** - * Returns time to wait until next attempt to get config after a failed request when the client has not - * gotten a successful response to a config subscription (i.e, the client has not been configured). - * A negative value means that there will never be a next attempt. If a negative value is set, the - * user must also setSubscribeTimeout(0) to prevent a deadlock while subscribing. - * - * @return delay in milliseconds, a negative value means never. - */ - public long getUnconfiguredDelay() { - return unconfiguredDelay; - } - - public TimingValues setUnconfiguredDelay(long d) { - unconfiguredDelay = d; - return this; - } - - /** - * Returns time to wait until next attempt to get config after a failed request when the client has - * previously gotten a successful response to a config subscription (i.e, the client is configured). - * A negative value means that there will never be a next attempt. - * - * @return delay in milliseconds, a negative value means never. - */ - public long getConfiguredErrorDelay() { - return configuredErrorDelay; - } - - public TimingValues setConfiguredErrorDelay(long d) { - configuredErrorDelay = d; - return this; - } - - /** * 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. * @@ -157,6 +112,11 @@ public class TimingValues { return fixedDelay; } + public TimingValues setFixedDelay(long t) { + fixedDelay = t; + return this; + } + /** * Returns a number +/- a random component * @@ -168,16 +128,6 @@ public class TimingValues { return Math.round(value - (value * fraction) + (rand.nextFloat() * 2L * value * fraction)); } - /** - * Returns a number between 0 and maxValue - * - * @param maxValue max maxValue - * @return a number - */ - public long getRandomTransientDelay(long maxValue) { - return Math.round(rand.nextFloat() * maxValue); - } - @Override public String toString() { return "TimingValues [successTimeout=" + successTimeout @@ -186,8 +136,6 @@ public class TimingValues { + ", 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 919155a3944..e60c84df887 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,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. 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; @@ -20,7 +20,6 @@ import java.util.Random; import static com.yahoo.config.subscription.impl.JRTConfigRequester.calculateFailedRequestDelay; 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; @@ -34,59 +33,23 @@ public class JRTConfigRequesterTest { @Test public void testDelayCalculation() { TimingValues defaultTimingValues = new TimingValues(); - Random random = new Random(0); // Use seed to make tests predictable + Random random = new Random(0); // Use seed to make delays deterministic TimingValues timingValues = new TimingValues(defaultTimingValues, random); - // transientFailures and fatalFailures are not set until after delay has been calculated, - // so false is the case for the first failure - boolean fatalFailures = false; - boolean configured = false; - - // First time failure, not configured - long delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - - - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - assertTrue("delay=" + delay, delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertTrue("delay=" + delay,delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertEquals(4481, delay); - - // First time failure, configured - configured = true; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getConfiguredErrorDelay(), delay); - - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertEquals(5275, delay); - - - // nth time failure, not configured - fatalFailures = true; - configured = false; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - final long l = timingValues.getFixedDelay() + timingValues.getUnconfiguredDelay(); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l); - assertEquals(6121, delay); - - - // nth time failure, configured - fatalFailures = true; - configured = true; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getConfiguredErrorDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - final long l1 = timingValues.getFixedDelay() + timingValues.getConfiguredErrorDelay(); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l1); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l1); - assertEquals(20780, delay); + int failures = 1; + // First time failure + long delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(10924, delay); + + failures++; + // 2nd time failure + delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(22652, delay); + + failures++; + // 3rd time failure + delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(35849, delay); } @Test @@ -120,7 +83,7 @@ public class JRTConfigRequesterTest { JRTServerConfigRequestV3 receivedRequest = JRTServerConfigRequestV3.createFromRequest(request); assertTrue(receivedRequest.validateParameters()); assertEquals(timingValues.getSubscribeTimeout(), receivedRequest.getTimeout()); - assertFalse(requester.getFatalFailures()); + assertEquals(0, requester.getFailures()); } @Test @@ -132,7 +95,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(createSubscription(subscriber, timingValues)); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -146,7 +109,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -158,7 +121,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(createSubscription(subscriber, timingValues)); waitUntilResponse(connection); - assertFalse(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -172,7 +135,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); - assertFalse(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -187,7 +150,7 @@ public class JRTConfigRequesterTest { assertEquals(requester.getConnectionPool(), connection); requester.request(sub); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -257,8 +220,6 @@ public class JRTConfigRequesterTest { 500, // errorTimeout 500, // initialTimeout 2000, // subscribeTimeout - 250, // unconfiguredDelay - 500, // configuredErrorDelay 250); // fixedDelay } @@ -321,10 +282,4 @@ public class JRTConfigRequesterTest { requester2.close(); } - private void assertTransientDelay(long maxDelay, long delay) { - long minDelay = 0; - assertTrue("delay=" + delay + ", minDelay=" + minDelay + ",maxDelay=" + maxDelay, - delay >= minDelay && delay <= maxDelay); - } - } |