From 2a682aeabfae08df0532fa9407deec0302932711 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Tue, 19 Apr 2022 12:21:52 +0200
Subject: Warn if config updated without generation updated

Always add response to response queue for subscription. Add warnings
if config changes without generation and of config state has an
unexpected change.
---
 .../yahoo/config/subscription/impl/ConfigSubscription.java |  5 +++--
 .../yahoo/config/subscription/impl/JRTConfigRequester.java |  4 +---
 .../config/subscription/impl/JRTConfigSubscription.java    | 14 +++++++++++++-
 3 files changed, 17 insertions(+), 6 deletions(-)

(limited to 'config')

diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
index 46961b2ffc3..5d720a74b15 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java
@@ -204,10 +204,11 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
         if (previousConfig != null && configChanged) {
             SnippetGenerator generator = new SnippetGenerator();
             int sizeHint = 500;
-            log.log(Level.INFO, "Config has changed unexpectedly for " + key + ", generation " + generation +
+            log.log(Level.WARNING, "Config has changed unexpectedly for " + key + ", generation " + generation +
                     ", config in state :" + generator.makeSnippet(previousConfig.toString(), sizeHint) + ", new config: " +
                     generator.makeSnippet(config.toString(), sizeHint) +
-                    ". This might happen if a feature flag is changed when upgrading config servers (either in code or with overrides)");
+                    ". This likely happened because config changed on a previous generation" +
+                    ", look for earlier entry in log with warning about config changing without a change in config generation.");
         }
         this.config.set(new ConfigState<>(true, generation, applyOnRestart, configChanged, config, payloadChecksums));
     }
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 604c85555db..e86948fb3ae 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
@@ -194,9 +194,7 @@ public class JRTConfigRequester implements RequestWaiter {
         failures = 0;
         sub.setLastCallBackOKTS(Instant.now());
         log.log(FINE, () -> "OK response received in handleOkRequest: " + jrtReq);
-        if (jrtReq.hasUpdatedGeneration()) {
-            sub.updateConfig(jrtReq);
-        }
+        sub.updateConfig(jrtReq);
         scheduleNextRequest(jrtReq, sub, calculateSuccessDelay(), calculateSuccessTimeout());
     }
 
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 abfbf2c6c79..653df26bb53 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
@@ -4,6 +4,7 @@ package com.yahoo.config.subscription.impl;
 import com.yahoo.config.ConfigInstance;
 import com.yahoo.config.ConfigurationRuntimeException;
 import com.yahoo.config.subscription.ConfigInterruptedException;
+import com.yahoo.text.internal.SnippetGenerator;
 import com.yahoo.vespa.config.ConfigKey;
 import com.yahoo.vespa.config.ConfigPayload;
 import com.yahoo.vespa.config.TimingValues;
@@ -16,6 +17,7 @@ import java.time.Instant;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
 
 import static com.yahoo.vespa.config.PayloadChecksum.Type.MD5;
 import static java.util.logging.Level.FINE;
@@ -63,6 +65,16 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
             } else {
                 setNewConfigAndGeneration(response);
             }
+        } else if (response.hasUpdatedConfig()) {
+            SnippetGenerator generator = new SnippetGenerator();
+            int sizeHint = 500;
+            T config = toConfigInstance(response);
+            log.log(Level.WARNING, "Config " + key + " has changed without a change in config generation: generation " +
+                    response.getNewGeneration() + ", config: " + generator.makeSnippet(config.toString(), sizeHint) +
+                    ". This might happen when a newly upgraded config server responds with different config when bootstrapping  " +
+                    " (changes to code generating config that are different between versions) or non-deterministic config generation" +
+                    " (e.g. when using collections with non-deterministic iteration order)");
+            setNewConfigAndGeneration(response);
         }
 
         return newConfigOrException();
@@ -141,7 +153,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
         return configInstance;
     }
 
-    // Called by JRTConfigRequester when there is a config with new generation for this subscription
+    // Called by JRTConfigRequester when there is a config response for this subscription
     void updateConfig(JRTClientConfigRequest jrtReq) {
         if ( ! responseQueue.offer(jrtReq))
             setException(new ConfigurationRuntimeException("Failed offering returned request to queue of subscription " + this));
-- 
cgit v1.2.3