From 58ca4782441f060f8b33ef2d117aaee25e7b9c61 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 10 Jun 2021 08:54:33 +0000 Subject: add rpc method for connectivity report --- configd/src/apps/sentinel/CMakeLists.txt | 1 + configd/src/apps/sentinel/connectivity.cpp | 44 +++++++------- configd/src/apps/sentinel/connectivity.h | 1 + configd/src/apps/sentinel/peer-check.cpp | 2 +- configd/src/apps/sentinel/report-connectivity.cpp | 71 +++++++++++++++++++++++ configd/src/apps/sentinel/report-connectivity.h | 67 +++++++++++++++++++++ configd/src/apps/sentinel/rpchooks.cpp | 17 +++++- configd/src/apps/sentinel/rpchooks.h | 1 + 8 files changed, 180 insertions(+), 24 deletions(-) create mode 100644 configd/src/apps/sentinel/report-connectivity.cpp create mode 100644 configd/src/apps/sentinel/report-connectivity.h (limited to 'configd') diff --git a/configd/src/apps/sentinel/CMakeLists.txt b/configd/src/apps/sentinel/CMakeLists.txt index 43b4f79a0b2..0999314c8ce 100644 --- a/configd/src/apps/sentinel/CMakeLists.txt +++ b/configd/src/apps/sentinel/CMakeLists.txt @@ -12,6 +12,7 @@ vespa_add_executable(configd_config-sentinel_app output-connection.cpp outward-check.cpp peer-check.cpp + report-connectivity.cpp rpchooks.cpp rpcserver.cpp sentinel.cpp diff --git a/configd/src/apps/sentinel/connectivity.cpp b/configd/src/apps/sentinel/connectivity.cpp index 8d1aa0e9673..12b645dd589 100644 --- a/configd/src/apps/sentinel/connectivity.cpp +++ b/configd/src/apps/sentinel/connectivity.cpp @@ -45,28 +45,6 @@ std::string spec(const SpecMap::value_type &host_and_port) { return fmt("tcp/%s:%d", host_and_port.first.c_str(), host_and_port.second); } -SpecMap specsFrom(const ModelConfig &model) { - SpecMap checkSpecs; - for (const auto & h : model.hosts) { - bool foundSentinelPort = false; - for (const auto & s : h.services) { - if (s.name == "config-sentinel") { - for (const auto & p : s.ports) { - if (p.tags.find("rpc") != p.tags.npos) { - checkSpecs[h.name] = p.number; - foundSentinelPort = true; - } - } - } - } - if (! foundSentinelPort) { - LOG(warning, "Did not find 'config-sentinel' RPC port in model for host %s [%zd services]", - h.name.c_str(), h.services.size()); - } - } - return checkSpecs; -} - void classifyConnFails(ConnectivityMap &connectivityMap, const SpecMap &specMap, RpcServer &rpcServer) @@ -118,6 +96,28 @@ void classifyConnFails(ConnectivityMap &connectivityMap, } // namespace +SpecMap Connectivity::specsFrom(const ModelConfig &model) { + SpecMap checkSpecs; + for (const auto & h : model.hosts) { + bool foundSentinelPort = false; + for (const auto & s : h.services) { + if (s.name == "config-sentinel") { + for (const auto & p : s.ports) { + if (p.tags.find("rpc") != p.tags.npos) { + checkSpecs[h.name] = p.number; + foundSentinelPort = true; + } + } + } + } + if (! foundSentinelPort) { + LOG(warning, "Did not find 'config-sentinel' RPC port in model for host %s [%zd services]", + h.name.c_str(), h.services.size()); + } + } + return checkSpecs; +} + void Connectivity::configure(const SentinelConfig::Connectivity &config) { _config = config; LOG(config, "connectivity.maxBadReverseCount = %d", _config.maxBadReverseCount); diff --git a/configd/src/apps/sentinel/connectivity.h b/configd/src/apps/sentinel/connectivity.h index a1e454a255a..440d7df84c0 100644 --- a/configd/src/apps/sentinel/connectivity.h +++ b/configd/src/apps/sentinel/connectivity.h @@ -26,6 +26,7 @@ public: ~Connectivity(); void configure(const SentinelConfig::Connectivity &config); bool checkConnectivity(RpcServer &rpcServer); + static SpecMap specsFrom(const ModelConfig &model); private: struct Accumulated { size_t numIssues = 0; diff --git a/configd/src/apps/sentinel/peer-check.cpp b/configd/src/apps/sentinel/peer-check.cpp index 60c3d9c96c9..46196f36bf1 100644 --- a/configd/src/apps/sentinel/peer-check.cpp +++ b/configd/src/apps/sentinel/peer-check.cpp @@ -33,7 +33,7 @@ void PeerCheck::RequestDone(FRT_RPCRequest *req) { LOG_ASSERT(req == _req); bool statusOk = false; if (req->IsError()) { - LOG(warning, "error on ping to %s [port %d]: %s (%d)", _hostname.c_str(), _portnum, + LOG(debug, "error on ping to %s [port %d]: %s (%d)", _hostname.c_str(), _portnum, req->GetErrorMessage(), req->GetErrorCode()); } else { LOG(debug, "OK ping to %s [port %d]", _hostname.c_str(), _portnum); diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp new file mode 100644 index 00000000000..8e855f296c1 --- /dev/null +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -0,0 +1,71 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "report-connectivity.h" +#include "connectivity.h" +#include + +LOG_SETUP(".report-connectivity"); + +using cloud::config::ModelConfig; + +namespace config::sentinel { + +ConnectivityCheckResult::~ConnectivityCheckResult() = default; + +void ConnectivityCheckResult::returnStatus(bool ok) { + status = ok ? "ok" : "ping failed"; + LOG(debug, "peer %s [port %d] -> %s", peerName.c_str(), peerPort, status.c_str()); + parent.requestDone(); +} + +ConnectivityReportResult::~ConnectivityReportResult() = default; + + +ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb) + : _parentRequest(req), + _orb(orb), + _result(), + _configFetcher() +{ + _configFetcher.subscribe("admin/model", this); + _configFetcher.start(); +} + +ReportConnectivity::~ReportConnectivity() = default; + +void ReportConnectivity::requestDone() { + { + std::lock_guard guard(_lock); + if (--_remaining != 0) { + return; + } + } + finish(); +} + +void ReportConnectivity::configure(std::unique_ptr config) { + _configFetcher.close(); + auto map = Connectivity::specsFrom(*config); + for (const auto & [ hostname, port ] : map) { + _result.peers.emplace_back(*this, hostname, port); + } + LOG(debug, "making connectivity report for %zd peers", _result.peers.size()); + _remaining = _result.peers.size(); + for (auto & peer : _result.peers) { + peer.check = std::make_unique(peer, peer.peerName, peer.peerPort, _orb, 2500); + } +} + + +void ReportConnectivity::finish() const { + FRT_Values *dst = _parentRequest->GetReturn(); + FRT_StringValue *pt_hn = dst->AddStringArray(_result.peers.size()); + FRT_StringValue *pt_ss = dst->AddStringArray(_result.peers.size()); + for (const auto & peer : _result.peers) { + dst->SetString(pt_hn++, peer.peerName.c_str()); + dst->SetString(pt_ss++, peer.status.c_str()); + } + _parentRequest->Return(); +} + +} diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h new file mode 100644 index 00000000000..ac62627fe41 --- /dev/null +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -0,0 +1,67 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include +#include +#include +#include +#include "peer-check.h" +#include "status-callback.h" + +#include +#include +#include +#include + +namespace config::sentinel { + +class ReportConnectivity; + +struct ConnectivityCheckResult : StatusCallback { + ReportConnectivity& parent; + std::string peerName; + int peerPort; + std::string status; + std::unique_ptr check; + + ConnectivityCheckResult(ReportConnectivity& owner, const std::string &hostname, int port) + : parent(owner), + peerName(hostname), + peerPort(port), + status("unknown"), + check(nullptr) + {} + + ConnectivityCheckResult(ConnectivityCheckResult &&) = default; + ConnectivityCheckResult(const ConnectivityCheckResult &) = default; + + virtual ~ConnectivityCheckResult(); + void returnStatus(bool ok) override; +}; + +struct ConnectivityReportResult { + std::vector peers; + ~ConnectivityReportResult(); +}; + + +class ReportConnectivity : public config::IFetcherCallback +{ +public: + ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb); + ~ReportConnectivity(); + void requestDone(); + /** from IFetcherCallback */ + void configure(std::unique_ptr config) override; +private: + void finish() const; + FRT_RPCRequest *_parentRequest; + FRT_Supervisor &_orb; + ConnectivityReportResult _result; + config::ConfigFetcher _configFetcher; + std::mutex _lock; + size_t _remaining; +}; + +} diff --git a/configd/src/apps/sentinel/rpchooks.cpp b/configd/src/apps/sentinel/rpchooks.cpp index 24e3cd53509..98386beac3c 100644 --- a/configd/src/apps/sentinel/rpchooks.cpp +++ b/configd/src/apps/sentinel/rpchooks.cpp @@ -1,9 +1,10 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "rpchooks.h" -#include "cmdq.h" #include "check-completion-handler.h" +#include "cmdq.h" #include "peer-check.h" +#include "report-connectivity.h" #include #include @@ -53,6 +54,12 @@ RPCHooks::initRPC(FRT_Supervisor *supervisor) rb.ParamDesc("timeout", "Timeout for check in milliseconds"); rb.ReturnDesc("status", "Status (ok, bad, or unknown) for peer"); //------------------------------------------------------------------------- + rb.DefineMethod("sentinel.report.connectivity", "", "SS", + FRT_METHOD(RPCHooks::rpc_reportConnectivity), this); + rb.MethodDesc("report connectivity for peer sentinels"); + rb.ReturnDesc("hostnames", "Names of peers checked"); + rb.ReturnDesc("peerstatus", "Status description for each peer"); + //------------------------------------------------------------------------- } void @@ -106,4 +113,12 @@ RPCHooks::rpc_checkConnectivity(FRT_RPCRequest *req) req->getStash().create(completionHandler, hostname, portnum, _orb, timeout); } +void +RPCHooks::rpc_reportConnectivity(FRT_RPCRequest *req) +{ + LOG(debug, "got reportConnectivity"); + req->Detach(); + req->getStash().create(req, _orb); +} + } // namespace slobrok diff --git a/configd/src/apps/sentinel/rpchooks.h b/configd/src/apps/sentinel/rpchooks.h index 67f5804dcf7..5b6cf878f26 100644 --- a/configd/src/apps/sentinel/rpchooks.h +++ b/configd/src/apps/sentinel/rpchooks.h @@ -36,6 +36,7 @@ private: void rpc_stopService(FRT_RPCRequest *req); void rpc_startService(FRT_RPCRequest *req); void rpc_checkConnectivity(FRT_RPCRequest *req); + void rpc_reportConnectivity(FRT_RPCRequest *req); }; } // namespace config::sentinel -- cgit v1.2.3 From a7f980e01ca483c30b4533f3835bb9b60e5a78ca Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 10 Jun 2021 17:02:05 +0000 Subject: simplify some --- configd/src/apps/sentinel/report-connectivity.cpp | 21 +++++++++++---------- configd/src/apps/sentinel/report-connectivity.h | 8 ++------ 2 files changed, 13 insertions(+), 16 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index 8e855f296c1..fff9435abdf 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -18,8 +18,9 @@ void ConnectivityCheckResult::returnStatus(bool ok) { parent.requestDone(); } -ConnectivityReportResult::~ConnectivityReportResult() = default; - +void ConnectivityCheckResult::startCheck(FRT_Supervisor &orb) { + check = std::make_unique(*this, peerName, peerPort, orb, 2500); +} ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb) : _parentRequest(req), @@ -47,21 +48,21 @@ void ReportConnectivity::configure(std::unique_ptr config) { _configFetcher.close(); auto map = Connectivity::specsFrom(*config); for (const auto & [ hostname, port ] : map) { - _result.peers.emplace_back(*this, hostname, port); + _result.emplace_back(*this, hostname, port); } - LOG(debug, "making connectivity report for %zd peers", _result.peers.size()); - _remaining = _result.peers.size(); - for (auto & peer : _result.peers) { - peer.check = std::make_unique(peer, peer.peerName, peer.peerPort, _orb, 2500); + LOG(debug, "making connectivity report for %zd peers", _result.size()); + _remaining = _result.size(); + for (auto & peer : _result) { + peer.startCheck(_orb); } } void ReportConnectivity::finish() const { FRT_Values *dst = _parentRequest->GetReturn(); - FRT_StringValue *pt_hn = dst->AddStringArray(_result.peers.size()); - FRT_StringValue *pt_ss = dst->AddStringArray(_result.peers.size()); - for (const auto & peer : _result.peers) { + FRT_StringValue *pt_hn = dst->AddStringArray(_result.size()); + FRT_StringValue *pt_ss = dst->AddStringArray(_result.size()); + for (const auto & peer : _result) { dst->SetString(pt_hn++, peer.peerName.c_str()); dst->SetString(pt_ss++, peer.status.c_str()); } diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index ac62627fe41..d44e4eb5549 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -37,14 +37,10 @@ struct ConnectivityCheckResult : StatusCallback { ConnectivityCheckResult(const ConnectivityCheckResult &) = default; virtual ~ConnectivityCheckResult(); + void startCheck(FRT_Supervisor &orb); void returnStatus(bool ok) override; }; -struct ConnectivityReportResult { - std::vector peers; - ~ConnectivityReportResult(); -}; - class ReportConnectivity : public config::IFetcherCallback { @@ -58,7 +54,7 @@ private: void finish() const; FRT_RPCRequest *_parentRequest; FRT_Supervisor &_orb; - ConnectivityReportResult _result; + std::vector _result; config::ConfigFetcher _configFetcher; std::mutex _lock; size_t _remaining; -- cgit v1.2.3 From 7b5b9060f45edafff45c0a465ad19963dff017a4 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 10 Jun 2021 17:03:32 +0000 Subject: rename helper class --- configd/src/apps/sentinel/report-connectivity.cpp | 6 +++--- configd/src/apps/sentinel/report-connectivity.h | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index fff9435abdf..a08ea7c45d1 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -10,15 +10,15 @@ using cloud::config::ModelConfig; namespace config::sentinel { -ConnectivityCheckResult::~ConnectivityCheckResult() = default; +SinglePing::~SinglePing() = default; -void ConnectivityCheckResult::returnStatus(bool ok) { +void SinglePing::returnStatus(bool ok) { status = ok ? "ok" : "ping failed"; LOG(debug, "peer %s [port %d] -> %s", peerName.c_str(), peerPort, status.c_str()); parent.requestDone(); } -void ConnectivityCheckResult::startCheck(FRT_Supervisor &orb) { +void SinglePing::startCheck(FRT_Supervisor &orb) { check = std::make_unique(*this, peerName, peerPort, orb, 2500); } diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index d44e4eb5549..d28368eca6c 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -18,14 +18,14 @@ namespace config::sentinel { class ReportConnectivity; -struct ConnectivityCheckResult : StatusCallback { +struct SinglePing : StatusCallback { ReportConnectivity& parent; std::string peerName; int peerPort; std::string status; std::unique_ptr check; - ConnectivityCheckResult(ReportConnectivity& owner, const std::string &hostname, int port) + SinglePing(ReportConnectivity& owner, const std::string &hostname, int port) : parent(owner), peerName(hostname), peerPort(port), @@ -33,10 +33,10 @@ struct ConnectivityCheckResult : StatusCallback { check(nullptr) {} - ConnectivityCheckResult(ConnectivityCheckResult &&) = default; - ConnectivityCheckResult(const ConnectivityCheckResult &) = default; + SinglePing(SinglePing &&) = default; + SinglePing(const SinglePing &) = default; - virtual ~ConnectivityCheckResult(); + virtual ~SinglePing(); void startCheck(FRT_Supervisor &orb); void returnStatus(bool ok) override; }; @@ -54,7 +54,7 @@ private: void finish() const; FRT_RPCRequest *_parentRequest; FRT_Supervisor &_orb; - std::vector _result; + std::vector _result; config::ConfigFetcher _configFetcher; std::mutex _lock; size_t _remaining; -- cgit v1.2.3 From e607968dff6eee82f22f8aacc35e31d4afa2d807 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 07:24:24 +0000 Subject: handle failures fetching model config --- configd/src/apps/sentinel/report-connectivity.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index a08ea7c45d1..3e5d462060f 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -2,11 +2,14 @@ #include "report-connectivity.h" #include "connectivity.h" +#include #include +#include LOG_SETUP(".report-connectivity"); using cloud::config::ModelConfig; +using namespace std::chrono_literals; namespace config::sentinel { @@ -28,8 +31,19 @@ ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb) _result(), _configFetcher() { - _configFetcher.subscribe("admin/model", this); - _configFetcher.start(); + try { + _configFetcher.subscribe("admin/model", this, 2000ms); + _configFetcher.start(); + return; + } catch (ConfigTimeoutException & ex) { + LOG(warning, "Timeout getting model config: %s [no connectivity report]", ex.getMessage().c_str()); + } catch (InvalidConfigException& ex) { + LOG(warning, "Invalid model config: %s [no connectivity report]", ex.getMessage().c_str()); + } catch (ConfigRuntimeException& ex) { + LOG(warning, "Runtime exception getting model config: %s [no connectivity report]", ex.getMessage().c_str()); + } + _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); + _parentRequest->Return(); } ReportConnectivity::~ReportConnectivity() = default; -- cgit v1.2.3 From 6f8792b0acfa3e417d124b519b6383065a6bfd7a Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 12:57:24 +0000 Subject: add separate config subscriber for model --- configd/src/apps/sentinel/CMakeLists.txt | 1 + configd/src/apps/sentinel/config-owner.cpp | 43 +++++++++-------- configd/src/apps/sentinel/config-owner.h | 9 +++- configd/src/apps/sentinel/connectivity.cpp | 8 +-- configd/src/apps/sentinel/connectivity.h | 3 +- configd/src/apps/sentinel/env.cpp | 11 +++-- configd/src/apps/sentinel/env.h | 3 ++ configd/src/apps/sentinel/model-subscriber.cpp | 59 +++++++++++++++++++++++ configd/src/apps/sentinel/model-subscriber.h | 30 ++++++++++++ configd/src/apps/sentinel/report-connectivity.cpp | 45 ++++++----------- configd/src/apps/sentinel/report-connectivity.h | 8 ++- configd/src/apps/sentinel/rpchooks.cpp | 7 +-- configd/src/apps/sentinel/rpchooks.h | 4 +- configd/src/apps/sentinel/rpcserver.cpp | 4 +- configd/src/apps/sentinel/rpcserver.h | 3 +- 15 files changed, 166 insertions(+), 72 deletions(-) create mode 100644 configd/src/apps/sentinel/model-subscriber.cpp create mode 100644 configd/src/apps/sentinel/model-subscriber.h (limited to 'configd') diff --git a/configd/src/apps/sentinel/CMakeLists.txt b/configd/src/apps/sentinel/CMakeLists.txt index 0999314c8ce..3818f334ff2 100644 --- a/configd/src/apps/sentinel/CMakeLists.txt +++ b/configd/src/apps/sentinel/CMakeLists.txt @@ -9,6 +9,7 @@ vespa_add_executable(configd_config-sentinel_app line-splitter.cpp manager.cpp metrics.cpp + model-subscriber.cpp output-connection.cpp outward-check.cpp peer-check.cpp diff --git a/configd/src/apps/sentinel/config-owner.cpp b/configd/src/apps/sentinel/config-owner.cpp index 840c5b1add1..074f7187537 100644 --- a/configd/src/apps/sentinel/config-owner.cpp +++ b/configd/src/apps/sentinel/config-owner.cpp @@ -10,13 +10,22 @@ LOG_SETUP(".config-owner"); namespace config::sentinel { -ConfigOwner::ConfigOwner() : _subscriber() {} +ConfigOwner::ConfigOwner() = default; ConfigOwner::~ConfigOwner() = default; void ConfigOwner::subscribe(const std::string & configId, std::chrono::milliseconds timeout) { _sentinelHandle = _subscriber.subscribe(configId, timeout); + try { + _modelHandle =_modelSubscriber.subscribe("admin/model", timeout); + } catch (ConfigTimeoutException & ex) { + LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (InvalidConfigException& ex) { + LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (ConfigRuntimeException& ex) { + LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } } void @@ -29,6 +38,7 @@ ConfigOwner::doConfigure() const auto & app = config.application; LOG(config, "Sentinel got %zd service elements [tenant(%s), application(%s), instance(%s)] for config generation %" PRId64, config.service.size(), app.tenant.c_str(), app.name.c_str(), app.instance.c_str(), _currGeneration); + getModelConfig(); } @@ -42,29 +52,20 @@ ConfigOwner::checkForConfigUpdate() { return false; } -std::unique_ptr -ConfigOwner::fetchModelConfig(std::chrono::milliseconds timeout) -{ - std::unique_ptr modelConfig; - ConfigSubscriber tempSubscriber; - try { - ConfigHandle::UP modelHandle = - tempSubscriber.subscribe("admin/model", timeout); - if (tempSubscriber.nextGenerationNow()) { - modelConfig = modelHandle->getConfig(); +std::optional +ConfigOwner::getModelConfig() { + if (_modelHandle && _modelSubscriber.nextGenerationNow()) { + if (auto newModel = _modelHandle->getConfig()) { LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", - modelConfig->vespaVersion.c_str(), modelConfig->hosts.size(), - tempSubscriber.getGeneration()); + newModel->vespaVersion.c_str(), newModel->hosts.size(), _modelSubscriber.getGeneration()); + _modelConfig = std::move(newModel); } - } catch (ConfigTimeoutException & ex) { - LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (InvalidConfigException& ex) { - LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (ConfigRuntimeException& ex) { - LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } - return modelConfig; + if (_modelConfig) { + return ModelConfig(*_modelConfig); + } else { + return {}; + } } } diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h index 2850e6b3904..7b79f65b1de 100644 --- a/configd/src/apps/sentinel/config-owner.h +++ b/configd/src/apps/sentinel/config-owner.h @@ -5,6 +5,7 @@ #include #include #include +#include using cloud::config::SentinelConfig; using cloud::config::ModelConfig; @@ -21,10 +22,14 @@ class ConfigOwner { private: ConfigSubscriber _subscriber; ConfigHandle::UP _sentinelHandle; - + int64_t _currGeneration = -1; std::unique_ptr _currConfig; + ConfigSubscriber _modelSubscriber; + ConfigHandle::UP _modelHandle; + std::unique_ptr _modelConfig; + ConfigOwner(const ConfigOwner&) = delete; ConfigOwner& operator =(const ConfigOwner&) = delete; @@ -37,7 +42,7 @@ public: bool hasConfig() const { return _currConfig.get() != nullptr; } const SentinelConfig& getConfig() const { return *_currConfig; } int64_t getGeneration() const { return _currGeneration; } - static std::unique_ptr fetchModelConfig(std::chrono::milliseconds timeout); + std::optional getModelConfig(); }; } diff --git a/configd/src/apps/sentinel/connectivity.cpp b/configd/src/apps/sentinel/connectivity.cpp index 12b645dd589..7ea548ea73d 100644 --- a/configd/src/apps/sentinel/connectivity.cpp +++ b/configd/src/apps/sentinel/connectivity.cpp @@ -118,13 +118,13 @@ SpecMap Connectivity::specsFrom(const ModelConfig &model) { return checkSpecs; } -void Connectivity::configure(const SentinelConfig::Connectivity &config) { +void Connectivity::configure(const SentinelConfig::Connectivity &config, + const ModelConfig &model) +{ _config = config; LOG(config, "connectivity.maxBadReverseCount = %d", _config.maxBadReverseCount); LOG(config, "connectivity.maxBadOutPercent = %d", _config.maxBadOutPercent); - if (auto up = ConfigOwner::fetchModelConfig(MODEL_TIMEOUT_MS)) { - _checkSpecs = specsFrom(*up); - } + _checkSpecs = specsFrom(model); } bool diff --git a/configd/src/apps/sentinel/connectivity.h b/configd/src/apps/sentinel/connectivity.h index 440d7df84c0..2ba17f5c07c 100644 --- a/configd/src/apps/sentinel/connectivity.h +++ b/configd/src/apps/sentinel/connectivity.h @@ -24,7 +24,8 @@ public: Connectivity(); ~Connectivity(); - void configure(const SentinelConfig::Connectivity &config); + void configure(const SentinelConfig::Connectivity &config, + const ModelConfig &model); bool checkConnectivity(RpcServer &rpcServer); static SpecMap specsFrom(const ModelConfig &model); private: diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp index 5bbbfd8f0bd..58b917ca16e 100644 --- a/configd/src/apps/sentinel/env.cpp +++ b/configd/src/apps/sentinel/env.cpp @@ -36,6 +36,7 @@ constexpr int maxConnectivityRetries = 100; Env::Env() : _cfgOwner(), + _modelSubscriber("admin/model"), _rpcCommandQueue(), _rpcServer(), _stateApi(), @@ -52,6 +53,7 @@ Env::~Env() = default; void Env::boot(const std::string &configId) { LOG(debug, "Reading configuration for ID: %s", configId.c_str()); _cfgOwner.subscribe(configId, CONFIG_TIMEOUT_MS); + _modelSubscriber.start(CONFIG_TIMEOUT_MS); // subscribe() should throw if something is not OK Connectivity checker; for (int retry = 0; retry < maxConnectivityRetries; ++retry) { @@ -64,7 +66,10 @@ void Env::boot(const std::string &configId) { configId.c_str(), cfg.port.telnet, cfg.port.rpc); rpcPort(cfg.port.rpc); statePort(cfg.port.telnet); - checker.configure(cfg.connectivity); + auto model = _modelSubscriber.getModelConfig(); + if (model.has_value()) { + checker.configure(cfg.connectivity, model.value()); + } } if (checker.checkConnectivity(*_rpcServer)) { _stateApi.myHealth.setOk(); @@ -94,7 +99,7 @@ void Env::rpcPort(int port) { if (_rpcServer && port == _rpcServer->getPort()) { return; // ok already } - _rpcServer = std::make_unique(port, _rpcCommandQueue); + _rpcServer = std::make_unique(port, _rpcCommandQueue, _modelSubscriber); } void Env::statePort(int port) { @@ -116,7 +121,7 @@ void Env::statePort(int port) { void Env::notifyConfigUpdated() { vespalib::ComponentConfigProducer::Config current("sentinel", _cfgOwner.getGeneration(), "ok"); _stateApi.myComponents.addConfig(current); - + _modelSubscriber.checkForUpdates(); } void Env::respondAsEmpty() { diff --git a/configd/src/apps/sentinel/env.h b/configd/src/apps/sentinel/env.h index f71fb537068..9319caa4bd2 100644 --- a/configd/src/apps/sentinel/env.h +++ b/configd/src/apps/sentinel/env.h @@ -5,6 +5,7 @@ #include "cmdq.h" #include "config-owner.h" #include "metrics.h" +#include "model-subscriber.h" #include "rpcserver.h" #include "state-api.h" #include @@ -22,6 +23,7 @@ public: ~Env(); ConfigOwner &configOwner() { return _cfgOwner; } + ModelSubscriber &modelSubscriber() { return _modelSubscriber; } CommandQueue &commandQueue() { return _rpcCommandQueue; } StartMetrics &metrics() { return _startMetrics; } @@ -33,6 +35,7 @@ public: private: void respondAsEmpty(); ConfigOwner _cfgOwner; + ModelSubscriber _modelSubscriber; CommandQueue _rpcCommandQueue; std::unique_ptr _rpcServer; StateApi _stateApi; diff --git a/configd/src/apps/sentinel/model-subscriber.cpp b/configd/src/apps/sentinel/model-subscriber.cpp new file mode 100644 index 00000000000..f603b5fcb1b --- /dev/null +++ b/configd/src/apps/sentinel/model-subscriber.cpp @@ -0,0 +1,59 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "model-subscriber.h" +#include +#include +#include +#include +#include + +LOG_SETUP(".sentinel.model-subscriber"); + +using namespace std::chrono_literals; + +namespace config::sentinel { + +std::optional ModelSubscriber::getModelConfig() { + checkForUpdates(); + if (_modelConfig) { + return ModelConfig(*_modelConfig); + } else { + return {}; + } +} + + +ModelSubscriber::ModelSubscriber(const std::string &configId) + : _configId(configId) +{} + +ModelSubscriber::~ModelSubscriber() = default; + +void +ModelSubscriber::start(std::chrono::milliseconds timeout) { + try { + _modelHandle =_subscriber.subscribe(_configId, timeout); + } catch (ConfigTimeoutException & ex) { + LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (InvalidConfigException& ex) { + LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (ConfigRuntimeException& ex) { + LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } +} + +void +ModelSubscriber::checkForUpdates() { + if (! _modelHandle) { + start(5ms); + } + if (_modelHandle && _subscriber.nextGenerationNow()) { + if (auto newModel = _modelHandle->getConfig()) { + LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", + newModel->vespaVersion.c_str(), newModel->hosts.size(), _subscriber.getGeneration()); + _modelConfig = std::move(newModel); + } + } +} + +} diff --git a/configd/src/apps/sentinel/model-subscriber.h b/configd/src/apps/sentinel/model-subscriber.h new file mode 100644 index 00000000000..1777e287d4f --- /dev/null +++ b/configd/src/apps/sentinel/model-subscriber.h @@ -0,0 +1,30 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include +#include +#include + +using cloud::config::ModelConfig; + +namespace config::sentinel { + +/** + * Handles config subscription and has a snapshot of current config. + **/ +class ModelSubscriber { +private: + std::string _configId; + config::ConfigSubscriber _subscriber; + config::ConfigHandle::UP _modelHandle; + std::unique_ptr _modelConfig; +public: + ModelSubscriber(const std::string &configId); + virtual ~ModelSubscriber(); + void start(std::chrono::milliseconds timeout); + void checkForUpdates(); + std::optional getModelConfig(); +}; + +} diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index 3e5d462060f..e23db85f4e2 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -25,25 +25,26 @@ void SinglePing::startCheck(FRT_Supervisor &orb) { check = std::make_unique(*this, peerName, peerPort, orb, 2500); } -ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb) +ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber) : _parentRequest(req), _orb(orb), - _result(), - _configFetcher() + _result() { - try { - _configFetcher.subscribe("admin/model", this, 2000ms); - _configFetcher.start(); - return; - } catch (ConfigTimeoutException & ex) { - LOG(warning, "Timeout getting model config: %s [no connectivity report]", ex.getMessage().c_str()); - } catch (InvalidConfigException& ex) { - LOG(warning, "Invalid model config: %s [no connectivity report]", ex.getMessage().c_str()); - } catch (ConfigRuntimeException& ex) { - LOG(warning, "Runtime exception getting model config: %s [no connectivity report]", ex.getMessage().c_str()); + auto cfg = modelSubscriber.getModelConfig(); + if (cfg.has_value()) { + auto map = Connectivity::specsFrom(cfg.value()); + LOG(debug, "making connectivity report for %zd peers", _result.size()); + _remaining = _result.size(); + for (const auto & [ hostname, port ] : map) { + _result.emplace_back(*this, hostname, port); + } + for (auto & peer : _result) { + peer.startCheck(_orb); + } + } else { + _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); + _parentRequest->Return(); } - _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); - _parentRequest->Return(); } ReportConnectivity::~ReportConnectivity() = default; @@ -58,20 +59,6 @@ void ReportConnectivity::requestDone() { finish(); } -void ReportConnectivity::configure(std::unique_ptr config) { - _configFetcher.close(); - auto map = Connectivity::specsFrom(*config); - for (const auto & [ hostname, port ] : map) { - _result.emplace_back(*this, hostname, port); - } - LOG(debug, "making connectivity report for %zd peers", _result.size()); - _remaining = _result.size(); - for (auto & peer : _result) { - peer.startCheck(_orb); - } -} - - void ReportConnectivity::finish() const { FRT_Values *dst = _parentRequest->GetReturn(); FRT_StringValue *pt_hn = dst->AddStringArray(_result.size()); diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index d28368eca6c..ed012e2e07e 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -6,6 +6,7 @@ #include #include #include +#include "model-subscriber.h" #include "peer-check.h" #include "status-callback.h" @@ -42,20 +43,17 @@ struct SinglePing : StatusCallback { }; -class ReportConnectivity : public config::IFetcherCallback +class ReportConnectivity { public: - ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb); + ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber); ~ReportConnectivity(); void requestDone(); - /** from IFetcherCallback */ - void configure(std::unique_ptr config) override; private: void finish() const; FRT_RPCRequest *_parentRequest; FRT_Supervisor &_orb; std::vector _result; - config::ConfigFetcher _configFetcher; std::mutex _lock; size_t _remaining; }; diff --git a/configd/src/apps/sentinel/rpchooks.cpp b/configd/src/apps/sentinel/rpchooks.cpp index 98386beac3c..a8d0240e6e8 100644 --- a/configd/src/apps/sentinel/rpchooks.cpp +++ b/configd/src/apps/sentinel/rpchooks.cpp @@ -13,9 +13,10 @@ LOG_SETUP(".rpchooks"); namespace config::sentinel { -RPCHooks::RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor) +RPCHooks::RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelSubscriber &modelSubscriber) : _commands(commands), - _orb(supervisor) + _orb(supervisor), + _modelSubscriber(modelSubscriber) { initRPC(&_orb); } @@ -118,7 +119,7 @@ RPCHooks::rpc_reportConnectivity(FRT_RPCRequest *req) { LOG(debug, "got reportConnectivity"); req->Detach(); - req->getStash().create(req, _orb); + req->getStash().create(req, _orb, _modelSubscriber); } } // namespace slobrok diff --git a/configd/src/apps/sentinel/rpchooks.h b/configd/src/apps/sentinel/rpchooks.h index 5b6cf878f26..badfd560034 100644 --- a/configd/src/apps/sentinel/rpchooks.h +++ b/configd/src/apps/sentinel/rpchooks.h @@ -2,6 +2,7 @@ #pragma once +#include "model-subscriber.h" #include #include @@ -25,8 +26,9 @@ class RPCHooks : public FRT_Invokable private: CommandQueue &_commands; FRT_Supervisor &_orb; + ModelSubscriber &_modelSubscriber; public: - RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor); + RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelSubscriber &modelSubscriber); ~RPCHooks() override; private: void initRPC(FRT_Supervisor *supervisor); diff --git a/configd/src/apps/sentinel/rpcserver.cpp b/configd/src/apps/sentinel/rpcserver.cpp index 80c3c81c826..18b0dfc3630 100644 --- a/configd/src/apps/sentinel/rpcserver.cpp +++ b/configd/src/apps/sentinel/rpcserver.cpp @@ -7,9 +7,9 @@ LOG_SETUP(".rpcserver"); namespace config::sentinel { -RpcServer::RpcServer(int portNumber, CommandQueue &cmdQ) +RpcServer::RpcServer(int portNumber, CommandQueue &cmdQ, ModelSubscriber &modelSubscriber) : _server(), - _rpcHooks(cmdQ, _server.supervisor()), + _rpcHooks(cmdQ, _server.supervisor(), modelSubscriber), _port(portNumber) { if (_server.supervisor().Listen(portNumber)) { diff --git a/configd/src/apps/sentinel/rpcserver.h b/configd/src/apps/sentinel/rpcserver.h index 4c6dea00ddf..e2c7caaacb1 100644 --- a/configd/src/apps/sentinel/rpcserver.h +++ b/configd/src/apps/sentinel/rpcserver.h @@ -5,6 +5,7 @@ #include #include "cmdq.h" +#include "model-subscriber.h" #include "rpchooks.h" #include @@ -18,7 +19,7 @@ private: int _port; public: - RpcServer(int port, CommandQueue &cmdQ); + RpcServer(int port, CommandQueue &cmdQ, ModelSubscriber &modelSubscriber); ~RpcServer(); int getPort() const { return _port; } -- cgit v1.2.3 From e97e12fec351222ff111c066a1dd8c831e436a36 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 13:08:42 +0000 Subject: open up with more APIs --- configd/src/apps/sentinel/peer-check.cpp | 8 ++++---- configd/src/apps/sentinel/peer-check.h | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/peer-check.cpp b/configd/src/apps/sentinel/peer-check.cpp index 46196f36bf1..daab80eefc7 100644 --- a/configd/src/apps/sentinel/peer-check.cpp +++ b/configd/src/apps/sentinel/peer-check.cpp @@ -15,7 +15,8 @@ PeerCheck::PeerCheck(StatusCallback &callback, const std::string &host, int port _hostname(host), _portnum(port), _target(nullptr), - _req(nullptr) + _req(nullptr), + _statusOk(false) { auto spec = fmt("tcp/%s:%d", _hostname.c_str(), _portnum); _target = orb.GetTarget(spec.c_str()); @@ -31,20 +32,19 @@ PeerCheck::~PeerCheck() { void PeerCheck::RequestDone(FRT_RPCRequest *req) { LOG_ASSERT(req == _req); - bool statusOk = false; if (req->IsError()) { LOG(debug, "error on ping to %s [port %d]: %s (%d)", _hostname.c_str(), _portnum, req->GetErrorMessage(), req->GetErrorCode()); } else { LOG(debug, "OK ping to %s [port %d]", _hostname.c_str(), _portnum); - statusOk = true; + _statusOk = true; } _req->SubRef(); _req = nullptr; _target->SubRef(); _target = nullptr; // Note: will delete this object, so must be called as final step: - _callback.returnStatus(statusOk); + _callback.returnStatus(_statusOk); } } diff --git a/configd/src/apps/sentinel/peer-check.h b/configd/src/apps/sentinel/peer-check.h index 096f304467b..ac124106387 100644 --- a/configd/src/apps/sentinel/peer-check.h +++ b/configd/src/apps/sentinel/peer-check.h @@ -17,6 +17,9 @@ public: PeerCheck(StatusCallback &callback, const std::string &host, int portnum, FRT_Supervisor &orb, int timeout_ms); ~PeerCheck(); + bool okStatus() const { return _statusOk; } + const std::string& getHostname() const { return _hostname; } + PeerCheck(const PeerCheck &) = delete; PeerCheck(PeerCheck &&) = delete; PeerCheck& operator= (const PeerCheck &) = delete; @@ -30,6 +33,7 @@ private: int _portnum; FRT_Target *_target; FRT_RPCRequest *_req; + bool _statusOk; }; } -- cgit v1.2.3 From d32a461a49354922d81e55aabddbd13cc8902c30 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 13:08:49 +0000 Subject: simplify using new PeerCheck apis --- configd/src/apps/sentinel/report-connectivity.cpp | 42 +++++++---------------- configd/src/apps/sentinel/report-connectivity.h | 39 ++++----------------- 2 files changed, 18 insertions(+), 63 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index e23db85f4e2..a98bbdc0eec 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -13,33 +13,19 @@ using namespace std::chrono_literals; namespace config::sentinel { -SinglePing::~SinglePing() = default; - -void SinglePing::returnStatus(bool ok) { - status = ok ? "ok" : "ping failed"; - LOG(debug, "peer %s [port %d] -> %s", peerName.c_str(), peerPort, status.c_str()); - parent.requestDone(); -} - -void SinglePing::startCheck(FRT_Supervisor &orb) { - check = std::make_unique(*this, peerName, peerPort, orb, 2500); -} ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber) : _parentRequest(req), _orb(orb), - _result() + _checks() { auto cfg = modelSubscriber.getModelConfig(); if (cfg.has_value()) { auto map = Connectivity::specsFrom(cfg.value()); - LOG(debug, "making connectivity report for %zd peers", _result.size()); - _remaining = _result.size(); + LOG(debug, "making connectivity report for %zd peers", map.size()); + _remaining = map.size(); for (const auto & [ hostname, port ] : map) { - _result.emplace_back(*this, hostname, port); - } - for (auto & peer : _result) { - peer.startCheck(_orb); + _checks.emplace_back(std::make_unique(*this, hostname, port, _orb, 2500)); } } else { _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); @@ -49,23 +35,19 @@ ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ReportConnectivity::~ReportConnectivity() = default; -void ReportConnectivity::requestDone() { - { - std::lock_guard guard(_lock); - if (--_remaining != 0) { - return; - } +void ReportConnectivity::returnStatus(bool) { + if (--_remaining == 0) { + finish(); } - finish(); } void ReportConnectivity::finish() const { FRT_Values *dst = _parentRequest->GetReturn(); - FRT_StringValue *pt_hn = dst->AddStringArray(_result.size()); - FRT_StringValue *pt_ss = dst->AddStringArray(_result.size()); - for (const auto & peer : _result) { - dst->SetString(pt_hn++, peer.peerName.c_str()); - dst->SetString(pt_ss++, peer.status.c_str()); + FRT_StringValue *pt_hn = dst->AddStringArray(_checks.size()); + FRT_StringValue *pt_ss = dst->AddStringArray(_checks.size()); + for (const auto & peer : _checks) { + dst->SetString(pt_hn++, peer->getHostname().c_str()); + dst->SetString(pt_ss++, peer->okStatus() ? "ok" : "ping failed"); } _parentRequest->Return(); } diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index ed012e2e07e..19b2d74fce1 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -10,52 +10,25 @@ #include "peer-check.h" #include "status-callback.h" +#include #include -#include #include #include namespace config::sentinel { -class ReportConnectivity; - -struct SinglePing : StatusCallback { - ReportConnectivity& parent; - std::string peerName; - int peerPort; - std::string status; - std::unique_ptr check; - - SinglePing(ReportConnectivity& owner, const std::string &hostname, int port) - : parent(owner), - peerName(hostname), - peerPort(port), - status("unknown"), - check(nullptr) - {} - - SinglePing(SinglePing &&) = default; - SinglePing(const SinglePing &) = default; - - virtual ~SinglePing(); - void startCheck(FRT_Supervisor &orb); - void returnStatus(bool ok) override; -}; - - -class ReportConnectivity +class ReportConnectivity : public StatusCallback { public: ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber); - ~ReportConnectivity(); - void requestDone(); + virtual ~ReportConnectivity(); + void returnStatus(bool ok) override; private: void finish() const; FRT_RPCRequest *_parentRequest; FRT_Supervisor &_orb; - std::vector _result; - std::mutex _lock; - size_t _remaining; + std::vector> _checks; + std::atomic _remaining; }; } -- cgit v1.2.3 From 714a924518bc5bef4f920899d3af96b56497eaf0 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 13:24:18 +0000 Subject: keep naming convention --- configd/src/apps/sentinel/CMakeLists.txt | 2 +- configd/src/apps/sentinel/config-owner.cpp | 6 +-- configd/src/apps/sentinel/config-owner.h | 2 +- configd/src/apps/sentinel/env.cpp | 10 ++-- configd/src/apps/sentinel/env.h | 6 +-- configd/src/apps/sentinel/model-owner.cpp | 59 +++++++++++++++++++++++ configd/src/apps/sentinel/model-owner.h | 30 ++++++++++++ configd/src/apps/sentinel/model-subscriber.cpp | 59 ----------------------- configd/src/apps/sentinel/model-subscriber.h | 30 ------------ configd/src/apps/sentinel/report-connectivity.cpp | 4 +- configd/src/apps/sentinel/report-connectivity.h | 4 +- configd/src/apps/sentinel/rpchooks.cpp | 6 +-- configd/src/apps/sentinel/rpchooks.h | 6 +-- configd/src/apps/sentinel/rpcserver.cpp | 4 +- configd/src/apps/sentinel/rpcserver.h | 4 +- 15 files changed, 116 insertions(+), 116 deletions(-) create mode 100644 configd/src/apps/sentinel/model-owner.cpp create mode 100644 configd/src/apps/sentinel/model-owner.h delete mode 100644 configd/src/apps/sentinel/model-subscriber.cpp delete mode 100644 configd/src/apps/sentinel/model-subscriber.h (limited to 'configd') diff --git a/configd/src/apps/sentinel/CMakeLists.txt b/configd/src/apps/sentinel/CMakeLists.txt index 3818f334ff2..0323df2864f 100644 --- a/configd/src/apps/sentinel/CMakeLists.txt +++ b/configd/src/apps/sentinel/CMakeLists.txt @@ -9,7 +9,7 @@ vespa_add_executable(configd_config-sentinel_app line-splitter.cpp manager.cpp metrics.cpp - model-subscriber.cpp + model-owner.cpp output-connection.cpp outward-check.cpp peer-check.cpp diff --git a/configd/src/apps/sentinel/config-owner.cpp b/configd/src/apps/sentinel/config-owner.cpp index 074f7187537..4042eded24e 100644 --- a/configd/src/apps/sentinel/config-owner.cpp +++ b/configd/src/apps/sentinel/config-owner.cpp @@ -18,7 +18,7 @@ void ConfigOwner::subscribe(const std::string & configId, std::chrono::milliseconds timeout) { _sentinelHandle = _subscriber.subscribe(configId, timeout); try { - _modelHandle =_modelSubscriber.subscribe("admin/model", timeout); + _modelHandle =_modelOwner.subscribe("admin/model", timeout); } catch (ConfigTimeoutException & ex) { LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); } catch (InvalidConfigException& ex) { @@ -54,10 +54,10 @@ ConfigOwner::checkForConfigUpdate() { std::optional ConfigOwner::getModelConfig() { - if (_modelHandle && _modelSubscriber.nextGenerationNow()) { + if (_modelHandle && _modelOwner.nextGenerationNow()) { if (auto newModel = _modelHandle->getConfig()) { LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", - newModel->vespaVersion.c_str(), newModel->hosts.size(), _modelSubscriber.getGeneration()); + newModel->vespaVersion.c_str(), newModel->hosts.size(), _modelOwner.getGeneration()); _modelConfig = std::move(newModel); } } diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h index 7b79f65b1de..b27c515e357 100644 --- a/configd/src/apps/sentinel/config-owner.h +++ b/configd/src/apps/sentinel/config-owner.h @@ -26,7 +26,7 @@ private: int64_t _currGeneration = -1; std::unique_ptr _currConfig; - ConfigSubscriber _modelSubscriber; + ConfigSubscriber _modelOwner; ConfigHandle::UP _modelHandle; std::unique_ptr _modelConfig; diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp index 58b917ca16e..3fd9849023d 100644 --- a/configd/src/apps/sentinel/env.cpp +++ b/configd/src/apps/sentinel/env.cpp @@ -36,7 +36,7 @@ constexpr int maxConnectivityRetries = 100; Env::Env() : _cfgOwner(), - _modelSubscriber("admin/model"), + _modelOwner("admin/model"), _rpcCommandQueue(), _rpcServer(), _stateApi(), @@ -53,7 +53,7 @@ Env::~Env() = default; void Env::boot(const std::string &configId) { LOG(debug, "Reading configuration for ID: %s", configId.c_str()); _cfgOwner.subscribe(configId, CONFIG_TIMEOUT_MS); - _modelSubscriber.start(CONFIG_TIMEOUT_MS); + _modelOwner.start(CONFIG_TIMEOUT_MS); // subscribe() should throw if something is not OK Connectivity checker; for (int retry = 0; retry < maxConnectivityRetries; ++retry) { @@ -66,7 +66,7 @@ void Env::boot(const std::string &configId) { configId.c_str(), cfg.port.telnet, cfg.port.rpc); rpcPort(cfg.port.rpc); statePort(cfg.port.telnet); - auto model = _modelSubscriber.getModelConfig(); + auto model = _modelOwner.getModelConfig(); if (model.has_value()) { checker.configure(cfg.connectivity, model.value()); } @@ -99,7 +99,7 @@ void Env::rpcPort(int port) { if (_rpcServer && port == _rpcServer->getPort()) { return; // ok already } - _rpcServer = std::make_unique(port, _rpcCommandQueue, _modelSubscriber); + _rpcServer = std::make_unique(port, _rpcCommandQueue, _modelOwner); } void Env::statePort(int port) { @@ -121,7 +121,7 @@ void Env::statePort(int port) { void Env::notifyConfigUpdated() { vespalib::ComponentConfigProducer::Config current("sentinel", _cfgOwner.getGeneration(), "ok"); _stateApi.myComponents.addConfig(current); - _modelSubscriber.checkForUpdates(); + _modelOwner.checkForUpdates(); } void Env::respondAsEmpty() { diff --git a/configd/src/apps/sentinel/env.h b/configd/src/apps/sentinel/env.h index 9319caa4bd2..1bd3a7380ba 100644 --- a/configd/src/apps/sentinel/env.h +++ b/configd/src/apps/sentinel/env.h @@ -5,7 +5,7 @@ #include "cmdq.h" #include "config-owner.h" #include "metrics.h" -#include "model-subscriber.h" +#include "model-owner.h" #include "rpcserver.h" #include "state-api.h" #include @@ -23,7 +23,7 @@ public: ~Env(); ConfigOwner &configOwner() { return _cfgOwner; } - ModelSubscriber &modelSubscriber() { return _modelSubscriber; } + ModelOwner &modelOwner() { return _modelOwner; } CommandQueue &commandQueue() { return _rpcCommandQueue; } StartMetrics &metrics() { return _startMetrics; } @@ -35,7 +35,7 @@ public: private: void respondAsEmpty(); ConfigOwner _cfgOwner; - ModelSubscriber _modelSubscriber; + ModelOwner _modelOwner; CommandQueue _rpcCommandQueue; std::unique_ptr _rpcServer; StateApi _stateApi; diff --git a/configd/src/apps/sentinel/model-owner.cpp b/configd/src/apps/sentinel/model-owner.cpp new file mode 100644 index 00000000000..601a57e16c9 --- /dev/null +++ b/configd/src/apps/sentinel/model-owner.cpp @@ -0,0 +1,59 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "model-owner.h" +#include +#include +#include +#include +#include + +LOG_SETUP(".sentinel.model-owner"); + +using namespace std::chrono_literals; + +namespace config::sentinel { + +std::optional ModelOwner::getModelConfig() { + checkForUpdates(); + if (_modelConfig) { + return ModelConfig(*_modelConfig); + } else { + return {}; + } +} + + +ModelOwner::ModelOwner(const std::string &configId) + : _configId(configId) +{} + +ModelOwner::~ModelOwner() = default; + +void +ModelOwner::start(std::chrono::milliseconds timeout) { + try { + _modelHandle =_subscriber.subscribe(_configId, timeout); + } catch (ConfigTimeoutException & ex) { + LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (InvalidConfigException& ex) { + LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } catch (ConfigRuntimeException& ex) { + LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + } +} + +void +ModelOwner::checkForUpdates() { + if (! _modelHandle) { + start(5ms); + } + if (_modelHandle && _subscriber.nextGenerationNow()) { + if (auto newModel = _modelHandle->getConfig()) { + LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", + newModel->vespaVersion.c_str(), newModel->hosts.size(), _subscriber.getGeneration()); + _modelConfig = std::move(newModel); + } + } +} + +} diff --git a/configd/src/apps/sentinel/model-owner.h b/configd/src/apps/sentinel/model-owner.h new file mode 100644 index 00000000000..d03e7dec06d --- /dev/null +++ b/configd/src/apps/sentinel/model-owner.h @@ -0,0 +1,30 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include +#include +#include + +using cloud::config::ModelConfig; + +namespace config::sentinel { + +/** + * Handles config subscription and has a snapshot of current config. + **/ +class ModelOwner { +private: + std::string _configId; + config::ConfigSubscriber _subscriber; + config::ConfigHandle::UP _modelHandle; + std::unique_ptr _modelConfig; +public: + ModelOwner(const std::string &configId); + virtual ~ModelOwner(); + void start(std::chrono::milliseconds timeout); + void checkForUpdates(); + std::optional getModelConfig(); +}; + +} diff --git a/configd/src/apps/sentinel/model-subscriber.cpp b/configd/src/apps/sentinel/model-subscriber.cpp deleted file mode 100644 index f603b5fcb1b..00000000000 --- a/configd/src/apps/sentinel/model-subscriber.cpp +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "model-subscriber.h" -#include -#include -#include -#include -#include - -LOG_SETUP(".sentinel.model-subscriber"); - -using namespace std::chrono_literals; - -namespace config::sentinel { - -std::optional ModelSubscriber::getModelConfig() { - checkForUpdates(); - if (_modelConfig) { - return ModelConfig(*_modelConfig); - } else { - return {}; - } -} - - -ModelSubscriber::ModelSubscriber(const std::string &configId) - : _configId(configId) -{} - -ModelSubscriber::~ModelSubscriber() = default; - -void -ModelSubscriber::start(std::chrono::milliseconds timeout) { - try { - _modelHandle =_subscriber.subscribe(_configId, timeout); - } catch (ConfigTimeoutException & ex) { - LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (InvalidConfigException& ex) { - LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (ConfigRuntimeException& ex) { - LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } -} - -void -ModelSubscriber::checkForUpdates() { - if (! _modelHandle) { - start(5ms); - } - if (_modelHandle && _subscriber.nextGenerationNow()) { - if (auto newModel = _modelHandle->getConfig()) { - LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", - newModel->vespaVersion.c_str(), newModel->hosts.size(), _subscriber.getGeneration()); - _modelConfig = std::move(newModel); - } - } -} - -} diff --git a/configd/src/apps/sentinel/model-subscriber.h b/configd/src/apps/sentinel/model-subscriber.h deleted file mode 100644 index 1777e287d4f..00000000000 --- a/configd/src/apps/sentinel/model-subscriber.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include -#include -#include - -using cloud::config::ModelConfig; - -namespace config::sentinel { - -/** - * Handles config subscription and has a snapshot of current config. - **/ -class ModelSubscriber { -private: - std::string _configId; - config::ConfigSubscriber _subscriber; - config::ConfigHandle::UP _modelHandle; - std::unique_ptr _modelConfig; -public: - ModelSubscriber(const std::string &configId); - virtual ~ModelSubscriber(); - void start(std::chrono::milliseconds timeout); - void checkForUpdates(); - std::optional getModelConfig(); -}; - -} diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index a98bbdc0eec..4b5d38ed125 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -14,12 +14,12 @@ using namespace std::chrono_literals; namespace config::sentinel { -ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber) +ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelOwner &modelOwner) : _parentRequest(req), _orb(orb), _checks() { - auto cfg = modelSubscriber.getModelConfig(); + auto cfg = modelOwner.getModelConfig(); if (cfg.has_value()) { auto map = Connectivity::specsFrom(cfg.value()); LOG(debug, "making connectivity report for %zd peers", map.size()); diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index 19b2d74fce1..b7b8100c6fa 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -6,7 +6,7 @@ #include #include #include -#include "model-subscriber.h" +#include "model-owner.h" #include "peer-check.h" #include "status-callback.h" @@ -20,7 +20,7 @@ namespace config::sentinel { class ReportConnectivity : public StatusCallback { public: - ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelSubscriber &modelSubscriber); + ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelOwner &modelOwner); virtual ~ReportConnectivity(); void returnStatus(bool ok) override; private: diff --git a/configd/src/apps/sentinel/rpchooks.cpp b/configd/src/apps/sentinel/rpchooks.cpp index a8d0240e6e8..603fb461fe2 100644 --- a/configd/src/apps/sentinel/rpchooks.cpp +++ b/configd/src/apps/sentinel/rpchooks.cpp @@ -13,10 +13,10 @@ LOG_SETUP(".rpchooks"); namespace config::sentinel { -RPCHooks::RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelSubscriber &modelSubscriber) +RPCHooks::RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelOwner &modelOwner) : _commands(commands), _orb(supervisor), - _modelSubscriber(modelSubscriber) + _modelOwner(modelOwner) { initRPC(&_orb); } @@ -119,7 +119,7 @@ RPCHooks::rpc_reportConnectivity(FRT_RPCRequest *req) { LOG(debug, "got reportConnectivity"); req->Detach(); - req->getStash().create(req, _orb, _modelSubscriber); + req->getStash().create(req, _orb, _modelOwner); } } // namespace slobrok diff --git a/configd/src/apps/sentinel/rpchooks.h b/configd/src/apps/sentinel/rpchooks.h index badfd560034..292e8198b55 100644 --- a/configd/src/apps/sentinel/rpchooks.h +++ b/configd/src/apps/sentinel/rpchooks.h @@ -2,7 +2,7 @@ #pragma once -#include "model-subscriber.h" +#include "model-owner.h" #include #include @@ -26,9 +26,9 @@ class RPCHooks : public FRT_Invokable private: CommandQueue &_commands; FRT_Supervisor &_orb; - ModelSubscriber &_modelSubscriber; + ModelOwner &_modelOwner; public: - RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelSubscriber &modelSubscriber); + RPCHooks(CommandQueue &commands, FRT_Supervisor &supervisor, ModelOwner &modelOwner); ~RPCHooks() override; private: void initRPC(FRT_Supervisor *supervisor); diff --git a/configd/src/apps/sentinel/rpcserver.cpp b/configd/src/apps/sentinel/rpcserver.cpp index 18b0dfc3630..be867ae95db 100644 --- a/configd/src/apps/sentinel/rpcserver.cpp +++ b/configd/src/apps/sentinel/rpcserver.cpp @@ -7,9 +7,9 @@ LOG_SETUP(".rpcserver"); namespace config::sentinel { -RpcServer::RpcServer(int portNumber, CommandQueue &cmdQ, ModelSubscriber &modelSubscriber) +RpcServer::RpcServer(int portNumber, CommandQueue &cmdQ, ModelOwner &modelOwner) : _server(), - _rpcHooks(cmdQ, _server.supervisor(), modelSubscriber), + _rpcHooks(cmdQ, _server.supervisor(), modelOwner), _port(portNumber) { if (_server.supervisor().Listen(portNumber)) { diff --git a/configd/src/apps/sentinel/rpcserver.h b/configd/src/apps/sentinel/rpcserver.h index e2c7caaacb1..8f60acce1ca 100644 --- a/configd/src/apps/sentinel/rpcserver.h +++ b/configd/src/apps/sentinel/rpcserver.h @@ -5,7 +5,7 @@ #include #include "cmdq.h" -#include "model-subscriber.h" +#include "model-owner.h" #include "rpchooks.h" #include @@ -19,7 +19,7 @@ private: int _port; public: - RpcServer(int port, CommandQueue &cmdQ, ModelSubscriber &modelSubscriber); + RpcServer(int port, CommandQueue &cmdQ, ModelOwner &modelOwner); ~RpcServer(); int getPort() const { return _port; } -- cgit v1.2.3 From b54d4da7d22b2e27618bb2bb8865429e01ddab95 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 13:53:26 +0000 Subject: take timeout as RPC parameter here also --- configd/src/apps/sentinel/report-connectivity.cpp | 5 ++--- configd/src/apps/sentinel/report-connectivity.h | 2 +- configd/src/apps/sentinel/rpchooks.cpp | 7 +++++-- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index 4b5d38ed125..eceb2cf64aa 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -13,8 +13,7 @@ using namespace std::chrono_literals; namespace config::sentinel { - -ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelOwner &modelOwner) +ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, int timeout_ms, FRT_Supervisor &orb, ModelOwner &modelOwner) : _parentRequest(req), _orb(orb), _checks() @@ -25,7 +24,7 @@ ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, LOG(debug, "making connectivity report for %zd peers", map.size()); _remaining = map.size(); for (const auto & [ hostname, port ] : map) { - _checks.emplace_back(std::make_unique(*this, hostname, port, _orb, 2500)); + _checks.emplace_back(std::make_unique(*this, hostname, port, _orb, timeout_ms)); } } else { _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index b7b8100c6fa..057a374a387 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -20,7 +20,7 @@ namespace config::sentinel { class ReportConnectivity : public StatusCallback { public: - ReportConnectivity(FRT_RPCRequest *req, FRT_Supervisor &orb, ModelOwner &modelOwner); + ReportConnectivity(FRT_RPCRequest *req, int timeout_ms, FRT_Supervisor &orb, ModelOwner &modelOwner); virtual ~ReportConnectivity(); void returnStatus(bool ok) override; private: diff --git a/configd/src/apps/sentinel/rpchooks.cpp b/configd/src/apps/sentinel/rpchooks.cpp index 603fb461fe2..0d49063db84 100644 --- a/configd/src/apps/sentinel/rpchooks.cpp +++ b/configd/src/apps/sentinel/rpchooks.cpp @@ -55,9 +55,10 @@ RPCHooks::initRPC(FRT_Supervisor *supervisor) rb.ParamDesc("timeout", "Timeout for check in milliseconds"); rb.ReturnDesc("status", "Status (ok, bad, or unknown) for peer"); //------------------------------------------------------------------------- - rb.DefineMethod("sentinel.report.connectivity", "", "SS", + rb.DefineMethod("sentinel.report.connectivity", "i", "SS", FRT_METHOD(RPCHooks::rpc_reportConnectivity), this); rb.MethodDesc("report connectivity for peer sentinels"); + rb.ParamDesc("timeout", "Timeout for check in milliseconds"); rb.ReturnDesc("hostnames", "Names of peers checked"); rb.ReturnDesc("peerstatus", "Status description for each peer"); //------------------------------------------------------------------------- @@ -118,8 +119,10 @@ void RPCHooks::rpc_reportConnectivity(FRT_RPCRequest *req) { LOG(debug, "got reportConnectivity"); + FRT_Values &args = *req->GetParams(); + int timeout = args[0]._intval32; req->Detach(); - req->getStash().create(req, _orb, _modelOwner); + req->getStash().create(req, timeout, _orb, _modelOwner); } } // namespace slobrok -- cgit v1.2.3 From eb4a57b1b344cc4d83ab2ed2a5cc6c740dd774e4 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 13:59:18 +0000 Subject: cleanup leftover code --- configd/src/apps/sentinel/config-owner.cpp | 26 -------------------------- configd/src/apps/sentinel/config-owner.h | 6 ------ 2 files changed, 32 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/config-owner.cpp b/configd/src/apps/sentinel/config-owner.cpp index 4042eded24e..665cf07d3e7 100644 --- a/configd/src/apps/sentinel/config-owner.cpp +++ b/configd/src/apps/sentinel/config-owner.cpp @@ -17,15 +17,6 @@ ConfigOwner::~ConfigOwner() = default; void ConfigOwner::subscribe(const std::string & configId, std::chrono::milliseconds timeout) { _sentinelHandle = _subscriber.subscribe(configId, timeout); - try { - _modelHandle =_modelOwner.subscribe("admin/model", timeout); - } catch (ConfigTimeoutException & ex) { - LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (InvalidConfigException& ex) { - LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } catch (ConfigRuntimeException& ex) { - LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); - } } void @@ -38,7 +29,6 @@ ConfigOwner::doConfigure() const auto & app = config.application; LOG(config, "Sentinel got %zd service elements [tenant(%s), application(%s), instance(%s)] for config generation %" PRId64, config.service.size(), app.tenant.c_str(), app.name.c_str(), app.instance.c_str(), _currGeneration); - getModelConfig(); } @@ -52,20 +42,4 @@ ConfigOwner::checkForConfigUpdate() { return false; } -std::optional -ConfigOwner::getModelConfig() { - if (_modelHandle && _modelOwner.nextGenerationNow()) { - if (auto newModel = _modelHandle->getConfig()) { - LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", - newModel->vespaVersion.c_str(), newModel->hosts.size(), _modelOwner.getGeneration()); - _modelConfig = std::move(newModel); - } - } - if (_modelConfig) { - return ModelConfig(*_modelConfig); - } else { - return {}; - } -} - } diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h index b27c515e357..1a8a0a74666 100644 --- a/configd/src/apps/sentinel/config-owner.h +++ b/configd/src/apps/sentinel/config-owner.h @@ -8,7 +8,6 @@ #include using cloud::config::SentinelConfig; -using cloud::config::ModelConfig; using config::ConfigSubscriber; using config::ConfigHandle; @@ -26,10 +25,6 @@ private: int64_t _currGeneration = -1; std::unique_ptr _currConfig; - ConfigSubscriber _modelOwner; - ConfigHandle::UP _modelHandle; - std::unique_ptr _modelConfig; - ConfigOwner(const ConfigOwner&) = delete; ConfigOwner& operator =(const ConfigOwner&) = delete; @@ -42,7 +37,6 @@ public: bool hasConfig() const { return _currConfig.get() != nullptr; } const SentinelConfig& getConfig() const { return *_currConfig; } int64_t getGeneration() const { return _currGeneration; } - std::optional getModelConfig(); }; } -- cgit v1.2.3 From e667cf6c86959fccce07091d97950f43f418b5c9 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 14:02:20 +0000 Subject: cleanup some using statements --- configd/src/apps/sentinel/config-owner.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h index 1a8a0a74666..628f868a513 100644 --- a/configd/src/apps/sentinel/config-owner.h +++ b/configd/src/apps/sentinel/config-owner.h @@ -9,9 +9,6 @@ using cloud::config::SentinelConfig; -using config::ConfigSubscriber; -using config::ConfigHandle; - namespace config::sentinel { /** @@ -19,8 +16,8 @@ namespace config::sentinel { **/ class ConfigOwner { private: - ConfigSubscriber _subscriber; - ConfigHandle::UP _sentinelHandle; + config::ConfigSubscriber _subscriber; + config::ConfigHandle::UP _sentinelHandle; int64_t _currGeneration = -1; std::unique_ptr _currConfig; -- cgit v1.2.3 From db8530327e2b5348239925013366665c012381d9 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 14:34:51 +0000 Subject: cleanup and add lock --- configd/src/apps/sentinel/config-owner.h | 1 - configd/src/apps/sentinel/connectivity.cpp | 2 -- configd/src/apps/sentinel/env.cpp | 3 +-- configd/src/apps/sentinel/manager.cpp | 1 + configd/src/apps/sentinel/model-owner.cpp | 19 +++++++++++++------ configd/src/apps/sentinel/model-owner.h | 6 ++++-- configd/src/apps/sentinel/report-connectivity.cpp | 3 +-- configd/src/apps/sentinel/report-connectivity.h | 1 - 8 files changed, 20 insertions(+), 16 deletions(-) (limited to 'configd') diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h index 628f868a513..b72aed59271 100644 --- a/configd/src/apps/sentinel/config-owner.h +++ b/configd/src/apps/sentinel/config-owner.h @@ -5,7 +5,6 @@ #include #include #include -#include using cloud::config::SentinelConfig; diff --git a/configd/src/apps/sentinel/connectivity.cpp b/configd/src/apps/sentinel/connectivity.cpp index 7ea548ea73d..8314a090616 100644 --- a/configd/src/apps/sentinel/connectivity.cpp +++ b/configd/src/apps/sentinel/connectivity.cpp @@ -17,8 +17,6 @@ using namespace std::chrono_literals; namespace config::sentinel { -constexpr std::chrono::milliseconds MODEL_TIMEOUT_MS = 60s; - Connectivity::Connectivity() = default; Connectivity::~Connectivity() = default; diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp index 3fd9849023d..10f1b683cb5 100644 --- a/configd/src/apps/sentinel/env.cpp +++ b/configd/src/apps/sentinel/env.cpp @@ -53,7 +53,7 @@ Env::~Env() = default; void Env::boot(const std::string &configId) { LOG(debug, "Reading configuration for ID: %s", configId.c_str()); _cfgOwner.subscribe(configId, CONFIG_TIMEOUT_MS); - _modelOwner.start(CONFIG_TIMEOUT_MS); + _modelOwner.start(CONFIG_TIMEOUT_MS, true); // subscribe() should throw if something is not OK Connectivity checker; for (int retry = 0; retry < maxConnectivityRetries; ++retry) { @@ -121,7 +121,6 @@ void Env::statePort(int port) { void Env::notifyConfigUpdated() { vespalib::ComponentConfigProducer::Config current("sentinel", _cfgOwner.getGeneration(), "ok"); _stateApi.myComponents.addConfig(current); - _modelOwner.checkForUpdates(); } void Env::respondAsEmpty() { diff --git a/configd/src/apps/sentinel/manager.cpp b/configd/src/apps/sentinel/manager.cpp index 6e0ed78211c..0c13292d704 100644 --- a/configd/src/apps/sentinel/manager.cpp +++ b/configd/src/apps/sentinel/manager.cpp @@ -116,6 +116,7 @@ Manager::doWork() if (_env.configOwner().checkForConfigUpdate()) { doConfigure(); } + _env.modelOwner().checkForUpdates(); handleRestarts(); handleCommands(); handleOutputs(); diff --git a/configd/src/apps/sentinel/model-owner.cpp b/configd/src/apps/sentinel/model-owner.cpp index 601a57e16c9..cfa9f1f6bf5 100644 --- a/configd/src/apps/sentinel/model-owner.cpp +++ b/configd/src/apps/sentinel/model-owner.cpp @@ -14,7 +14,7 @@ using namespace std::chrono_literals; namespace config::sentinel { std::optional ModelOwner::getModelConfig() { - checkForUpdates(); + std::lock_guard guard(_lock); if (_modelConfig) { return ModelConfig(*_modelConfig); } else { @@ -30,27 +30,34 @@ ModelOwner::ModelOwner(const std::string &configId) ModelOwner::~ModelOwner() = default; void -ModelOwner::start(std::chrono::milliseconds timeout) { +ModelOwner::start(std::chrono::milliseconds timeout, bool firstTime) { try { _modelHandle =_subscriber.subscribe(_configId, timeout); } catch (ConfigTimeoutException & ex) { - LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + if (firstTime) { + LOG(warning, "Timeout getting model config: %s [skipping connectivity checks]", ex.message()); + } } catch (InvalidConfigException& ex) { - LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + if (firstTime) { + LOG(warning, "Invalid model config: %s [skipping connectivity checks]", ex.message()); + } } catch (ConfigRuntimeException& ex) { - LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.getMessage().c_str()); + if (firstTime) { + LOG(warning, "Runtime exception getting model config: %s [skipping connectivity checks]", ex.message()); + } } } void ModelOwner::checkForUpdates() { if (! _modelHandle) { - start(5ms); + start(250ms, false); } if (_modelHandle && _subscriber.nextGenerationNow()) { if (auto newModel = _modelHandle->getConfig()) { LOG(config, "Sentinel got model info [version %s] for %zd hosts [config generation %" PRId64 "]", newModel->vespaVersion.c_str(), newModel->hosts.size(), _subscriber.getGeneration()); + std::lock_guard guard(_lock); _modelConfig = std::move(newModel); } } diff --git a/configd/src/apps/sentinel/model-owner.h b/configd/src/apps/sentinel/model-owner.h index d03e7dec06d..0513463e955 100644 --- a/configd/src/apps/sentinel/model-owner.h +++ b/configd/src/apps/sentinel/model-owner.h @@ -5,6 +5,7 @@ #include #include #include +#include using cloud::config::ModelConfig; @@ -18,11 +19,12 @@ private: std::string _configId; config::ConfigSubscriber _subscriber; config::ConfigHandle::UP _modelHandle; + std::mutex _lock; std::unique_ptr _modelConfig; public: ModelOwner(const std::string &configId); - virtual ~ModelOwner(); - void start(std::chrono::milliseconds timeout); + ~ModelOwner(); + void start(std::chrono::milliseconds timeout, bool firstTime); void checkForUpdates(); std::optional getModelConfig(); }; diff --git a/configd/src/apps/sentinel/report-connectivity.cpp b/configd/src/apps/sentinel/report-connectivity.cpp index eceb2cf64aa..c1e519a4a9a 100644 --- a/configd/src/apps/sentinel/report-connectivity.cpp +++ b/configd/src/apps/sentinel/report-connectivity.cpp @@ -15,7 +15,6 @@ namespace config::sentinel { ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, int timeout_ms, FRT_Supervisor &orb, ModelOwner &modelOwner) : _parentRequest(req), - _orb(orb), _checks() { auto cfg = modelOwner.getModelConfig(); @@ -24,7 +23,7 @@ ReportConnectivity::ReportConnectivity(FRT_RPCRequest *req, int timeout_ms, FRT_ LOG(debug, "making connectivity report for %zd peers", map.size()); _remaining = map.size(); for (const auto & [ hostname, port ] : map) { - _checks.emplace_back(std::make_unique(*this, hostname, port, _orb, timeout_ms)); + _checks.emplace_back(std::make_unique(*this, hostname, port, orb, timeout_ms)); } } else { _parentRequest->SetError(FRTE_RPC_METHOD_FAILED, "failed getting model config"); diff --git a/configd/src/apps/sentinel/report-connectivity.h b/configd/src/apps/sentinel/report-connectivity.h index 057a374a387..1f243b73028 100644 --- a/configd/src/apps/sentinel/report-connectivity.h +++ b/configd/src/apps/sentinel/report-connectivity.h @@ -26,7 +26,6 @@ public: private: void finish() const; FRT_RPCRequest *_parentRequest; - FRT_Supervisor &_orb; std::vector> _checks; std::atomic _remaining; }; -- cgit v1.2.3 From e3a93a6bfd8b90ced59f458675685ca7a45c9ded Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 11 Jun 2021 16:01:48 +0000 Subject: must check for model config updates in boot() also --- configd/src/apps/sentinel/env.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'configd') diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp index 10f1b683cb5..f857f95a496 100644 --- a/configd/src/apps/sentinel/env.cpp +++ b/configd/src/apps/sentinel/env.cpp @@ -66,6 +66,7 @@ void Env::boot(const std::string &configId) { configId.c_str(), cfg.port.telnet, cfg.port.rpc); rpcPort(cfg.port.rpc); statePort(cfg.port.telnet); + _modelOwner.checkForUpdates(); auto model = _modelOwner.getModelConfig(); if (model.has_value()) { checker.configure(cfg.connectivity, model.value()); -- cgit v1.2.3