diff options
Diffstat (limited to 'config')
5 files changed, 33 insertions, 25 deletions
diff --git a/config/src/tests/subscriber/subscriber.cpp b/config/src/tests/subscriber/subscriber.cpp index 7613ba41234..39a537486cd 100644 --- a/config/src/tests/subscriber/subscriber.cpp +++ b/config/src/tests/subscriber/subscriber.cpp @@ -341,22 +341,22 @@ TEST_FF("requireThatHandlesAreMarkedAsChanged", MyManager, APIFixture(f1)) { ConfigSubscriber s(IConfigContext::SP(new APIFixture(f2))); ConfigHandle<FooConfig>::UP h1 = s.subscribe<FooConfig>("myid2"); ConfigHandle<BarConfig>::UP h2 = s.subscribe<BarConfig>("myid2"); - ASSERT_FALSE(s.nextConfig(0)); + EXPECT_FALSE(s.nextConfig(0)); f1.updateValue(0, createFooValue("foo"), 1); f1.updateValue(1, createFooValue("bar"), 1); - ASSERT_TRUE(s.nextConfig(100)); - ASSERT_TRUE(h1->isChanged()); - ASSERT_TRUE(h2->isChanged()); + EXPECT_TRUE(s.nextConfig(100)); + EXPECT_TRUE(h1->isChanged()); + EXPECT_TRUE(h2->isChanged()); - ASSERT_FALSE(s.nextConfig(100)); - ASSERT_FALSE(h1->isChanged()); - ASSERT_FALSE(h2->isChanged()); + EXPECT_FALSE(s.nextConfig(100)); + EXPECT_FALSE(h1->isChanged()); + EXPECT_FALSE(h2->isChanged()); f1.updateValue(0, createFooValue("bar"), 2); f1.updateGeneration(1, 2); - ASSERT_TRUE(s.nextConfig(100)); - ASSERT_TRUE(h1->isChanged()); - ASSERT_FALSE(h2->isChanged()); + EXPECT_TRUE(s.nextConfig(100)); + EXPECT_TRUE(h1->isChanged()); + EXPECT_FALSE(h2->isChanged()); } TEST_FF("requireThatNextGenerationMarksChanged", MyManager, APIFixture(f1)) { diff --git a/config/src/tests/subscription/subscription.cpp b/config/src/tests/subscription/subscription.cpp index 5d36753813c..a9e4c923e92 100644 --- a/config/src/tests/subscription/subscription.cpp +++ b/config/src/tests/subscription/subscription.cpp @@ -101,11 +101,11 @@ TEST_MT_F("requireThatNextUpdateReturnsInterrupted", 2, SubscriptionFixture(Conf TEST_F("Require that isChanged takes generation into account", SubscriptionFixture(ConfigKey::create<MyConfig>("myid"))) { - f1.holder->handle(ConfigUpdate::UP(new ConfigUpdate(ConfigValue(), true, 1))); + f1.holder->handle(ConfigUpdate::UP(new ConfigUpdate(ConfigValue(std::vector<vespalib::string>(), "a"), true, 1))); ASSERT_TRUE(f1.sub.nextUpdate(0, 0)); f1.sub.flip(); ASSERT_EQUAL(1, f1.sub.getLastGenerationChanged()); - f1.holder->handle(ConfigUpdate::UP(new ConfigUpdate(ConfigValue(), true, 2))); + f1.holder->handle(ConfigUpdate::UP(new ConfigUpdate(ConfigValue(std::vector<vespalib::string>(), "b"), true, 2))); ASSERT_TRUE(f1.sub.nextUpdate(1, 0)); f1.sub.flip(); ASSERT_EQUAL(2, f1.sub.getLastGenerationChanged()); diff --git a/config/src/vespa/config/subscription/configsubscription.cpp b/config/src/vespa/config/subscription/configsubscription.cpp index b79e3dab1fd..efa3f5a02e3 100644 --- a/config/src/vespa/config/subscription/configsubscription.cpp +++ b/config/src/vespa/config/subscription/configsubscription.cpp @@ -28,8 +28,9 @@ ConfigSubscription::~ConfigSubscription() bool ConfigSubscription::nextUpdate(int64_t generation, uint64_t timeoutInMillis) { - if (_closed || !_holder->poll()) + if (_closed || !_holder->poll()) { return false; + } _next = _holder->provide(); if (isGenerationNewer(_next->getGeneration(), generation)) { return true; @@ -38,9 +39,15 @@ ConfigSubscription::nextUpdate(int64_t generation, uint64_t timeoutInMillis) } bool +ConfigSubscription::hasGenerationChanged() const +{ + return (!_closed && _next && ((_current && (_current->getGeneration() != _next->getGeneration())) || ! _current)); +} + +bool ConfigSubscription::hasChanged() const { - return (!_closed && (_next->hasChanged() || _current.get() == NULL)); + return (!_closed && _next && ((_next->hasChanged() && _current && (_current->getValue() != _next->getValue())) || ! _current)); } int64_t @@ -88,7 +95,7 @@ ConfigSubscription::flip() { bool change = hasChanged(); if (change) { - _current.reset(_next.release()); + _current = std::move(_next); _lastGenerationChanged = _current->getGeneration(); } else { _current.reset(new ConfigUpdate(_current->getValue(), false, _next->getGeneration())); @@ -96,13 +103,15 @@ ConfigSubscription::flip() _isChanged = change; } -ConfigValue +const ConfigValue & ConfigSubscription::getConfig() const { - if (_closed) + if (_closed) { throw ConfigRuntimeException("Subscription is closed, config no longer available"); - if (_current.get() == NULL) + } + if ( ! _current) { throw ConfigRuntimeException("No configuration available"); + } return _current->getValue(); } diff --git a/config/src/vespa/config/subscription/configsubscription.h b/config/src/vespa/config/subscription/configsubscription.h index 04c3da114fd..56b65922a61 100644 --- a/config/src/vespa/config/subscription/configsubscription.h +++ b/config/src/vespa/config/subscription/configsubscription.h @@ -28,7 +28,7 @@ public: * * @return the current ConfigValue. */ - ConfigValue getConfig() const; + const ConfigValue & getConfig() const; /** * Checks whether or not the config has changed. @@ -48,6 +48,7 @@ public: bool nextUpdate(int64_t generation, uint64_t timeoutInMillis); int64_t getGeneration() const; bool hasChanged() const; + bool hasGenerationChanged() const; void flip(); void reset(); void close(); diff --git a/config/src/vespa/config/subscription/configsubscriptionset.cpp b/config/src/vespa/config/subscription/configsubscriptionset.cpp index f6268c8a84a..ab38ba18351 100644 --- a/config/src/vespa/config/subscription/configsubscriptionset.cpp +++ b/config/src/vespa/config/subscription/configsubscriptionset.cpp @@ -38,10 +38,6 @@ ConfigSubscriptionSet::acquireSnapshot(uint64_t timeoutInMillis, bool ignoreChan int64_t lastGeneration = _currentGeneration; bool inSync = false; - for (const auto & subscription : _subscriptionList) { - subscription->reset(); - } - LOG(debug, "Going into nextConfig loop, time left is %d", timeLeft); while (_state != CLOSED && timeLeft >= 0 && !inSync) { size_t numChanged = 0; @@ -52,8 +48,10 @@ ConfigSubscriptionSet::acquireSnapshot(uint64_t timeoutInMillis, bool ignoreChan // Run nextUpdate on all subscribers to get them in sync. for (const auto & subscription : _subscriptionList) { - if (!subscription->nextUpdate(_currentGeneration, timeLeft)) - break; + if (!subscription->nextUpdate(_currentGeneration, timeLeft) && !subscription->hasGenerationChanged()) { + subscription->reset(); + continue; + } const ConfigKey & key(subscription->getKey()); if (subscription->hasChanged()) { |