diff options
34 files changed, 139 insertions, 129 deletions
diff --git a/config/src/tests/configfetcher/configfetcher.cpp b/config/src/tests/configfetcher/configfetcher.cpp index 2a1158de48f..5a2943f7512 100644 --- a/config/src/tests/configfetcher/configfetcher.cpp +++ b/config/src/tests/configfetcher/configfetcher.cpp @@ -29,7 +29,7 @@ public: }; MyCallback::MyCallback(const std::string & badConfig) : _config(), _configured(false), _badConfig(badConfig) { } -MyCallback::~MyCallback() { } +MyCallback::~MyCallback() = default; TEST("requireThatConfigIsAvailableOnConstruction") { RawSpec spec("myField \"foo\"\n"); @@ -39,7 +39,7 @@ TEST("requireThatConfigIsAvailableOnConstruction") { ConfigFetcher fetcher(spec); fetcher.subscribe<MyConfig>("myid", &cb); fetcher.start(); - ASSERT_TRUE(cb._config.get() != NULL); + ASSERT_TRUE(cb._config); ASSERT_EQUAL("my", cb._config->defName()); ASSERT_EQUAL("foo", cb._config->myField); } diff --git a/config/src/tests/configholder/configholder.cpp b/config/src/tests/configholder/configholder.cpp index a9b6dafb72a..a488cca705a 100644 --- a/config/src/tests/configholder/configholder.cpp +++ b/config/src/tests/configholder/configholder.cpp @@ -1,9 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/config/common/configholder.h> -#include <vespa/fastos/time.h> using namespace config; +using namespace std::chrono_literals; namespace { @@ -35,12 +35,12 @@ TEST("Require that waiting is done") ConfigHolder holder; FastOS_Time timer; timer.SetNow(); - holder.wait(1000); + holder.wait(1000ms); EXPECT_GREATER_EQUAL(timer.MilliSecsToNow(), ONE_SEC); EXPECT_LESS(timer.MilliSecsToNow(), ONE_MINUTE); holder.handle(std::make_unique<ConfigUpdate>(value, true, 0)); - ASSERT_TRUE(holder.wait(100)); + ASSERT_TRUE(holder.wait(100ms)); } TEST("Require that polling for elements work") @@ -73,13 +73,13 @@ TEST_MT_F("Require that wait is interrupted", 2, ConfigHolder) FastOS_Time timer; timer.SetNow(); TEST_BARRIER(); - f.wait(1000); + f.wait(1000ms); EXPECT_LESS(timer.MilliSecsToNow(), ONE_MINUTE); EXPECT_GREATER(timer.MilliSecsToNow(), 400.0); TEST_BARRIER(); } else { TEST_BARRIER(); - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + std::this_thread::sleep_for(500ms); f.interrupt(); TEST_BARRIER(); } diff --git a/config/src/tests/unittest/unittest.cpp b/config/src/tests/unittest/unittest.cpp index 1569e545f03..4b46dcef900 100644 --- a/config/src/tests/unittest/unittest.cpp +++ b/config/src/tests/unittest/unittest.cpp @@ -1,13 +1,16 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("unittest"); + #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/config/config.h> #include "config-my.h" #include "config-foo.h" #include "config-bar.h" +#include <vespa/log/log.h> +LOG_SETUP("unittest"); + using namespace config; +using namespace std::chrono_literals; namespace { void verifyConfig(const std::string & expected, std::unique_ptr<FooConfig> cfg) @@ -41,15 +44,15 @@ TEST("requireThatConfigCanBeReloaded") { ConfigSubscriber subscriber(ctx); ConfigHandle<MyConfig>::UP handle = subscriber.subscribe<MyConfig>("myid"); - ASSERT_TRUE(subscriber.nextConfig(0)); + ASSERT_TRUE(subscriber.nextConfigNow()); std::unique_ptr<MyConfig> cfg(handle->getConfig()); ASSERT_TRUE(cfg.get() != NULL); ASSERT_EQUAL("myfoo", cfg->myField); ctx->reload(); - ASSERT_FALSE(subscriber.nextConfig(1000)); + ASSERT_FALSE(subscriber.nextConfig(1000ms)); builder.myField = "foobar"; ctx->reload(); - ASSERT_TRUE(subscriber.nextConfig(10000)); + ASSERT_TRUE(subscriber.nextConfig(10000ms)); cfg = handle->getConfig(); ASSERT_TRUE(cfg.get() != NULL); ASSERT_EQUAL("foobar", cfg->myField); @@ -71,21 +74,21 @@ TEST("requireThatCanSubscribeWithSameIdToDifferentDefs") { ConfigHandle<FooConfig>::UP h1 = subscriber.subscribe<FooConfig>("fooid"); ConfigHandle<BarConfig>::UP h2 = subscriber.subscribe<BarConfig>("fooid"); - ASSERT_TRUE(subscriber.nextConfig(0)); + ASSERT_TRUE(subscriber.nextConfigNow()); verifyConfig("myfoo", h1->getConfig()); verifyConfig("mybar", h2->getConfig()); ctx->reload(); - ASSERT_FALSE(subscriber.nextConfig(100)); + ASSERT_FALSE(subscriber.nextConfig(100ms)); fooBuilder.fooValue = "blabla"; ctx->reload(); - ASSERT_TRUE(subscriber.nextConfig(5000)); + ASSERT_TRUE(subscriber.nextConfig(5000ms)); verifyConfig("blabla", h1->getConfig()); verifyConfig("mybar", h2->getConfig()); barBuilder.barValue = "blabar"; ctx->reload(); - ASSERT_TRUE(subscriber.nextConfig(5000)); + ASSERT_TRUE(subscriber.nextConfig(5000ms)); verifyConfig("blabla", h1->getConfig()); verifyConfig("blabar", h2->getConfig()); } diff --git a/config/src/vespa/config/common/configholder.cpp b/config/src/vespa/config/common/configholder.cpp index d3d7e8fd5de..c2839b4a123 100644 --- a/config/src/vespa/config/common/configholder.cpp +++ b/config/src/vespa/config/common/configholder.cpp @@ -31,7 +31,7 @@ ConfigHolder::handle(ConfigUpdate::UP update) } bool -ConfigHolder::wait(uint64_t timeoutInMillis) +ConfigHolder::wait(milliseconds timeoutInMillis) { vespalib::MonitorGuard guard(_monitor); return static_cast<bool>(_current) || guard.wait(timeoutInMillis); diff --git a/config/src/vespa/config/common/configholder.h b/config/src/vespa/config/common/configholder.h index 55dfbf58af9..be65d6cff39 100644 --- a/config/src/vespa/config/common/configholder.h +++ b/config/src/vespa/config/common/configholder.h @@ -13,11 +13,11 @@ class ConfigHolder : public IConfigHolder { public: ConfigHolder(); - ~ConfigHolder(); + ~ConfigHolder() override; ConfigUpdate::UP provide() override; void handle(ConfigUpdate::UP update) override; - bool wait(uint64_t timeoutInMillis) override; + bool wait(milliseconds timeoutInMillis) override; bool poll() override; void interrupt() override; public: diff --git a/config/src/vespa/config/common/configmanager.cpp b/config/src/vespa/config/common/configmanager.cpp index 521909fa836..11fe66a1c64 100644 --- a/config/src/vespa/config/common/configmanager.cpp +++ b/config/src/vespa/config/common/configmanager.cpp @@ -2,7 +2,6 @@ #include "configmanager.h" #include "exceptions.h" #include "configholder.h" -#include <vespa/fastos/time.h> #include <thread> #include <sstream> @@ -10,6 +9,7 @@ LOG_SETUP(".config.common.configmanager"); using namespace std::chrono_literals; +using namespace std::chrono; namespace config { @@ -22,25 +22,24 @@ ConfigManager::ConfigManager(SourceFactory::UP sourceFactory, int64_t initialGen _firstLock() { } -ConfigManager::~ConfigManager() { } +ConfigManager::~ConfigManager() = default; ConfigSubscription::SP -ConfigManager::subscribe(const ConfigKey & key, uint64_t timeoutInMillis) +ConfigManager::subscribe(const ConfigKey & key, milliseconds timeoutInMillis) { LOG(debug, "subscribing on def %s, configid %s", key.getDefName().c_str(), key.getConfigId().c_str()); SubscriptionId id(_idGenerator.fetch_add(1)); - IConfigHolder::SP holder(new ConfigHolder()); + auto holder = std::make_shared<ConfigHolder>(); Source::UP source = _sourceFactory->createSource(holder, key); source->reload(_generation); source->getConfig(); ConfigSubscription::SP subscription(new ConfigSubscription(id, key, holder, std::move(source))); - FastOS_Time timer; - timer.SetNow(); - while (timer.MilliSecsToNow() < timeoutInMillis) { + steady_clock::time_point endTime = steady_clock::now() + timeoutInMillis; + while (steady_clock::now() < endTime) { if (holder->poll()) break; std::this_thread::sleep_for(10ms); diff --git a/config/src/vespa/config/common/configmanager.h b/config/src/vespa/config/common/configmanager.h index e0e2bed6a53..6210a03c0d1 100644 --- a/config/src/vespa/config/common/configmanager.h +++ b/config/src/vespa/config/common/configmanager.h @@ -25,7 +25,7 @@ public: ~ConfigManager(); // Implements IConfigManager - ConfigSubscription::SP subscribe(const ConfigKey & key, uint64_t timeoutInMillis) override; + ConfigSubscription::SP subscribe(const ConfigKey & key, milliseconds timeoutInMillis) override; // Implements IConfigManager void unsubscribe(const ConfigSubscription::SP & subscription) override; diff --git a/config/src/vespa/config/common/subscribehandler.h b/config/src/vespa/config/common/subscribehandler.h index 6f0273aa9ea..dba7b606825 100644 --- a/config/src/vespa/config/common/subscribehandler.h +++ b/config/src/vespa/config/common/subscribehandler.h @@ -1,13 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include "configkey.h" #include <vespa/config/subscription/configsubscription.h> -#include <vespa/config/common/configkey.h> namespace config { struct SubscribeHandler { + using milliseconds = std::chrono::milliseconds; /** * Subscribes to a spesific config given by a subscription. * If the subscribe call is successful, the callback handler will be called @@ -17,7 +18,7 @@ struct SubscribeHandler * @param timeoutInMillis the timeout of the subscribe call. * @return subscription object containing data relevant to client */ - virtual ConfigSubscription::SP subscribe(const ConfigKey & key, uint64_t timeoutInMillis) = 0; + virtual ConfigSubscription::SP subscribe(const ConfigKey & key, milliseconds timeoutInMillis) = 0; virtual ~SubscribeHandler() { } }; diff --git a/config/src/vespa/config/common/timingvalues.cpp b/config/src/vespa/config/common/timingvalues.cpp index 58d10d831b7..a2ad586d090 100644 --- a/config/src/vespa/config/common/timingvalues.cpp +++ b/config/src/vespa/config/common/timingvalues.cpp @@ -4,6 +4,12 @@ namespace config { +using std::chrono::milliseconds; + +const milliseconds DEFAULT_NEXTCONFIG_TIMEOUT(55000); +const milliseconds DEFAULT_SUBSCRIBE_TIMEOUT(55000); +const milliseconds DEFAULT_GETCONFIGS_TIMEOUT(55000); + TimingValues::TimingValues() : successTimeout(600000), errorTimeout(25000), @@ -22,7 +28,7 @@ TimingValues::TimingValues() TimingValues::TimingValues(uint64_t initSuccessTimeout, uint64_t initErrorTimeout, uint64_t initInitialTimeout, - uint64_t initSubscribeTimeout, + milliseconds initSubscribeTimeout, uint64_t initFixedDelay, uint64_t initSuccessDelay, uint64_t initUnconfiguredDelay, diff --git a/config/src/vespa/config/common/timingvalues.h b/config/src/vespa/config/common/timingvalues.h index a11a78dcb0d..b6d69e62510 100644 --- a/config/src/vespa/config/common/timingvalues.h +++ b/config/src/vespa/config/common/timingvalues.h @@ -3,20 +3,22 @@ #pragma once #include <cstdint> +#include <chrono> namespace config { -static const uint64_t DEFAULT_NEXTCONFIG_TIMEOUT = 55000; -static const uint64_t DEFAULT_SUBSCRIBE_TIMEOUT = 55000; -static const uint64_t DEFAULT_GETCONFIGS_TIMEOUT = 55000; +extern const std::chrono::milliseconds DEFAULT_NEXTCONFIG_TIMEOUT; +extern const std::chrono::milliseconds DEFAULT_SUBSCRIBE_TIMEOUT; +extern const std::chrono::milliseconds DEFAULT_GETCONFIGS_TIMEOUT; struct TimingValues { + using milliseconds = std::chrono::milliseconds; uint64_t successTimeout; // Timeout when previous config request was a success. uint64_t errorTimeout; // Timeout when previous config request was an error. uint64_t initialTimeout; // Timeout used when requesting config for the first time. - uint64_t subscribeTimeout; // Timeout used to find out when to give up unsubscribe. + milliseconds subscribeTimeout; // Timeout used to find out when to give up unsubscribe. uint64_t fixedDelay; // Fixed delay between config requests. uint64_t successDelay; // Delay if until next request after successful getConfig. @@ -31,7 +33,7 @@ struct TimingValues TimingValues(uint64_t initSuccessTimeout, uint64_t initerrorTimeout, uint64_t initInitialTimeout, - uint64_t initSubscribeTimeout, + milliseconds initSubscribeTimeout, uint64_t initFixedDelay, uint64_t initSuccessDelay, uint64_t initUnconfiguredDelay, diff --git a/config/src/vespa/config/common/waitable.h b/config/src/vespa/config/common/waitable.h index 16ce9a1f6fa..9dcd04fbd8b 100644 --- a/config/src/vespa/config/common/waitable.h +++ b/config/src/vespa/config/common/waitable.h @@ -2,7 +2,7 @@ #pragma once -#include <memory> +#include <chrono> namespace config { @@ -11,7 +11,8 @@ namespace config { */ struct Waitable { - virtual bool wait(uint64_t timeoutInMillis) = 0; + using milliseconds = std::chrono::milliseconds; + virtual bool wait(milliseconds) = 0; virtual ~Waitable() {} }; diff --git a/config/src/vespa/config/helper/configfetcher.h b/config/src/vespa/config/helper/configfetcher.h index 8f318ea511c..dc9cf6299e8 100644 --- a/config/src/vespa/config/helper/configfetcher.h +++ b/config/src/vespa/config/helper/configfetcher.h @@ -15,12 +15,13 @@ namespace config { class ConfigFetcher { public: + using milliseconds = std::chrono::milliseconds; ConfigFetcher(const IConfigContext::SP & context); ConfigFetcher(const SourceSpec & spec = ServerSpec()); ~ConfigFetcher(); template <typename ConfigType> - void subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, uint64_t subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); + void subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, milliseconds subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); void start(); void close(); diff --git a/config/src/vespa/config/helper/configfetcher.hpp b/config/src/vespa/config/helper/configfetcher.hpp index d773c60b3fd..862e93eac68 100644 --- a/config/src/vespa/config/helper/configfetcher.hpp +++ b/config/src/vespa/config/helper/configfetcher.hpp @@ -4,7 +4,7 @@ namespace config { template <typename ConfigType> void -ConfigFetcher::subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, uint64_t subscribeTimeout) +ConfigFetcher::subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, milliseconds subscribeTimeout) { _poller.subscribe<ConfigType>(configId, callback, subscribeTimeout); } diff --git a/config/src/vespa/config/helper/configgetter.hpp b/config/src/vespa/config/helper/configgetter.hpp index ac52fdb164f..6d1f6fe78e5 100644 --- a/config/src/vespa/config/helper/configgetter.hpp +++ b/config/src/vespa/config/helper/configgetter.hpp @@ -2,6 +2,7 @@ #include "configgetter.h" #include <vespa/config/subscription/configsubscriber.h> + namespace config { template <typename ConfigType> @@ -10,7 +11,7 @@ ConfigGetter<ConfigType>::getConfig(int64_t &generation, const std::string & con { ConfigSubscriber s(spec); std::unique_ptr< ConfigHandle<ConfigType> > h = s.subscribe<ConfigType>(configId); - s.nextConfig(0); + s.nextConfigNow(); generation = s.getGeneration(); return h->getConfig(); } @@ -21,7 +22,7 @@ ConfigGetter<ConfigType>::getConfig(int64_t &generation, const std::string & con { ConfigSubscriber s(context); std::unique_ptr< ConfigHandle<ConfigType> > h = s.subscribe<ConfigType>(configId, subscribeTimeout); - s.nextConfig(0); + s.nextConfigNow(); generation = s.getGeneration(); return h->getConfig(); } diff --git a/config/src/vespa/config/helper/configpoller.h b/config/src/vespa/config/helper/configpoller.h index 88e589fd16a..e44b9ae8e84 100644 --- a/config/src/vespa/config/helper/configpoller.h +++ b/config/src/vespa/config/helper/configpoller.h @@ -15,11 +15,12 @@ namespace config { */ class ConfigPoller : public vespalib::Runnable { public: + using milliseconds = std::chrono::milliseconds; ConfigPoller(const IConfigContext::SP & context); ~ConfigPoller(); void run() override; template <typename ConfigType> - void subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, uint64_t subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); + void subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, milliseconds subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); void poll(); void close(); int64_t getGeneration() const { return _generation; } diff --git a/config/src/vespa/config/helper/configpoller.hpp b/config/src/vespa/config/helper/configpoller.hpp index c6fad9e2e97..b87df593d29 100644 --- a/config/src/vespa/config/helper/configpoller.hpp +++ b/config/src/vespa/config/helper/configpoller.hpp @@ -4,7 +4,7 @@ namespace config { template <typename ConfigType> void -ConfigPoller::subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, uint64_t subscribeTimeout) +ConfigPoller::subscribe(const std::string & configId, IFetcherCallback<ConfigType> * callback, milliseconds subscribeTimeout) { std::unique_ptr<ConfigHandle<ConfigType> > handle(_subscriber.subscribe<ConfigType>(configId, subscribeTimeout)); _handleList.emplace_back(std::make_unique<GenericHandle<ConfigType>>(std::move(handle))); diff --git a/config/src/vespa/config/retriever/configretriever.cpp b/config/src/vespa/config/retriever/configretriever.cpp index 2056ae6351d..08e7f15f0c5 100644 --- a/config/src/vespa/config/retriever/configretriever.cpp +++ b/config/src/vespa/config/retriever/configretriever.cpp @@ -3,12 +3,16 @@ #include "configretriever.h" #include <vespa/config/common/exceptions.h> +using std::chrono::milliseconds; namespace config { + const milliseconds ConfigRetriever::DEFAULT_SUBSCRIBE_TIMEOUT(60000); + const milliseconds ConfigRetriever::DEFAULT_NEXTGENERATION_TIMEOUT(60000); + ConfigRetriever::ConfigRetriever(const ConfigKeySet & bootstrapSet, const IConfigContext::SP & context, - int64_t subscribeTimeout) + milliseconds subscribeTimeout) : _bootstrapSubscriber(bootstrapSet, context, subscribeTimeout), _configSubscriber(), _lock(), @@ -25,7 +29,7 @@ ConfigRetriever::ConfigRetriever(const ConfigKeySet & bootstrapSet, ConfigRetriever::~ConfigRetriever() = default; ConfigSnapshot -ConfigRetriever::getBootstrapConfigs(int timeoutInMillis) +ConfigRetriever::getBootstrapConfigs(milliseconds timeoutInMillis) { bool ret = _bootstrapSubscriber.nextGeneration(timeoutInMillis); if (!ret) { @@ -36,7 +40,7 @@ ConfigRetriever::getBootstrapConfigs(int timeoutInMillis) } ConfigSnapshot -ConfigRetriever::getConfigs(const ConfigKeySet & keySet, int timeoutInMillis) +ConfigRetriever::getConfigs(const ConfigKeySet & keySet, milliseconds timeoutInMillis) { if (_closed) return ConfigSnapshot(); diff --git a/config/src/vespa/config/retriever/configretriever.h b/config/src/vespa/config/retriever/configretriever.h index 89397959a6c..14e3171fc62 100644 --- a/config/src/vespa/config/retriever/configretriever.h +++ b/config/src/vespa/config/retriever/configretriever.h @@ -24,9 +24,10 @@ namespace config { class ConfigRetriever { public: + using milliseconds = std::chrono::milliseconds; ConfigRetriever(const ConfigKeySet & bootstrapSet, const IConfigContext::SP & context, - int64_t subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); + milliseconds subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); ~ConfigRetriever(); /** @@ -39,7 +40,7 @@ public: * retriever has been closed. * @throws ConfigTimeoutException if initial subscribe timed out. */ - ConfigSnapshot getBootstrapConfigs(int timeoutInMillis = DEFAULT_NEXTGENERATION_TIMEOUT); + ConfigSnapshot getBootstrapConfigs(milliseconds timeoutInMillis = DEFAULT_NEXTGENERATION_TIMEOUT); /** * Return the configs represented by a ConfigKeySet in a snapshot, and makes @@ -60,7 +61,7 @@ public: * method can be used to check for this condition. * @throws ConfigTimeoutException if resubscribe timed out. */ - ConfigSnapshot getConfigs(const ConfigKeySet & keySet, int timeoutInMillis = DEFAULT_NEXTGENERATION_TIMEOUT); + ConfigSnapshot getConfigs(const ConfigKeySet & keySet, milliseconds timeoutInMillis = DEFAULT_NEXTGENERATION_TIMEOUT); /** * Close this retriever in order to shut down. @@ -88,20 +89,20 @@ public: */ int64_t getGeneration() const { return _generation; } - static const int DEFAULT_SUBSCRIBE_TIMEOUT = 60000; - static const int DEFAULT_NEXTGENERATION_TIMEOUT = 60000; + static const milliseconds DEFAULT_SUBSCRIBE_TIMEOUT; + static const milliseconds DEFAULT_NEXTGENERATION_TIMEOUT; private: - FixedConfigSubscriber _bootstrapSubscriber; + FixedConfigSubscriber _bootstrapSubscriber; std::unique_ptr<GenericConfigSubscriber> _configSubscriber; - vespalib::Lock _lock; - std::vector<ConfigSubscription::SP> _subscriptionList; - ConfigKeySet _lastKeySet; - IConfigContext::SP _context; + vespalib::Lock _lock; + std::vector<ConfigSubscription::SP> _subscriptionList; + ConfigKeySet _lastKeySet; + IConfigContext::SP _context; std::unique_ptr<SourceSpec> _spec; - bool _closed; - int64_t _generation; - int64_t _subscribeTimeout; - bool _bootstrapRequired; + bool _closed; + int64_t _generation; + milliseconds _subscribeTimeout; + bool _bootstrapRequired; }; } // namespace config diff --git a/config/src/vespa/config/retriever/fixedconfigsubscriber.cpp b/config/src/vespa/config/retriever/fixedconfigsubscriber.cpp index 9b8d49d125b..fa904b54a0d 100644 --- a/config/src/vespa/config/retriever/fixedconfigsubscriber.cpp +++ b/config/src/vespa/config/retriever/fixedconfigsubscriber.cpp @@ -4,17 +4,17 @@ namespace config { FixedConfigSubscriber::FixedConfigSubscriber(const ConfigKeySet & keySet, const IConfigContext::SP & context, - int64_t subscribeTimeout) + milliseconds subscribeTimeout) : _set(context), _subscriptionList() { - for (ConfigKeySet::const_iterator it(keySet.begin()), mt(keySet.end()); it != mt; it++) { - _subscriptionList.push_back(_set.subscribe(*it, subscribeTimeout)); + for (const ConfigKey & key : keySet) { + _subscriptionList.push_back(_set.subscribe(key, subscribeTimeout)); } } bool -FixedConfigSubscriber::nextGeneration(int timeoutInMillis) +FixedConfigSubscriber::nextGeneration(milliseconds timeoutInMillis) { return _set.acquireSnapshot(timeoutInMillis, true); } diff --git a/config/src/vespa/config/retriever/fixedconfigsubscriber.h b/config/src/vespa/config/retriever/fixedconfigsubscriber.h index b48be326897..de6201f74a2 100644 --- a/config/src/vespa/config/retriever/fixedconfigsubscriber.h +++ b/config/src/vespa/config/retriever/fixedconfigsubscriber.h @@ -1,9 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/config/subscription/configsubscriptionset.h> #include "configkeyset.h" #include "configsnapshot.h" +#include <vespa/config/subscription/configsubscriptionset.h> namespace config { @@ -14,8 +14,9 @@ namespace config { class FixedConfigSubscriber { public: - FixedConfigSubscriber(const ConfigKeySet & keySet, const IConfigContext::SP & context, int64_t subscribeTimeout); - bool nextGeneration(int timeoutInMillis); + using milliseconds = std::chrono::milliseconds; + FixedConfigSubscriber(const ConfigKeySet & keySet, const IConfigContext::SP & context, milliseconds subscribeTimeout); + bool nextGeneration(milliseconds timeoutInMillis); void close(); int64_t getGeneration() const; ConfigSnapshot getConfigSnapshot() const; diff --git a/config/src/vespa/config/retriever/genericconfigsubscriber.cpp b/config/src/vespa/config/retriever/genericconfigsubscriber.cpp index 5681a7602e7..14f1b7304ca 100644 --- a/config/src/vespa/config/retriever/genericconfigsubscriber.cpp +++ b/config/src/vespa/config/retriever/genericconfigsubscriber.cpp @@ -8,13 +8,13 @@ GenericConfigSubscriber::GenericConfigSubscriber(const IConfigContext::SP & cont { } bool -GenericConfigSubscriber::nextGeneration(int timeoutInMillis) +GenericConfigSubscriber::nextGeneration(milliseconds timeoutInMillis) { return _set.acquireSnapshot(timeoutInMillis, true); } ConfigSubscription::SP -GenericConfigSubscriber::subscribe(const ConfigKey & key, int timeoutInMillis) +GenericConfigSubscriber::subscribe(const ConfigKey & key, milliseconds timeoutInMillis) { return _set.subscribe(key, timeoutInMillis); } diff --git a/config/src/vespa/config/retriever/genericconfigsubscriber.h b/config/src/vespa/config/retriever/genericconfigsubscriber.h index d19ce608cc7..c0d7cb11b0c 100644 --- a/config/src/vespa/config/retriever/genericconfigsubscriber.h +++ b/config/src/vespa/config/retriever/genericconfigsubscriber.h @@ -13,9 +13,10 @@ namespace config { class GenericConfigSubscriber { public: + using milliseconds = std::chrono::milliseconds; GenericConfigSubscriber(const IConfigContext::SP & context); - bool nextGeneration(int timeoutInMillis); - ConfigSubscription::SP subscribe(const ConfigKey & key, int timeoutInMillis); + bool nextGeneration(milliseconds timeoutInMillis); + ConfigSubscription::SP subscribe(const ConfigKey & key, milliseconds timeoutInMillis); void close(); int64_t getGeneration() const; private: diff --git a/config/src/vespa/config/retriever/simpleconfigretriever.cpp b/config/src/vespa/config/retriever/simpleconfigretriever.cpp index 26087d6641a..62e2405e809 100644 --- a/config/src/vespa/config/retriever/simpleconfigretriever.cpp +++ b/config/src/vespa/config/retriever/simpleconfigretriever.cpp @@ -4,17 +4,17 @@ namespace config { SimpleConfigRetriever::SimpleConfigRetriever(const ConfigKeySet & keySet, const IConfigContext::SP & context, - uint64_t subscribeTimeout) + milliseconds subscribeTimeout) : _set(context), _subscriptionList() { - for (ConfigKeySet::const_iterator it(keySet.begin()), mt(keySet.end()); it != mt; it++) { - _subscriptionList.push_back(_set.subscribe(*it, subscribeTimeout)); + for (const ConfigKey & key : keySet) { + _subscriptionList.push_back(_set.subscribe(key, subscribeTimeout)); } } ConfigSnapshot -SimpleConfigRetriever::getConfigs(uint64_t timeoutInMillis) +SimpleConfigRetriever::getConfigs(milliseconds timeoutInMillis) { if (_set.acquireSnapshot(timeoutInMillis, true)) { return ConfigSnapshot(_subscriptionList, _set.getGeneration()); diff --git a/config/src/vespa/config/retriever/simpleconfigretriever.h b/config/src/vespa/config/retriever/simpleconfigretriever.h index b65b5d52ced..3bd35baddce 100644 --- a/config/src/vespa/config/retriever/simpleconfigretriever.h +++ b/config/src/vespa/config/retriever/simpleconfigretriever.h @@ -17,10 +17,11 @@ class SimpleConfigRetriever { public: typedef std::unique_ptr<SimpleConfigRetriever> UP; + using milliseconds = std::chrono::milliseconds; SimpleConfigRetriever(const ConfigKeySet & keySet, const IConfigContext::SP & context, - uint64_t subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); + milliseconds subscribeTimeout = DEFAULT_SUBSCRIBE_TIMEOUT); /** * Attempt retrieving a snapshot of configs. @@ -28,7 +29,7 @@ public: * @return A new snapshot. The snapshot is empty if timeout was reached or * if the retriever was closed. */ - ConfigSnapshot getConfigs(uint64_t timeoutInMillis = DEFAULT_GETCONFIGS_TIMEOUT); + ConfigSnapshot getConfigs(milliseconds timeoutInMillis = DEFAULT_GETCONFIGS_TIMEOUT); void close(); bool isClosed() const; diff --git a/config/src/vespa/config/subscription/configsubscriber.cpp b/config/src/vespa/config/subscription/configsubscriber.cpp index 05cabd4e4e1..b226c149b9e 100644 --- a/config/src/vespa/config/subscription/configsubscriber.cpp +++ b/config/src/vespa/config/subscription/configsubscriber.cpp @@ -11,17 +11,17 @@ ConfigSubscriber::ConfigSubscriber(const IConfigContext::SP & context) { } ConfigSubscriber::ConfigSubscriber(const SourceSpec & spec) - : _set(IConfigContext::SP(new ConfigContext(spec))) + : _set(std::make_shared<ConfigContext>(spec)) { } bool -ConfigSubscriber::nextConfig(uint64_t timeoutInMillis) +ConfigSubscriber::nextConfig(milliseconds timeoutInMillis) { return _set.acquireSnapshot(timeoutInMillis, false); } bool -ConfigSubscriber::nextGeneration(uint64_t timeoutInMillis) +ConfigSubscriber::nextGeneration(milliseconds timeoutInMillis) { return _set.acquireSnapshot(timeoutInMillis, true); } @@ -44,4 +44,4 @@ ConfigSubscriber::getGeneration() const return _set.getGeneration(); } -} // namespace config +} diff --git a/config/src/vespa/config/subscription/configsubscriber.h b/config/src/vespa/config/subscription/configsubscriber.h index 7e0729ed0d5..4787c6cd858 100644 --- a/config/src/vespa/config/subscription/configsubscriber.h +++ b/config/src/vespa/config/subscription/configsubscriber.h @@ -1,10 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <memory> -#include <map> -#include <vespa/config/common/iconfigholder.h> -#include <vespa/config/common/configcontext.h> -#include <vespa/config/common/timingvalues.h> + #include "confighandle.h" #include "subscriptionid.h" #include "configsubscription.h" @@ -30,6 +26,7 @@ namespace config { class ConfigSubscriber { public: + using milliseconds = std::chrono::milliseconds; typedef std::unique_ptr<ConfigSubscriber> UP; /** @@ -56,7 +53,8 @@ public: * @return true if new configs are available, false if timeout was reached * or subscriber has been closed. */ - bool nextConfig(uint64_t timeoutInMillis = DEFAULT_NEXTCONFIG_TIMEOUT); + bool nextConfig(milliseconds timeoutInMillis = DEFAULT_NEXTCONFIG_TIMEOUT); + bool nextConfigNow() { return nextConfig(milliseconds(0)); } /** * Checks if the generation of this config set is updated. @@ -65,8 +63,8 @@ public: * @return true if a new generation are available, false if timeout was reached * or subscriber has been closed. */ - bool nextGeneration(uint64_t timeoutInMillis = DEFAULT_NEXTCONFIG_TIMEOUT); - + bool nextGeneration(milliseconds timeoutInMillis = DEFAULT_NEXTCONFIG_TIMEOUT); + bool nextGenerationNow() { return nextGeneration(milliseconds(0)); } /** * Subscribe to a config fetched from the default source specification. * @@ -80,7 +78,7 @@ public: */ template <typename ConfigType> std::unique_ptr<ConfigHandle<ConfigType> > - subscribe(const std::string & configId, uint64_t timeoutInMillis = DEFAULT_SUBSCRIBE_TIMEOUT); + subscribe(const std::string & configId, milliseconds timeoutInMillis = DEFAULT_SUBSCRIBE_TIMEOUT); /** * Return the current generation number for configs. diff --git a/config/src/vespa/config/subscription/configsubscriber.hpp b/config/src/vespa/config/subscription/configsubscriber.hpp index f73c6ce129f..17cd817b092 100644 --- a/config/src/vespa/config/subscription/configsubscriber.hpp +++ b/config/src/vespa/config/subscription/configsubscriber.hpp @@ -4,7 +4,7 @@ namespace config { template <typename ConfigType> std::unique_ptr<ConfigHandle<ConfigType> > -ConfigSubscriber::subscribe(const std::string & configId, uint64_t timeoutInMillis) +ConfigSubscriber::subscribe(const std::string & configId, milliseconds timeoutInMillis) { const ConfigKey key(ConfigKey::create<ConfigType>(configId)); return std::unique_ptr<ConfigHandle<ConfigType> >(new ConfigHandle<ConfigType>(_set.subscribe(key, timeoutInMillis))); diff --git a/config/src/vespa/config/subscription/configsubscription.cpp b/config/src/vespa/config/subscription/configsubscription.cpp index efa3f5a02e3..93f8b99521c 100644 --- a/config/src/vespa/config/subscription/configsubscription.cpp +++ b/config/src/vespa/config/subscription/configsubscription.cpp @@ -26,7 +26,7 @@ ConfigSubscription::~ConfigSubscription() bool -ConfigSubscription::nextUpdate(int64_t generation, uint64_t timeoutInMillis) +ConfigSubscription::nextUpdate(int64_t generation, std::chrono::milliseconds timeout) { if (_closed || !_holder->poll()) { return false; @@ -35,7 +35,7 @@ ConfigSubscription::nextUpdate(int64_t generation, uint64_t timeoutInMillis) if (isGenerationNewer(_next->getGeneration(), generation)) { return true; } - return (!_closed && _holder->wait(timeoutInMillis)); + return (!_closed && _holder->wait(timeout)); } bool diff --git a/config/src/vespa/config/subscription/configsubscription.h b/config/src/vespa/config/subscription/configsubscription.h index 56b65922a61..80051edb86b 100644 --- a/config/src/vespa/config/subscription/configsubscription.h +++ b/config/src/vespa/config/subscription/configsubscription.h @@ -7,6 +7,7 @@ #include "subscriptionid.h" #include <atomic> +#include <chrono> namespace config { @@ -45,7 +46,7 @@ public: /// Used by ConfigSubscriptionSet SubscriptionId getSubscriptionId() const { return _id; } const ConfigKey & getKey() const; - bool nextUpdate(int64_t generation, uint64_t timeoutInMillis); + bool nextUpdate(int64_t generation, std::chrono::milliseconds timeoutInMillis); int64_t getGeneration() const; bool hasChanged() const; bool hasGenerationChanged() const; diff --git a/config/src/vespa/config/subscription/configsubscriptionset.cpp b/config/src/vespa/config/subscription/configsubscriptionset.cpp index f00b7048416..0b73929c738 100644 --- a/config/src/vespa/config/subscription/configsubscriptionset.cpp +++ b/config/src/vespa/config/subscription/configsubscriptionset.cpp @@ -3,13 +3,13 @@ #include "configsubscriptionset.h" #include <vespa/config/common/exceptions.h> #include <vespa/config/common/misc.h> -#include <vespa/fastos/time.h> #include <thread> #include <vespa/log/log.h> LOG_SETUP(".config.subscription.configsubscriptionset"); using namespace std::chrono_literals; +using namespace std::chrono; namespace config { @@ -27,7 +27,7 @@ ConfigSubscriptionSet::~ConfigSubscriptionSet() } bool -ConfigSubscriptionSet::acquireSnapshot(uint64_t timeoutInMillis, bool ignoreChange) +ConfigSubscriptionSet::acquireSnapshot(milliseconds timeoutInMillis, bool ignoreChange) { if (_state == CLOSED) { return false; @@ -35,14 +35,13 @@ ConfigSubscriptionSet::acquireSnapshot(uint64_t timeoutInMillis, bool ignoreChan _state = FROZEN; } - FastOS_Time timer; - timer.SetNow(); - int timeLeft = timeoutInMillis; + steady_clock::time_point startTime = steady_clock::now(); + milliseconds timeLeft = timeoutInMillis; int64_t lastGeneration = _currentGeneration; bool inSync = false; - LOG(debug, "Going into nextConfig loop, time left is %d", timeLeft); - while (!isClosed() && (timeLeft >= 0) && !inSync) { + LOG(debug, "Going into nextConfig loop, time left is %ld", timeLeft.count()); + while (!isClosed() && (timeLeft.count() >= 0) && !inSync) { size_t numChanged = 0; size_t numGenerationChanged = 0; bool generationsInSync = true; @@ -74,13 +73,13 @@ ConfigSubscriptionSet::acquireSnapshot(uint64_t timeoutInMillis, bool ignoreChan generationsInSync = false; } // Adjust timeout - timeLeft = timeoutInMillis - static_cast<uint64_t>(timer.MilliSecsToNow()); + timeLeft = timeoutInMillis - duration_cast<milliseconds>(steady_clock::now() - startTime); } inSync = generationsInSync && (_subscriptionList.size() == numGenerationChanged) && (ignoreChange || numChanged > 0); lastGeneration = generation; - timeLeft = timeoutInMillis - static_cast<uint64_t>(timer.MilliSecsToNow()); - if (!inSync && (timeLeft > 0)) { - std::this_thread::sleep_for(std::chrono::milliseconds(std::min(10, timeLeft))); + timeLeft = timeoutInMillis - duration_cast<milliseconds>(steady_clock::now() - startTime); + if (!inSync && (timeLeft.count() > 0)) { + std::this_thread::sleep_for(std::chrono::milliseconds(std::min(10l, timeLeft.count()))); } else { break; } @@ -121,7 +120,7 @@ ConfigSubscriptionSet::isClosed() const } ConfigSubscription::SP -ConfigSubscriptionSet::subscribe(const ConfigKey & key, uint64_t timeoutInMillis) +ConfigSubscriptionSet::subscribe(const ConfigKey & key, milliseconds timeoutInMillis) { if (_state != OPEN) { throw ConfigRuntimeException("Adding subscription after calling nextConfig() is not allowed"); diff --git a/config/src/vespa/config/subscription/configsubscriptionset.h b/config/src/vespa/config/subscription/configsubscriptionset.h index ce1f45b0eb8..181f79f0ef2 100644 --- a/config/src/vespa/config/subscription/configsubscriptionset.h +++ b/config/src/vespa/config/subscription/configsubscriptionset.h @@ -19,6 +19,7 @@ namespace config { class ConfigSubscriptionSet { public: + using milliseconds = std::chrono::milliseconds; /** * Constructs a new ConfigSubscriptionSet object which can be used to subscribe for 1 * or more configs from a specific source. @@ -48,10 +49,10 @@ public: bool isClosed() const; // Helpers for doing the subscription - ConfigSubscription::SP subscribe(const ConfigKey & key, uint64_t timeoutInMillis); + ConfigSubscription::SP subscribe(const ConfigKey & key, milliseconds timeoutInMillis); // Tries to acquire a new snapshot of config within the timeout - bool acquireSnapshot(uint64_t timeoutInMillis, bool requireDifference); + bool acquireSnapshot(milliseconds timeoutInMillis, bool requireDifference); private: // Describes the state of the subscriber. diff --git a/logd/src/logd/config_subscriber.cpp b/logd/src/logd/config_subscriber.cpp index 7981e13a9a1..1e6c9a040d3 100644 --- a/logd/src/logd/config_subscriber.cpp +++ b/logd/src/logd/config_subscriber.cpp @@ -3,8 +3,6 @@ #include "config_subscriber.h" #include "empty_forwarder.h" #include "rpc_forwarder.h" -#include <fcntl.h> -#include <unistd.h> #include <vespa/log/log.h> LOG_SETUP(""); @@ -71,7 +69,7 @@ ConfigSubscriber::configure(std::unique_ptr<LogdConfig> cfg) bool ConfigSubscriber::checkAvailable() { - if (_subscriber.nextGeneration(0)) { + if (_subscriber.nextGenerationNow()) { _has_available = true; } return _has_available; @@ -103,7 +101,7 @@ ConfigSubscriber::ConfigSubscriber(const config::ConfigUri& configUri) _server() { _handle = _subscriber.subscribe<LogdConfig>(configUri.getConfigId()); - _subscriber.nextConfig(0); + _subscriber.nextConfigNow(); configure(_handle->getConfig()); LOG(debug, "got logServer %s", _logserver_host.c_str()); diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index 40622b8c786..74842801909 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -806,7 +806,7 @@ MetricManager::tick(const MetricLockGuard & guard, time_t currentTime) } // Check for new config and reconfigure - if (_configSubscriber.get() && _configSubscriber->nextConfig(0)) { + if (_configSubscriber.get() && _configSubscriber->nextConfigNow()) { configure(guard, _configHandle->getConfig()); } @@ -841,8 +841,7 @@ MetricManager::tick(const MetricLockGuard & guard, time_t currentTime) visit(guard, *_totalMetrics, totalVisitor, "log"); visit(guard, _snapshots[0]->getSnapshot(), fiveMinVisitor , "log"); if (_logPeriod.second + _logPeriod.first < currentTime) { - uint64_t next = _snapshots[0]->getFromTime() - + _logPeriod.first; + uint64_t next = _snapshots[0]->getFromTime() + _logPeriod.first; LOG(warning, "Logged events at time %" PRIu64 " for time %" PRIu64 ". Since this is more than a period %u " "in the past, next run has been set to next " diff --git a/vespalib/src/vespa/vespalib/util/sync.h b/vespalib/src/vespa/vespalib/util/sync.h index 6461eaf9c3f..12961dffef7 100644 --- a/vespalib/src/vespa/vespalib/util/sync.h +++ b/vespalib/src/vespa/vespalib/util/sync.h @@ -376,7 +376,10 @@ public: * @return true if a signal was received, false if the wait timed out. **/ bool wait(int msTimeout) { - return _cond->wait_for(_guard, std::chrono::milliseconds(msTimeout)) == std::cv_status::no_timeout; + return wait(std::chrono::milliseconds(msTimeout)); + } + bool wait(std::chrono::milliseconds timeout) { + return _cond->wait_for(_guard, timeout) == std::cv_status::no_timeout; } /** * @brief Send a signal to a single waiter on the underlying @@ -404,19 +407,7 @@ public: _cond->notify_one(); _cond = nullptr; } - /** - * @brief Send a signal to all waiters on the underlying Monitor, - * but unlock the monitor right before doing so. - * - * This is inherently unsafe and the caller needs external - * synchronization to ensure that the underlying Monitor object - * will live long enough to be signaled. - **/ - void unsafeBroadcastUnlock() { - _guard.unlock(); - _cond->notify_all(); - _cond = nullptr; - } + /** * @brief Release the lock held by this object if unlock has not * been called. |