From c1ff01c4040cc0799135206090d4d2cc613d54f9 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Fri, 3 May 2019 09:36:35 +0200 Subject: Clear cache when getting config with generation 0 (empty config) --- .../java/com/yahoo/vespa/config/proxy/MemoryCache.java | 14 ++++++++++---- .../vespa/config/proxy/UpstreamConfigSubscriber.java | 2 +- .../vespa/config/proxy/ConfigProxyRpcServerTest.java | 4 ++-- .../vespa/config/proxy/DelayedResponseHandlerTest.java | 2 +- .../vespa/config/proxy/MemoryCacheConfigClientTest.java | 2 +- .../com/yahoo/vespa/config/proxy/MemoryCacheTest.java | 8 ++++---- .../vespa/config/proxy/MockConfigSourceClient.java | 2 +- .../com/yahoo/vespa/config/proxy/ProxyServerTest.java | 17 +++++++++++------ 8 files changed, 31 insertions(+), 20 deletions(-) diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java index 80bca85d9e4..112bd1f86de 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java @@ -37,11 +37,17 @@ public class MemoryCache { /** * Put in cache, except when config has an error * - * @param config config to put in cache + * @param config config to update in cache */ - public void put(RawConfig config) { - // Do not cache errors or empty configs (which have generation 0) - if (config.isError() || config.getGeneration() == 0) return; + public void update(RawConfig config) { + // Do not cache errors + if (config.isError()) return; + + // Do not cache empty configs (which have generation 0), remove everything in cache + if (config.getGeneration() == 0) { + cache.clear(); + return; + } log.log(LogLevel.DEBUG, () -> "Putting '" + config + "' into memory cache"); cache.put(new ConfigCacheKey(config.getKey(), config.getDefMd5()), config); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java index 7e27b19bda2..b99b7d679ca 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java @@ -69,7 +69,7 @@ public class UpstreamConfigSubscriber implements Subscriber { log.log(LogLevel.DEBUG, () -> "config to be returned for '" + newConfig.getKey() + "', generation=" + newConfig.getGeneration() + ", payload=" + newConfig.getPayload()); - memoryCache.put(newConfig); + memoryCache.update(newConfig); clientUpdater.updateSubscribers(newConfig); } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java index 4a9d2acb4c5..f32bb2ac024 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java @@ -73,7 +73,7 @@ public class ConfigProxyRpcServerTest { assertThat(ret.length, is(0)); final RawConfig config = ProxyServerTest.fooConfig; - proxyServer.getMemoryCache().put(config); + proxyServer.getMemoryCache().update(config); req = new Request("listCachedConfig"); rpcServer.listCachedConfig(req); assertFalse(req.errorMessage(), req.isError()); @@ -100,7 +100,7 @@ public class ConfigProxyRpcServerTest { assertThat(ret.length, is(0)); final RawConfig config = ProxyServerTest.fooConfig; - proxyServer.getMemoryCache().put(config); + proxyServer.getMemoryCache().update(config); req = new Request("listCachedConfigFull"); rpcServer.listCachedConfigFull(req); assertFalse(req.errorMessage(), req.isError()); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java index 84b3e5dbe95..3ac1af6c4d8 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java @@ -32,7 +32,7 @@ public class DelayedResponseHandlerTest { DelayedResponses delayedResponses = new DelayedResponses(statistics); final MockRpcServer mockRpcServer = new MockRpcServer(); final MemoryCache memoryCache = new MemoryCache(); - memoryCache.put(ConfigTester.fooConfig); + memoryCache.update(ConfigTester.fooConfig); final DelayedResponseHandler delayedResponseHandler = new DelayedResponseHandler(delayedResponses, memoryCache, mockRpcServer); delayedResponses.add(new DelayedResponse(tester.createRequest(ProxyServerTest.fooConfig, 0))); delayedResponses.add(new DelayedResponse(tester.createRequest(ProxyServerTest.fooConfig, 1200000))); // should not be returned yet diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClientTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClientTest.java index 70bb9e45e51..7c040735e13 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClientTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClientTest.java @@ -17,7 +17,7 @@ public class MemoryCacheConfigClientTest { @Test public void basic() { MemoryCache cache = new MemoryCache(); - cache.put(ConfigTester.fooConfig); + cache.update(ConfigTester.fooConfig); MemoryCacheConfigClient client = new MemoryCacheConfigClient(cache); assertThat(client.getConfig(ConfigTester.fooConfig, null), is(ConfigTester.fooConfig)); assertNull(client.getConfig(ConfigTester.barConfig, null)); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java index 16bc2c66a94..485a091d9ae 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java @@ -76,8 +76,8 @@ public class MemoryCacheTest { public void basic() { MemoryCache cache = new MemoryCache(); - cache.put(config); - cache.put(config2); + cache.update(config); + cache.update(config2); assertThat(cache.size(), is(2)); assertTrue(cache.containsKey(cacheKey)); assertTrue(cache.containsKey(cacheKey2)); @@ -101,8 +101,8 @@ public class MemoryCacheTest { public void testSameConfigNameDifferentMd5() { MemoryCache cache = new MemoryCache(); - cache.put(config); - cache.put(configDifferentMd5); // same name, different defMd5 + cache.update(config); + cache.update(configDifferentMd5); // same name, different defMd5 assertThat(cache.size(), is(2)); assertTrue(cache.containsKey(cacheKey)); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSourceClient.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSourceClient.java index 784c82e2f88..5a15ade6968 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSourceClient.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConfigSourceClient.java @@ -25,7 +25,7 @@ public class MockConfigSourceClient implements ConfigSourceClient{ @Override public RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { final RawConfig config = getConfig(input.getKey()); - memoryCache.put(config); + memoryCache.update(config); return config; } 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 e21df82a036..19732e4b5b5 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 @@ -137,7 +137,7 @@ public class ProxyServerTest { /** * Verifies that error responses are not cached. When the config has been successfully retrieved, - * it must be put in the cache. + * it must be updated in cache. */ @Test public void testNoCachingOfErrorRequests() { @@ -175,21 +175,26 @@ public class ProxyServerTest { * Verifies that config with generation 0 (used for empty sentinel config) is not cached. * If it was cached, it would be served even when newer config is available * (as they ask for config, saying that it now has config with generation 0). - * When the config has been successfully retrieved it must be put in the cache. + * When the config has been successfully retrieved it must be updated in cache. */ @Test public void testNoCachingOfEmptyConfig() { ConfigTester tester = new ConfigTester(); + MemoryCache cache = proxy.getMemoryCache(); + + assertEquals(0, cache.size()); + RawConfig res = proxy.resolveConfig(tester.createRequest(fooConfig)); + assertNotNull(res); + assertThat(res.getPayload().toString(), is(ConfigTester.fooPayload.toString())); + assertEquals(1, cache.size()); + // Simulate an empty response RawConfig emptyConfig = new RawConfig(fooConfig.getKey(), fooConfig.getDefMd5(), Payload.from("{}"), fooConfig.getConfigMd5(), 0, false, 0, fooConfig.getDefContent(), Optional.empty()); source.put(fooConfig.getKey(), emptyConfig); - MemoryCache cache = proxy.getMemoryCache(); - assertEquals(0, cache.size()); - - RawConfig res = proxy.resolveConfig(tester.createRequest(fooConfig)); + res = proxy.resolveConfig(tester.createRequest(fooConfig)); assertNotNull(res.getPayload()); assertThat(res.getPayload().toString(), is(emptyConfig.getPayload().toString())); assertEquals(0, cache.size()); -- cgit v1.2.3