Merge pull request #13779 from vespa-engine/bratseth/healt-check-improvements

Bratseth/healt check improvements

3 files changed, 32 insertions, 18 deletions

diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java
index 9710ee607eb..82d02ec89f9 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java
@@ -22,9 +22,9 @@ public class ConfigInstanceUtil {
      * Values that have not been explicitly set in the source builder, will be left unchanged
      * in the destination.
      *
-     * @param destination The builder to copy values into.
-     * @param source The builder to copy values from. Unset values are not copied.
-     * @param <BUILDER> The builder class.
+     * @param destination the builder to copy values into
+     * @param source the builder to copy values from. Unset values are not copied
+     * @param <BUILDER> the builder class
      */
     public static<BUILDER extends ConfigBuilder> void setValues(BUILDER destination, BUILDER source) {
         try {
@@ -56,9 +56,9 @@ public class ConfigInstanceUtil {
             setConfigId(i, configId);
 
         } catch (InstantiationException | InvocationTargetException | NoSuchMethodException |
-                NoSuchFieldException | IllegalAccessException e) {
+                 NoSuchFieldException | IllegalAccessException e) {
             throw new IllegalArgumentException("Failed creating new instance of '" + type.getCanonicalName() +
-                    "' for config id '" + configId + "': " + Exceptions.toMessageString(e), e);
+                                               "' for config id '" + configId + "'", e);
         }
         return instance;
     }
diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
index 814222989a1..5fcbd4f8c21 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
@@ -423,21 +423,35 @@ public class ConfigSubscriber implements AutoCloseable {
      * @return The handle of the config
      * @see #startConfigThread(Runnable)
      */
-    public <T extends ConfigInstance> ConfigHandle<T> subscribe(final SingleSubscriber<T> singleSubscriber, Class<T> configClass, String configId) {
-        if (!subscriptionHandles.isEmpty())
-            throw new IllegalStateException("Can not start single-subscription because subscriptions were previously opened on this.");
-        final ConfigHandle<T> handle = subscribe(configClass, configId);
-        if (!nextConfig())
-            throw new ConfigurationRuntimeException("Initial config of " + configClass.getName() + " failed.");
+    public <T extends ConfigInstance> ConfigHandle<T> subscribe(SingleSubscriber<T> singleSubscriber, Class<T> configClass, String configId) {
+        if ( ! subscriptionHandles.isEmpty())
+            throw new IllegalStateException("Can not start single-subscription because subscriptions were previously opened on this");
+
+        ConfigHandle<T> handle = subscribe(configClass, configId);
+
+        if ( ! nextConfig())
+            throw new ConfigurationRuntimeException("Initial config of " + configClass.getName() + " failed");
+
         singleSubscriber.configure(handle.getConfig());
         startConfigThread(() -> {
                 while (!isClosed()) {
+                    boolean hasNewConfig = false;
+
                     try {
-                        if (nextConfig()) {
-                            if (handle.isChanged()) singleSubscriber.configure(handle.getConfig());
-                        }
-                    } catch (Exception e) {
-                        log.log(SEVERE, "Exception from config system, continuing config thread: " + Exceptions.toMessageString(e));
+                        hasNewConfig = nextConfig();
+                    }
+                    catch (Exception e) {
+                        log.log(SEVERE, "Exception on receiving config. Ignoring this change.", e);
+                    }
+
+                    try {
+                        if (hasNewConfig)
+                            singleSubscriber.configure(handle.getConfig());
+                    }
+                    catch (Exception e) {
+                        log.warning("Exception on applying config " + configClass.getName() +
+                                    " for config id " + configId + ": Ignoring this change: " +
+                                    Exceptions.toMessageString(e));
                     }
                 }
             }
diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java
index 3bdaee09eaf..fa24988709a 100644
--- a/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java
@@ -14,7 +14,6 @@ import java.io.File;
 import java.util.Arrays;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.containsString;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
@@ -165,7 +164,8 @@ public class ConfigInstanceUtilTest {
             ConfigInstanceUtil.getNewInstance(TestNodefsConfig.class, "id0", ConfigPayload.fromBuilder(payloadBuilder));
             assert(false);
         } catch (IllegalArgumentException e) {
-            assertThat(e.getMessage(), containsString("Failed creating new instance of 'com.yahoo.foo.TestNodefsConfig' for config id 'id0':"));
+            assertEquals("Failed creating new instance of 'com.yahoo.foo.TestNodefsConfig' for config id 'id0'",
+                         e.getMessage());
         }
     }
 }