diff options
author | Harald Musum <musum@verizonmedia.com> | 2019-03-12 21:46:44 +0100 |
---|---|---|
committer | Harald Musum <musum@verizonmedia.com> | 2019-03-12 21:46:44 +0100 |
commit | 11b93cd8814c0db6609ee9b92cd5103aee51eb43 (patch) | |
tree | e1f536eaa11d225bfa1179efe7300407a0659980 /config-proxy | |
parent | 767db18444bc5364618f987a413143bc27818c34 (diff) |
Send response also when config generation is 0
Needed for returning empty sentinel config, since we cannot use a new generation
numbed. 0 is used as a special generation number that should allow response
to be given to client even if it is not higher than the current generation.
Diffstat (limited to 'config-proxy')
3 files changed, 67 insertions, 34 deletions
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java index f20b45cbe45..e57758f3eec 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ClientUpdater.java @@ -49,7 +49,8 @@ class ClientUpdater { for (DelayedResponse response : responseDelayQueue.toArray(new DelayedResponse[0])) { JRTServerConfigRequest request = response.getRequest(); if (request.getConfigKey().equals(config.getKey()) - && (config.getGeneration() >= request.getRequestGeneration())) { + // Generation 0 is special, used when returning empty sentinel config + && (config.getGeneration() >= request.getRequestGeneration() || config.getGeneration() == 0)) { if (delayedResponses.remove(response)) { found = true; log.log(LogLevel.DEBUG, () -> "Call returnOkResponse for " + config.getKey() + "," + config.getGeneration()); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ClientUpdaterTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ClientUpdaterTest.java index fdf59a45841..25140afd4cc 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ClientUpdaterTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ClientUpdaterTest.java @@ -10,14 +10,13 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; /** * @author hmusum */ public class ClientUpdaterTest { + private MockRpcServer rpcServer; private ConfigProxyStatistics statistics; private DelayedResponses delayedResponses; @@ -37,53 +36,83 @@ public class ClientUpdaterTest { @Test public void basic() { - assertThat(rpcServer.responses, is(0L)); - final RawConfig fooConfig = ProxyServerTest.fooConfig; - clientUpdater.updateSubscribers(fooConfig); - - // No delayed response, so not returned - assertEquals(0, rpcServer.responses); + configUpdatedSendResponse(fooConfig); + // Nobody asked for the config, so no response sent + assertSentResponses(0); - delayedResponses.add(new DelayedResponse(JRTServerConfigRequestV3.createFromRequest(JRTConfigRequestFactory.createFromRaw(fooConfig, -10L).getRequest()))); - clientUpdater.updateSubscribers(fooConfig); - assertEquals(1, rpcServer.responses); + simulateClientRequestingConfig(fooConfig); + configUpdatedSendResponse(fooConfig); + assertSentResponses(1); - // Will not find bar config in delayed responses + // Nobody asked for 'bar' config RawConfig barConfig = new RawConfig(new ConfigKey<>("bar", "id", "namespace"), fooConfig.getDefMd5()); - clientUpdater.updateSubscribers(barConfig); - assertEquals(1, rpcServer.responses); + configUpdatedSendResponse(barConfig); + assertSentResponses(1); } @Test public void errorResponse() { - assertThat(rpcServer.responses, is(0L)); - - clientUpdater.updateSubscribers(ProxyServerTest.errorConfig); - assertThat(rpcServer.responses, is(0L)); - assertThat(statistics.errors(), is(1L)); + configUpdatedSendResponse(ProxyServerTest.errorConfig); + assertSentResponses(0); + assertEquals(1, statistics.errors()); } @Test public void it_does_not_send_old_config_in_response() { - assertThat(rpcServer.responses, is(0L)); + RawConfig fooConfigOldGeneration = ProxyServerTest.fooConfig; + + RawConfig fooConfig = createConfigWithNextConfigGeneration(fooConfigOldGeneration); + configUpdatedSendResponse(fooConfig); + + // Nobody asked for the config + assertSentResponses(0); + + simulateClientRequestingConfig(fooConfig); + configUpdatedSendResponse(fooConfig); + assertSentResponses(1); + + simulateClientRequestingConfig(fooConfig); + configUpdatedSendResponse(fooConfigOldGeneration); + // Old config generation, so no response returned + assertSentResponses(1); + } + @Test + public void it_does_send_config_with_generation_0_in_response() { RawConfig fooConfigOldGeneration = ProxyServerTest.fooConfig; - final RawConfig fooConfig = ProxyServerTest.createConfigWithNextConfigGeneration(fooConfigOldGeneration, 0); - clientUpdater.updateSubscribers(fooConfig); + RawConfig fooConfig = createConfigWithNextConfigGeneration(fooConfigOldGeneration, 1); + + simulateClientRequestingConfig(fooConfig); + configUpdatedSendResponse(fooConfig); + assertSentResponses(1); + + RawConfig fooConfig2 = createConfigWithNextConfigGeneration(fooConfigOldGeneration, 0); + simulateClientRequestingConfig(fooConfig2); + configUpdatedSendResponse(fooConfig2); + assertSentResponses(2); + } + + private void assertSentResponses(int expected) { + assertEquals(expected, rpcServer.responses); + } - // No delayed response, so not returned - assertEquals(0, rpcServer.responses); + private void simulateClientRequestingConfig(RawConfig config) { + delayedResponses.add(new DelayedResponse(JRTServerConfigRequestV3.createFromRequest(JRTConfigRequestFactory.createFromRaw(config, -10L).getRequest()))); + } - delayedResponses.add(new DelayedResponse(JRTServerConfigRequestV3.createFromRequest(JRTConfigRequestFactory.createFromRaw(fooConfig, -10L).getRequest()))); - clientUpdater.updateSubscribers(fooConfig); - assertEquals(1, rpcServer.responses); + private void configUpdatedSendResponse(RawConfig config) { + clientUpdater.updateSubscribers(config); + } + + private RawConfig createConfigWithNextConfigGeneration(RawConfig config) { + return createConfigWithNextConfigGeneration(config, config.getGeneration() + 1); + } - delayedResponses.add(new DelayedResponse(JRTServerConfigRequestV3.createFromRequest(JRTConfigRequestFactory.createFromRaw(fooConfig, -10L).getRequest()))); - clientUpdater.updateSubscribers(fooConfigOldGeneration); - // Old config generation, so not returned - assertEquals(1, rpcServer.responses); + private RawConfig createConfigWithNextConfigGeneration(RawConfig config, long newConfigGeneration) { + final int errorCode = 0; + return ProxyServerTest.createConfigWithNextConfigGeneration(config, errorCode, ProxyServerTest.fooConfig.getPayload(), newConfigGeneration); } } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index 513a5caa08d..22488da7c80 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. 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.config.subscription.ConfigSourceSet; import com.yahoo.vespa.config.*; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import com.yahoo.vespa.config.protocol.Payload; @@ -220,9 +219,13 @@ public class ProxyServerTest { } private static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode, Payload payload) { + return createConfigWithNextConfigGeneration(config, errorCode, payload, config.getGeneration() + 1); + } + + static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode, Payload payload, long configGeneration) { return new RawConfig(config.getKey(), config.getDefMd5(), payload, config.getConfigMd5(), - config.getGeneration() + 1, false, + configGeneration, false, errorCode, config.getDefContent(), Optional.empty()); } |