cleanup and add lock

8 files changed, 20 insertions, 16 deletions

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 <vespa/config-sentinel.h>
 #include <vespa/config-model.h>
 #include <vespa/config/config.h>
-#include <optional>
 
 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<ModelConfig> ModelOwner::getModelConfig() {
-    checkForUpdates();
+    std::lock_guard<std::mutex> 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<ModelConfig>(_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<std::mutex> 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 <vespa/config-model.h>
 #include <vespa/config/config.h>
 #include <optional>
+#include <mutex>
 
 using cloud::config::ModelConfig;
 
@@ -18,11 +19,12 @@ private:
     std::string _configId;
     config::ConfigSubscriber _subscriber;
     config::ConfigHandle<ModelConfig>::UP _modelHandle;
+    std::mutex _lock;
     std::unique_ptr<ModelConfig> _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<ModelConfig> 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<PeerCheck>(*this, hostname, port, _orb, timeout_ms));
+            _checks.emplace_back(std::make_unique<PeerCheck>(*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<std::unique_ptr<PeerCheck>> _checks;
     std::atomic<size_t> _remaining;
 };