Merge pull request #9251 from vespa-engine/hmusum/handle-getting-new-config-after-getting-empty-config

Handle getting new config after getting empty config

3 files changed, 37 insertions, 2 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 0fe7fe01467..80bca85d9e4 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
@@ -40,7 +40,8 @@ public class MemoryCache {
      * @param config config to put in cache
      */
     public void put(RawConfig config) {
-        if (config.isError()) return;
+        // Do not cache errors or empty configs (which have generation 0)
+        if (config.isError() || config.getGeneration() == 0) 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/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java
index b4eda05fde4..9306220f518 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
@@ -132,7 +132,7 @@ class RpcConfigSourceClient implements ConfigSourceClient {
                     ret = cachedConfig;
                 }
             }
-            if (!cachedConfig.isError()) {
+            if (!cachedConfig.isError() && cachedConfig.getGeneration() > 0) {
                 needToGetConfig = false;
             }
         }
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 22488da7c80..e21df82a036 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
@@ -171,6 +171,40 @@ public class ProxyServerTest {
         assertEquals(1, cacheManager.size());
     }
 
+    /**
+     * 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.
+     */
+    @Test
+    public void testNoCachingOfEmptyConfig() {
+        ConfigTester tester = new ConfigTester();
+        // 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));
+        assertNotNull(res.getPayload());
+        assertThat(res.getPayload().toString(), is(emptyConfig.getPayload().toString()));
+        assertEquals(0, cache.size());
+
+        // Put a version of the same config into backend with new generation and see that it now works (i.e. we are
+        // not getting a cached response (of the error in the previous request)
+        source.put(fooConfig.getKey(), createConfigWithNextConfigGeneration(fooConfig, 0));
+
+        // Verify that we get the config now and that it is cached
+        res = proxy.resolveConfig(tester.createRequest(fooConfig));
+        assertNotNull(res.getPayload().getData());
+        assertThat(res.getPayload().toString(), is(ConfigTester.fooPayload.toString()));
+        assertEquals(1, cache.size());
+    }
+
     @Test
     public void testReconfiguration() {
         ConfigTester tester = new ConfigTester();