summaryrefslogtreecommitdiffstats
path: root/config
diff options
context:
space:
mode:
authorHenning Baldersheim <balder@yahoo-inc.com>2018-02-23 09:43:04 +0100
committerHenning Baldersheim <balder@yahoo-inc.com>2018-03-05 12:29:45 +0000
commit251243a8b326149c6f96ce32bd94d8375ce0ce44 (patch)
tree5a177113043757d6847f37579cc18fab7fdeef31 /config
parent4483a401ae03ccad8b581585aecd0c45cdea0d36 (diff)
Revert "Revert "Balder/remove config race""
Diffstat (limited to 'config')
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java2
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java20
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/ConfigSetSubscription.java27
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java133
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java21
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java3
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java31
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java4
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java15
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java8
-rw-r--r--config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java9
-rwxr-xr-xconfig/src/main/java/com/yahoo/vespa/config/RawConfig.java3
-rw-r--r--config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java6
-rw-r--r--config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java41
-rw-r--r--config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java9
-rw-r--r--config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java2
16 files changed, 171 insertions, 163 deletions
diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java b/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java
index 6b8059ad5af..bb29e329121 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java
@@ -52,7 +52,7 @@ public class ConfigHandle<T extends ConfigInstance> {
*/
public T getConfig() {
// TODO throw if subscriber not frozen?
- return sub.getConfig();
+ return sub.getConfigState().getConfig();
}
@Override
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 1c9f659b45f..bf8b9372d58 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
@@ -230,7 +230,7 @@ public class ConfigSubscriber {
boolean anyConfigChanged = false;
boolean allGenerationsChanged = true;
boolean allGenerationsTheSame = true;
- Long currentGenChecker = null;
+ Long currentGen = null;
for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) {
h.setChanged(false); // Reset this flag, if it was set, the user should have acted on it the last time this method returned true.
}
@@ -244,10 +244,11 @@ public class ConfigSubscriber {
return false;
}
throwIfExceptionSet(subscription);
- if (currentGenChecker == null) currentGenChecker = subscription.getGeneration();
- if (!currentGenChecker.equals(subscription.getGeneration())) allGenerationsTheSame = false;
- allGenerationsChanged = allGenerationsChanged && subscription.isGenerationChanged();
- if (subscription.isConfigChanged()) anyConfigChanged = true;
+ ConfigSubscription.ConfigState<? extends ConfigInstance> config = subscription.getConfigState();
+ if (currentGen == null) currentGen = config.getGeneration();
+ if (!currentGen.equals(config.getGeneration())) allGenerationsTheSame = false;
+ allGenerationsChanged = allGenerationsChanged && config.isGenerationChanged();
+ if (config.isConfigChanged()) anyConfigChanged = true;
timeLeftMillis = timeLeftMillis - (System.currentTimeMillis() - started);
}
reconfigDue = (anyConfigChanged || !requireChange) && allGenerationsChanged && allGenerationsTheSame;
@@ -258,8 +259,8 @@ public class ConfigSubscriber {
if (reconfigDue) {
// This indicates the clients will possibly reconfigure their services, so "reset" changed-logic in subscriptions.
// Also if appropriate update the changed flag on the handler, which clients use.
- markSubsChangedSeen();
- generation = subscriptionHandles.get(0).subscription().getGeneration();
+ markSubsChangedSeen(currentGen);
+ generation = currentGen;
}
return reconfigDue;
}
@@ -285,11 +286,10 @@ public class ConfigSubscriber {
}
}
- private void markSubsChangedSeen() {
+ private void markSubsChangedSeen(Long requiredGen) {
for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) {
ConfigSubscription<? extends ConfigInstance> sub = h.subscription();
- h.setChanged(sub.isConfigChanged());
- sub.resetChangedFlags();
+ h.setChanged(sub.isConfigChangedAndReset(requiredGen));
}
}
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSetSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSetSubscription.java
index b39ae9e93cc..acd5b823af3 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSetSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSetSubscription.java
@@ -35,22 +35,19 @@ public class ConfigSetSubscription<T extends ConfigInstance> extends ConfigSubsc
public boolean nextConfig(long timeout) {
long end = System.currentTimeMillis() + timeout;
do {
- ConfigInstance myInstance = getNewInstance();
+ T myInstance = getNewInstance();
+ ConfigState<T> configState = getConfigState();
// User forced reload
if (checkReloaded()) {
- updateInstance(myInstance);
+ setConfigIfChanged(myInstance);
return true;
}
- if (!myInstance.equals(config)) {
- generation++;
- updateInstance(myInstance);
+ if (!myInstance.equals(configState.getConfig())) {
+ setConfigIfChangedIncGen(myInstance);
return true;
}
sleep();
} while (System.currentTimeMillis() < end);
- // These shouldn't be checked anywhere since we return false now, but setting them still
- setGenerationChanged(false);
- setConfigChanged(false);
return false;
}
@@ -62,25 +59,17 @@ public class ConfigSetSubscription<T extends ConfigInstance> extends ConfigSubsc
}
}
- @SuppressWarnings("unchecked")
- private void updateInstance(ConfigInstance myInstance) {
- if (!myInstance.equals(config)) {
- setConfigChanged(true);
- }
- setConfig((T) myInstance);
- setGenerationChanged(true);
- }
-
@Override
public boolean subscribe(long timeout) {
return true;
}
- public ConfigInstance getNewInstance() {
+ @SuppressWarnings("unchecked")
+ private T getNewInstance() {
try {
ConfigInstance.Builder builder = set.get(subKey);
Constructor<?> constructor = builder.getClass().getDeclaringClass().getConstructor(builder.getClass());
- return (ConfigInstance) constructor.newInstance(builder);
+ return (T) constructor.newInstance(builder);
} catch (Exception e) {
e.printStackTrace();
throw new RuntimeException(e);
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 398392ba8d3..02c455bf1c3 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
@@ -2,6 +2,8 @@
package com.yahoo.config.subscription.impl;
import java.io.File;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;
import com.yahoo.config.ConfigInstance;
@@ -25,22 +27,43 @@ import com.yahoo.vespa.config.protocol.DefContent;
*/
public abstract class ConfigSubscription<T extends ConfigInstance> {
protected static Logger log = Logger.getLogger(ConfigSubscription.class.getName());
- protected ConfigSubscriber subscriber;
- protected boolean configChanged = false;
- protected boolean generationChanged = false;
- protected volatile T config = null;
- protected Long generation = null;
- protected ConfigKey<T> key;
- protected Class<T> configClass;
+ protected final ConfigSubscriber subscriber;
+ private final AtomicReference<ConfigState<T>> config = new AtomicReference<>();
+ protected final ConfigKey<T> key;
+ protected final Class<T> configClass;
private volatile RuntimeException exception = null;
private State state = State.OPEN;
+
+ public static class ConfigState<T extends ConfigInstance> {
+ private final boolean configChanged;
+ private final boolean generationChanged;
+ private final T config;
+ private final Long generation;
+ private ConfigState(boolean generationChanged, Long generation, boolean configChanged, T config) {
+ this.configChanged = configChanged;
+ this.config = config;
+ this.generationChanged = generationChanged;
+ this.generation = generation;
+ }
+ private ConfigState(Long generation, T config) {
+ this(false, generation,false, config);
+ }
+ private ConfigState() {
+ this(false, 0L, false, null);
+ }
+ private ConfigState<T> createUnchanged() { return new ConfigState<T>(generation, config); }
+ public boolean isConfigChanged() { return configChanged; }
+ public boolean isGenerationChanged() { return generationChanged; }
+ public Long getGeneration() { return generation; }
+ public T getConfig() { return config; }
+ }
/**
* If non-null: The user has set this generation explicitly. nextConfig should take this into account.
* Access to these variables _must_ be synchronized, as nextConfig and reload() is likely to be run from
* independent threads.
*/
- private boolean doReload = false;
- private long reloadedGeneration = -1;
+
+ private final AtomicReference<Long> reloadedGeneration = new AtomicReference<>();
enum State {
OPEN, CLOSED
@@ -56,6 +79,7 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
this.key = key;
this.configClass = key.getConfigClass();
this.subscriber = subscriber;
+ this.config.set(new ConfigState<T>());
}
@@ -66,7 +90,8 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
* @param subscriber the subscriber for this subscription
* @return a subclass of a ConfigsSubscription
*/
- public static <T extends ConfigInstance> ConfigSubscription<T> get(ConfigKey<T> key, ConfigSubscriber subscriber, ConfigSource source, TimingValues timingValues) {
+ public static <T extends ConfigInstance> ConfigSubscription<T> get(ConfigKey<T> key, ConfigSubscriber subscriber,
+ ConfigSource source, TimingValues timingValues) {
String configId = key.getConfigId();
if (source instanceof RawSource || configId.startsWith("raw:")) return getRawSub(key, subscriber, source);
if (source instanceof FileSource || configId.startsWith("file:")) return getFileSub(key, subscriber, source);
@@ -129,50 +154,54 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
return false;
}
- void setConfigChanged(boolean changed) {
- this.configChanged = changed;
- }
-
- void setGenerationChanged(boolean genChanged) {
- this.generationChanged = genChanged;
- }
/**
* Called from {@link ConfigSubscriber} when the changed status of this config is propagated to the clients
*/
- public void resetChangedFlags() {
- setConfigChanged(false);
- setGenerationChanged(false);
+ public boolean isConfigChangedAndReset(Long requiredGen) {
+ ConfigState<T> prev = config.get();
+ while ( prev.getGeneration().equals(requiredGen) && !config.compareAndSet(prev, prev.createUnchanged())) {
+ prev = config.get();
+ }
+ // A false positive is a lot better than a false negative
+ return !prev.getGeneration().equals(requiredGen) || prev.isConfigChanged();
}
- public boolean isConfigChanged() {
- return configChanged;
+ void setConfig(Long generation, T config) {
+ this.config.set(new ConfigState<>(true, generation, true, config));
}
- public boolean isGenerationChanged() {
- return generationChanged;
+ // Only used by {@link FileConfigSubscription}
+ protected void setConfigIncGen(T config) {
+ ConfigState<T> prev = this.config.get();
+ setConfig(prev.getGeneration() + 1, config);
}
- void setConfig(T config) {
- this.config = config;
+ // Only used by {@link FileConfigSubscription} and {@link ConfigSetSubscription}
+ protected void setConfigIfChangedIncGen(T config) {
+ ConfigState<T> prev = this.config.get();
+ this.config.set(new ConfigState<>(true, prev.getGeneration() + 1,
+ !config.equals(prev.getConfig()), config));
+ }
+ protected void setConfigIfChanged(T config) {
+ ConfigState<T> prev = this.config.get();
+ this.config.set(new ConfigState<>(true, prev.getGeneration(),
+ !config.equals(prev.getConfig()), config));
}
- /**
- * The config object of this subscription
- *
- * @return the ConfigInstance (the config) of this subscription
- */
- public T getConfig() {
- return config;
+ void setGeneration(Long generation) {
+ ConfigState<T> c = config.get();
+ this.config.set(new ConfigState<>(true, generation, c.isConfigChanged(), c.getConfig()));
}
/**
- * The generation of this subscription
+ * The config state object of this subscription
*
- * @return the generation of this subscription
+ * @return the ConfigInstance (the config) of this subscription
*/
- public Long getGeneration() {
- return generation;
+
+ public ConfigState<T> getConfigState() {
+ return config.get();
}
/**
@@ -183,16 +212,13 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
return configClass;
}
- void setGeneration(Long generation) {
- this.generation = generation;
- }
-
@Override
public String toString() {
StringBuilder s = new StringBuilder(key.toString());
- s.append(", Current generation: ").append(generation)
- .append(", Generation changed: ").append(generationChanged)
- .append(", Config changed: ").append(configChanged);
+ ConfigState<T> c = config.get();
+ s.append(", Current generation: ").append(c.getGeneration())
+ .append(", Generation changed: ").append(c.isGenerationChanged())
+ .append(", Config changed: ").append(c.isConfigChanged());
if (exception != null)
s.append(", Exception: ").append(exception);
return s.toString();
@@ -212,7 +238,7 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
* that can be set by {@link ConfigSubscriber#reload(long)}.
*
* @param timeout in milliseconds
- * @return false if timed out, true if the state of {@link #configChanged}, {@link #generationChanged} or {@link #exception} changed. If true, the {@link #config} field will be set also.
+ * @return false if timed out, true if generation or config or {@link #exception} changed. If true, the {@link #config} field will be set also.
* has changed
*/
public abstract boolean nextConfig(long timeout);
@@ -277,23 +303,20 @@ public abstract class ConfigSubscription<T extends ConfigInstance> {
* Force this into the given generation, used in testing
* @param generation a config generation
*/
- public synchronized void reload(long generation) {
- this.doReload = true;
- this.reloadedGeneration = generation;
+ public void reload(long generation) {
+ reloadedGeneration.set(generation);
}
/**
* True if someone has set the {@link #reloadedGeneration} number by calling {@link #reload(long)}
* and hence wants to force a given generation programmatically. If that is the case,
- * sets the {@link #generation} and {@link #generationChanged} fields accordingly.
+ * sets the generation and flags it as changed accordingly.
* @return true if {@link #reload(long)} has been called, false otherwise
*/
- protected synchronized boolean checkReloaded() {
- if (doReload) {
- // User has called reload
- generation = reloadedGeneration;
- setGenerationChanged(true);
- doReload = false;
+ protected boolean checkReloaded() {
+ Long reloaded = reloadedGeneration.getAndSet(null);
+ if (reloaded != null) {
+ setGeneration(reloaded);
return true;
}
return false;
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java
index 60c702895f8..8c3f87d0702 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java
@@ -40,15 +40,14 @@ public class FileConfigSubscription<T extends ConfigInstance> extends ConfigSubs
if (checkReloaded()) {
log.log(LogLevel.DEBUG, "User forced config reload at " + System.currentTimeMillis());
// User forced reload
- updateConfig();
- log.log(LogLevel.DEBUG, "Config updated at " + System.currentTimeMillis() + ", changed: " + isConfigChanged());
- log.log(LogLevel.DEBUG, "Config: " + config.toString());
+ setConfigIfChanged(updateConfig());
+ ConfigState<T> configState = getConfigState();
+ log.log(LogLevel.DEBUG, "Config updated at " + System.currentTimeMillis() + ", changed: " + configState.isConfigChanged());
+ log.log(LogLevel.DEBUG, "Config: " + configState.getConfig().toString());
return true;
}
if (file.lastModified()!=ts) {
- updateConfig();
- generation++;
- setGenerationChanged(true);
+ setConfigIncGen(updateConfig());
return true;
}
try {
@@ -56,22 +55,18 @@ public class FileConfigSubscription<T extends ConfigInstance> extends ConfigSubs
} catch (InterruptedException e) {
throw new ConfigInterruptedException(e);
}
- // These shouldn't be checked anywhere since we return false now, but setting them still
- setGenerationChanged(false);
- setConfigChanged(false);
+
return false;
}
- private void updateConfig() {
+ private T updateConfig() {
ts=file.lastModified();
- ConfigInstance prev = config;
try {
ConfigPayload payload = new CfgConfigPayloadBuilder().deserialize(Arrays.asList(IOUtils.readFile(file).split("\n")));
- config = payload.toInstance(configClass, key.getConfigId());
+ return payload.toInstance(configClass, key.getConfigId());
} catch (IOException e) {
throw new ConfigurationRuntimeException(e);
}
- setConfigChanged(!config.equals(prev));
}
@Override
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
index 7ef94e98cc8..a33c1556dd3 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
@@ -10,6 +10,7 @@ import com.yahoo.config.subscription.ConfigSource;
import com.yahoo.config.subscription.ConfigSourceSet;
import com.yahoo.config.subscription.ConfigSubscriber;
import com.yahoo.vespa.config.ConfigKey;
+import com.yahoo.vespa.config.RawConfig;
import com.yahoo.vespa.config.TimingValues;
/**
@@ -43,7 +44,7 @@ public class GenericConfigSubscriber extends ConfigSubscriber {
* @param timingValues {@link TimingValues}
* @return generic handle
*/
- public GenericConfigHandle subscribe(ConfigKey<?> key, List<String> defContent, ConfigSource source, TimingValues timingValues) {
+ public GenericConfigHandle subscribe(ConfigKey<RawConfig> key, List<String> defContent, ConfigSource source, TimingValues timingValues) {
checkStateBeforeSubscribe();
GenericJRTConfigSubscription sub = new GenericJRTConfigSubscription(key, defContent, this, source, timingValues);
GenericConfigHandle handle = new GenericConfigHandle(sub);
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java
index ab53ce6dc50..068b24cdf1a 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java
@@ -3,7 +3,6 @@ package com.yahoo.config.subscription.impl;
import java.util.List;
-import com.yahoo.config.ConfigInstance;
import com.yahoo.config.subscription.ConfigSource;
import com.yahoo.config.subscription.ConfigSubscriber;
import com.yahoo.log.LogLevel;
@@ -19,27 +18,22 @@ import com.yahoo.vespa.config.protocol.JRTClientConfigRequest;
* @author vegardh
*
*/
-@SuppressWarnings("rawtypes")
-public class GenericJRTConfigSubscription extends JRTConfigSubscription {
+public class GenericJRTConfigSubscription extends JRTConfigSubscription<RawConfig> {
- private RawConfig config;
private final List<String> defContent;
- @SuppressWarnings("unchecked")
- public GenericJRTConfigSubscription(ConfigKey<?> key,
- List<String> defContent,
- ConfigSubscriber subscriber,
- ConfigSource source,
- TimingValues timingValues) {
+ public GenericJRTConfigSubscription(ConfigKey<RawConfig> key, List<String> defContent, ConfigSubscriber subscriber,
+ ConfigSource source, TimingValues timingValues)
+ {
super(key, subscriber, source, timingValues);
this.defContent = defContent;
}
@Override
protected void setNewConfig(JRTClientConfigRequest jrtReq) {
- this.config = RawConfig.createFromResponseParameters(jrtReq);
+ setConfig(jrtReq.getNewGeneration(), RawConfig.createFromResponseParameters(jrtReq) );
if (log.isLoggable(LogLevel.DEBUG)) {
- log.log(LogLevel.DEBUG, "in setNewConfig, config=" + this.config);
+ log.log(LogLevel.DEBUG, "in setNewConfig, config=" + this.getConfigState().getConfig());
}
}
@@ -48,13 +42,15 @@ public class GenericJRTConfigSubscription extends JRTConfigSubscription {
@Override
void setGeneration(Long generation) {
super.setGeneration(generation);
- if (this.config != null) {
- this.config.setGeneration(generation);
+ ConfigState<RawConfig> configState = getConfigState();
+
+ if (configState.getConfig() != null) {
+ configState.getConfig().setGeneration(generation);
}
}
public RawConfig getRawConfig() {
- return config;
+ return getConfigState().getConfig();
}
/**
@@ -66,9 +62,4 @@ public class GenericJRTConfigSubscription extends JRTConfigSubscription {
public DefContent getDefContent() {
return (DefContent.fromList(defContent));
}
-
- @Override
- public ConfigInstance getConfig() {
- return null;
- }
}
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 7d830484edd..88af414e28d 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
@@ -163,7 +163,7 @@ public class JRTConfigRequester implements RequestWaiter {
}
private void handleFailedRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, Connection connection) {
- final boolean configured = (sub.getConfig() != null);
+ final boolean configured = (sub.getConfigState().getConfig() != null);
if (configured) {
// The subscription object has an "old" config, which is all we have to offer back now
log.log(LogLevel.INFO, "Failure of config subscription, clients will keep existing config until resolved: " + sub);
@@ -226,7 +226,7 @@ public class JRTConfigRequester implements RequestWaiter {
if (sub.getState() != ConfigSubscription.State.OPEN) return;
fatalFailures++;
// The logging depends on whether we are configured or not.
- Level logLevel = sub.getConfig() == null ? LogLevel.DEBUG : LogLevel.INFO;
+ Level logLevel = sub.getConfigState().getConfig() == null ? LogLevel.DEBUG : LogLevel.INFO;
String logMessage = "Request for config " + jrtReq.getShortDescription() + "' failed with error code " +
jrtReq.errorCode() + " (" + jrtReq.errorMessage() + "), scheduling new connect " +
" in " + delay + " ms";
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 6adeaa80cd1..9ae3e226d89 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
@@ -49,7 +49,8 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
public boolean nextConfig(long timeoutMillis) {
// These flags may have been left true from a previous call, since ConfigSubscriber's nextConfig
// not necessarily returned true and reset the flags then
- boolean gotNew = isGenerationChanged() || isConfigChanged() || hasException();
+ ConfigState<T> configState = getConfigState();
+ boolean gotNew = configState.isGenerationChanged() || configState.isConfigChanged() || hasException();
// Return that now, if there's nothing in queue, so that ConfigSubscriber can move on to other subscriptions to check
if (getReqQueue().peek()==null && gotNew) {
return true;
@@ -60,7 +61,8 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
// there is a race here. However: the caller will handle it no matter what it gets from the queue here,
// the important part is that local state on the subscription objects is preserved.
if (!pollQueue(timeoutMillis)) return gotNew;
- gotNew = isGenerationChanged() || isConfigChanged() || hasException();
+ configState = getConfigState();
+ gotNew = configState.isGenerationChanged() || configState.isConfigChanged() || hasException();
return gotNew;
}
@@ -83,20 +85,17 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
return false;
}
if (jrtReq.hasUpdatedGeneration()) {
- //printStatus(jrtReq, "Updated generation or config");
- setGeneration(jrtReq.getNewGeneration());
- setGenerationChanged(true);
if (jrtReq.hasUpdatedConfig()) {
- // payload changed
setNewConfig(jrtReq);
- setConfigChanged(true);
+ } else {
+ setGeneration(jrtReq.getNewGeneration());
}
}
return true;
}
protected void setNewConfig(JRTClientConfigRequest jrtReq) {
- setConfig(toConfigInstance(jrtReq));
+ setConfig(jrtReq.getNewGeneration(), toConfigInstance(jrtReq) );
}
/**
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java
index 2b656b0f498..4870d69bb23 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java
@@ -53,6 +53,7 @@ public class JarConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
}
zipEntry = getEntry(jarFile, path);
if (zipEntry==null) throw new IllegalArgumentException("Config '" + key.getName() + "' not found in '" + jarName + "!/" + path + "'.");
+ T config = null;
try {
ConfigPayload payload = new CfgConfigPayloadBuilder().deserialize(Arrays.asList(IOUtils.readAll(new InputStreamReader(jarFile.getInputStream(zipEntry), "UTF-8")).split("\n")));
config = payload.toInstance(configClass, key.getConfigId());
@@ -61,9 +62,7 @@ public class JarConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
} catch (IOException e) {
throw new ConfigurationRuntimeException(e);
}
- setGeneration(0L);
- setGenerationChanged(true);
- setConfigChanged(true);
+ setConfig(0L, config);
try {
jarFile.close();
} catch (IOException e) {
@@ -77,9 +76,6 @@ public class JarConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
} catch (InterruptedException e) {
throw new ConfigInterruptedException(e);
}
- // These shouldn't be checked anywhere since we return false now, but setting them still
- setGenerationChanged(false);
- setConfigChanged(false);
return false;
}
/**
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java
index ce82b62cfd2..0d3f04361aa 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java
@@ -35,12 +35,8 @@ public class RawConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
}
if (payload==null) {
payload = inputPayload;
- setGeneration(0L);
- setGenerationChanged(true);
- setConfigChanged(true);
-
ConfigPayload configPayload = new CfgConfigPayloadBuilder().deserialize(Arrays.asList(payload.split("\n")));
- config = configPayload.toInstance(configClass, key.getConfigId());
+ setConfig(0L, configPayload.toInstance(configClass, key.getConfigId()));
return true;
}
try {
@@ -48,9 +44,6 @@ public class RawConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
} catch (InterruptedException e) {
throw new ConfigInterruptedException(e);
}
- // These shouldn't be checked anywhere since we return false now, but setting them still
- setGenerationChanged(false);
- setConfigChanged(false);
return false;
}
diff --git a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
index 898e8edfb08..75c5161103e 100755
--- a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
+++ b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
@@ -1,6 +1,7 @@
// 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;
+import com.yahoo.config.ConfigInstance;
import com.yahoo.text.Utf8String;
import com.yahoo.vespa.config.protocol.CompressionInfo;
import com.yahoo.vespa.config.protocol.JRTClientConfigRequest;
@@ -20,7 +21,7 @@ import java.util.Optional;
*
* @author hmusum
*/
-public class RawConfig {
+public class RawConfig extends ConfigInstance {
private final ConfigKey<?> key;
private final String defMd5;
diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java
index 4ba6a74ce6f..3d7c0dc1e80 100644
--- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java
+++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java
@@ -2,6 +2,7 @@
package com.yahoo.vespa.config.protocol;
import com.yahoo.config.ConfigInstance;
+import com.yahoo.config.subscription.impl.ConfigSubscription;
import com.yahoo.config.subscription.impl.JRTConfigSubscription;
import com.yahoo.jrt.Request;
import com.yahoo.text.Utf8Array;
@@ -73,12 +74,13 @@ public class JRTClientConfigRequestV3 extends SlimeClientConfigRequest {
public static <T extends ConfigInstance> JRTClientConfigRequest createFromSub(JRTConfigSubscription<T> sub, Trace trace, CompressionType compressionType, Optional<VespaVersion> vespaVersion) {
String hostname = ConfigUtils.getCanonicalHostName();
ConfigKey<T> key = sub.getKey();
- T i = sub.getConfig();
+ ConfigSubscription.ConfigState<T> configState = sub.getConfigState();
+ T i = configState.getConfig();
return createWithParams(key,
sub.getDefContent(),
hostname,
i != null ? i.getConfigMd5() : "",
- sub.getGeneration() != null ? sub.getGeneration() : 0L,
+ configState.getGeneration() != null ? configState.getGeneration() : 0L,
sub.timingValues().getSubscribeTimeout(),
trace,
compressionType,
diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
index 3862cfc9069..8fdd9d5e3b3 100644
--- a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java
@@ -18,6 +18,8 @@ import java.nio.file.Files;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -48,11 +50,11 @@ public class FileConfigSubscriptionTest {
subscriber,
TEST_TYPES_FILE);
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.intval(), is(23));
+ assertThat(sub.getConfigState().getConfig().intval(), is(23));
Thread.sleep(1000);
writeConfig("intval", "33");
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.intval(), is(33));
+ assertThat(sub.getConfigState().getConfig().intval(), is(33));
}
@Test
@@ -64,18 +66,37 @@ public class FileConfigSubscriptionTest {
subscriber,
TEST_TYPES_FILE);
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.intval(), is(23));
+ assertThat(sub.getConfigState().getConfig().intval(), is(23));
writeConfig("intval", "33");
sub.reload(1);
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.intval(), is(33));
- assertTrue(sub.isConfigChanged());
- assertTrue(sub.isGenerationChanged());
+ ConfigSubscription.ConfigState<SimpletypesConfig> configState = sub.getConfigState();
+ assertThat(configState.getConfig().intval(), is(33));
+ assertTrue(configState.isConfigChanged());
+ assertTrue(configState.isGenerationChanged());
+
+ assertTrue(sub.isConfigChangedAndReset(7L));
+ assertSame(configState, sub.getConfigState());
+ assertTrue(configState.isConfigChanged());
+ assertTrue(configState.isGenerationChanged());
+ assertTrue(sub.isConfigChangedAndReset(1L));
+ assertNotSame(configState, sub.getConfigState());
+ configState = sub.getConfigState();
+ assertFalse(configState.isConfigChanged());
+ assertFalse(configState.isGenerationChanged());
+
sub.reload(2);
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.intval(), is(33));
- assertFalse(sub.isConfigChanged());
- assertTrue(sub.isGenerationChanged());
+ configState = sub.getConfigState();
+ assertThat(configState.getConfig().intval(), is(33));
+ assertFalse(configState.isConfigChanged());
+ assertTrue(configState.isGenerationChanged());
+
+ assertFalse(sub.isConfigChangedAndReset(2L));
+ assertNotSame(configState, sub.getConfigState());
+ configState = sub.getConfigState();
+ assertFalse(configState.isConfigChanged());
+ assertFalse(configState.isGenerationChanged());
}
@Test
@@ -86,7 +107,7 @@ public class FileConfigSubscriptionTest {
ConfigSubscriber subscriber = new ConfigSubscriber();
ConfigSubscription<TestReferenceConfig> sub = ConfigSubscription.get(key, subscriber, new DirSource(new File(cfgDir)), new TimingValues());
assertTrue(sub.nextConfig(1000));
- assertThat(sub.config.configId(), is(cfgId));
+ assertThat(sub.getConfigState().getConfig().configId(), is(cfgId));
}
@Test(expected = IllegalArgumentException.class)
diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
index 402c3b9b23f..1ad66cbaf58 100644
--- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java
@@ -181,8 +181,7 @@ public class JRTConfigRequesterTest {
ConfigSubscriber subscriber = new ConfigSubscriber();
final TimingValues timingValues = getTestTimingValues();
JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues);
- sub.setConfig(config());
- sub.setGeneration(1L);
+ sub.setConfig(1L, config());
final MockConnection connection = new MockConnection(new ErrorResponseHandler());
JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
@@ -210,8 +209,7 @@ public class JRTConfigRequesterTest {
ConfigSubscriber subscriber = new ConfigSubscriber();
final TimingValues timingValues = getTestTimingValues();
JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues);
- sub.setConfig(config());
- sub.setGeneration(1L);
+ sub.setConfig(1L, config());
final MockConnection connection = new MockConnection(new ErrorResponseHandler(com.yahoo.jrt.ErrorCode.TIMEOUT));
JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
@@ -226,8 +224,7 @@ public class JRTConfigRequesterTest {
ConfigSubscriber subscriber = new ConfigSubscriber();
final TimingValues timingValues = getTestTimingValues();
JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues);
- sub.setConfig(config());
- sub.setGeneration(1L);
+ sub.setConfig(1L, config());
final MockConnection connection = new MockConnection(new ErrorResponseHandler(ErrorCode.UNKNOWN_DEFINITION));
JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues);
diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java
index 7fa205fef15..1f5274b832b 100644
--- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java
+++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java
@@ -249,7 +249,7 @@ public abstract class JRTConfigRequestBase {
assertTrue(sub.nextConfig(120_0000));
sub.close();
JRTClientConfigRequest nextReq = createReq(sub, Trace.createNew());
- SimpletypesConfig config = sub.getConfig();
+ SimpletypesConfig config = sub.getConfigState().getConfig();
assertThat(nextReq.getRequestConfigMd5(), is(config.getConfigMd5()));
assertThat(nextReq.getRequestGeneration(), is(currentGeneration));
System.setProperty("VESPA_CONFIG_PROTOCOL_VERSION", "");