diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-02-15 13:21:48 +0100 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2018-02-15 13:21:48 +0100 |
commit | f44e03ecc977708bd7ee76712a165414c5b192d1 (patch) | |
tree | 8d062352bd6aa1bfedf8458d15d555192fb1586c /config | |
parent | 94f774252d213ca5f99ee8f5d09d53edf7fa5c63 (diff) |
Encapsulate the configstate in one immutable object and remove the interfaces that could produce a race.
Diffstat (limited to 'config')
14 files changed, 125 insertions, 136 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..5873d726857 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -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 (currentGenChecker == null) currentGenChecker = config.getGeneration(); + if (!currentGenChecker.equals(config.getGeneration())) allGenerationsTheSame = false; + allGenerationsChanged = allGenerationsChanged && config.isGenerationChanged(); + if (config.isConfigChanged()) anyConfigChanged = true; timeLeftMillis = timeLeftMillis - (System.currentTimeMillis() - started); } reconfigDue = (anyConfigChanged || !requireChange) && allGenerationsChanged && allGenerationsTheSame; @@ -259,7 +260,7 @@ public class ConfigSubscriber { // 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(); + generation = currentGenChecker; } return reconfigDue; } @@ -288,8 +289,7 @@ public class ConfigSubscriber { private void markSubsChangedSeen() { for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) { ConfigSubscription<? extends ConfigInstance> sub = h.subscription(); - h.setChanged(sub.isConfigChanged()); - sub.resetChangedFlags(); + h.setChanged(sub.isConfigChangedAndReset()); } } 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..c09dc4d2dd7 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 @@ -36,21 +36,19 @@ public class ConfigSetSubscription<T extends ConfigInstance> extends ConfigSubsc long end = System.currentTimeMillis() + timeout; do { ConfigInstance myInstance = getNewInstance(); + ConfigState<T> configState = getConfigState(); + Long nextGen = configState.getGeneration() + 1; // User forced reload if (checkReloaded()) { - updateInstance(myInstance); + updateInstance(nextGen, myInstance); return true; } - if (!myInstance.equals(config)) { - generation++; - updateInstance(myInstance); + if (!myInstance.equals(configState.getConfig())) { + updateInstance(nextGen, 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; } @@ -63,12 +61,8 @@ 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); + private void updateInstance(Long generation, ConfigInstance myInstance) { + setConfigIfChanged(generation, (T)myInstance); } @Override 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..4573f4bdbc2 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,49 @@ 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) { + configChanged = false; + generationChanged = false; + this.generation = generation; + this.config = config; + } + private ConfigState() { + generation = 0L; + config = null; + configChanged = false; + generationChanged = false; + } + private ConfigState createUnchanged() { return new ConfigState(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 +85,7 @@ public abstract class ConfigSubscription<T extends ConfigInstance> { this.key = key; this.configClass = key.getConfigClass(); this.subscriber = subscriber; + this.config.set(new ConfigState<T>()); } @@ -129,50 +159,39 @@ 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() { + ConfigState<T> prev = config.get(); + while ( !config.compareAndSet(prev, prev.createUnchanged())) { + prev = config.get(); + } + return 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; + void setConfigIfChanged(Long generation, T config) { + this.config.set(new ConfigState<>(true, generation, !config.equals(this.config.get().getConfig()), config)); } - void setConfig(T config) { - this.config = config; + void setGeneration(Long generation) { + ConfigState<T> c = config.get(); + this.config.set(new ConfigState<>(true, generation, c.isConfigChanged(), c.getConfig())); } /** - * The config object of this subscription + * The config state object of this subscription * * @return the ConfigInstance (the config) of this subscription */ - public T getConfig() { - return config; - } - /** - * The generation of this subscription - * - * @return the generation of this subscription - */ - public Long getGeneration() { - return generation; + public ConfigState<T> getConfigState() { + return config.get(); } /** @@ -183,16 +202,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 +228,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 +293,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..e2e7bc1d2a0 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 @@ -37,18 +37,19 @@ public class FileConfigSubscription<T extends ConfigInstance> extends ConfigSubs @Override public boolean nextConfig(long timeout) { if (!file.exists() && !file.isFile()) throw new IllegalArgumentException("Not a file: "+file); + Long nextGen = getConfigState().getGeneration() + 1; 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()); + T newConfig = updateConfig(); + setConfigIfChanged(nextGen, 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); + setConfig(nextGen, updateConfig()); return true; } try { @@ -56,22 +57,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/GenericJRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java index ab53ce6dc50..95be992163d 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 @@ -37,6 +37,7 @@ public class GenericJRTConfigSubscription extends JRTConfigSubscription { @Override protected void setNewConfig(JRTClientConfigRequest jrtReq) { + setConfig(jrtReq.getNewGeneration(), null ); this.config = RawConfig.createFromResponseParameters(jrtReq); if (log.isLoggable(LogLevel.DEBUG)) { log.log(LogLevel.DEBUG, "in setNewConfig, config=" + this.config); @@ -66,9 +67,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/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..14669e969ca 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 @@ -48,11 +48,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 +64,20 @@ 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()); 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()); } @Test @@ -86,7 +88,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", ""); |