do diff of new/old worlds as part of health check

6 files changed, 64 insertions, 13 deletions

diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
index 87892c59f21..4d34d6ecb6b 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
@@ -4,6 +4,8 @@
 #include "rpc_server_map.h"
 #include "sbenv.h"
 #include <vespa/fnet/frt/supervisor.h>
+#include <vespa/vespalib/util/overload.h>
+#include <vespa/vespalib/util/visit_ranges.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".slobrok.server.exchange_manager");
@@ -106,9 +108,42 @@ ExchangeManager::lookupPartner(const std::string & name) const {
 void
 ExchangeManager::healthCheck()
 {
+    auto newWorldList = env().consensusMap().currentConsensus();
+    auto oldWorldServices = env().rpcServerMap().allManaged();
+    if (newWorldList.size() != oldWorldServices.size()) {
+        LOG(warning, "healthCheck compare fails: old world sz=%zu, new world sz=%zu",
+            oldWorldServices.size(), newWorldList.size());
+        ServiceMappingList oldWorldList;
+        for (const auto *nsp : oldWorldServices) {
+            oldWorldList.emplace_back(nsp->getName(), nsp->getSpec());
+        }
+        std::sort(oldWorldList.begin(), oldWorldList.end());
+        auto visitor = vespalib::overload
+        {
+            [&](vespalib::visit_ranges_first, const auto &m) {
+                LOG(warning, "new world only: %s->%s", m.name.c_str(), m.spec.c_str());
+            },
+            [&](vespalib::visit_ranges_second, const auto &m) {
+                LOG(warning, "old world only: %s->%s", m.name.c_str(), m.spec.c_str());
+            },
+            [&](vespalib::visit_ranges_both, const auto &, const auto &) {}
+        };
+        vespalib::visit_ranges(visitor,
+                               newWorldList.begin(), newWorldList.end(),
+                               oldWorldList.begin(), oldWorldList.end());
+    }
     for (const auto & [ name, partner ] : _partners) {
         partner->maybeStartFetch();
         partner->maybePushMine();
+        auto remoteList = partner->remoteMap().allMappings();
+        // 0 is expected (when remote is down)
+        if (remoteList.size() != 0 &&
+            remoteList.size() != newWorldList.size())
+        {
+            LOG(warning, "peer slobrok at %s has %zu mappings (expected %zu)",
+                name.c_str(), remoteList.size(), newWorldList.size());
+            // consider doing the visit_ranges diff here also
+        }
     }
     LOG(debug, "ExchangeManager::healthCheck for %ld partners", _partners.size());
 }
diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.h b/slobrok/src/vespa/slobrok/server/exchange_manager.h
index a7ff60d9ecc..6bb89fc0d8d 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.h
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.h
@@ -77,15 +77,19 @@ private:
         ~WorkPackage();
     };
 
+    SBEnv             &_env;
+    RpcServerManager  &_rpcsrvmanager;
+    RpcServerMap      &_rpcsrvmap;
+
 public:
     ExchangeManager(const ExchangeManager &) = delete;
     ExchangeManager &operator=(const ExchangeManager &) = delete;
     ExchangeManager(SBEnv &env, RpcServerMap &rpcsrvmap);
     ~ExchangeManager();
 
-    SBEnv             &_env;
-    RpcServerManager  &_rpcsrvmanager;
-    RpcServerMap      &_rpcsrvmap;
+    SBEnv             &env() { return _env; }
+    RpcServerManager  &rpcServerManager() { return _rpcsrvmanager; }
+    RpcServerMap      &rpcServerMap() { return _rpcsrvmap; }
 
     OkState addPartner(const std::string & name, const std::string & spec);
     void removePartner(const std::string & name);
diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
index dc568a83282..85fe9df00d3 100644
--- a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
+++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
@@ -18,13 +18,13 @@ namespace slobrok {
 RemoteSlobrok::RemoteSlobrok(const std::string &name, const std::string &spec,
                              ExchangeManager &manager)
     : _exchanger(manager),
-      _rpcsrvmanager(manager._rpcsrvmanager),
+      _rpcsrvmanager(manager.rpcServerManager()),
       _remote(nullptr),
       _serviceMapMirror(),
       _rpcserver(name, spec, *this),
       _reconnecter(getSupervisor()->GetScheduler(), *this),
       _failCnt(0),
-      _consensusSubscription(MapSubscription::subscribe(_serviceMapMirror, _exchanger._env.consensusMap())),
+      _consensusSubscription(MapSubscription::subscribe(_serviceMapMirror, _exchanger.env().consensusMap())),
       _remAddPeerReq(nullptr),
       _remListReq(nullptr),
       _remAddReq(nullptr),
@@ -80,19 +80,19 @@ RemoteSlobrok::doPending()
         std::unique_ptr<NamedService> todo = std::move(_pending.front());
         _pending.pop_front();
 
-        const NamedService *rpcsrv = _exchanger._rpcsrvmap.lookup(todo->getName());
+        const NamedService *rpcsrv = _exchanger.rpcServerMap().lookup(todo->getName());
 
         if (rpcsrv == nullptr) {
             _remRemReq = getSupervisor()->AllocRPCRequest();
             _remRemReq->SetMethodName("slobrok.internal.doRemove");
-            _remRemReq->GetParams()->AddString(_exchanger._env.mySpec().c_str());
+            _remRemReq->GetParams()->AddString(_exchanger.env().mySpec().c_str());
             _remRemReq->GetParams()->AddString(todo->getName().c_str());
             _remRemReq->GetParams()->AddString(todo->getSpec().c_str());
             _remote->InvokeAsync(_remRemReq, 2.0, this);
         } else {
             _remAddReq = getSupervisor()->AllocRPCRequest();
             _remAddReq->SetMethodName("slobrok.internal.doAdd");
-            _remAddReq->GetParams()->AddString(_exchanger._env.mySpec().c_str());
+            _remAddReq->GetParams()->AddString(_exchanger.env().mySpec().c_str());
             _remAddReq->GetParams()->AddString(todo->getName().c_str());
             _remAddReq->GetParams()->AddString(rpcsrv->getSpec().c_str());
             _remote->InvokeAsync(_remAddReq, 2.0, this);
@@ -174,7 +174,7 @@ void
 RemoteSlobrok::pushMine()
 {
     // all mine
-    std::vector<const NamedService *> mine = _exchanger._rpcsrvmap.allManaged();
+    std::vector<const NamedService *> mine = _exchanger.rpcServerMap().allManaged();
     while (mine.size() > 0) {
         const NamedService *now = mine.back();
         mine.pop_back();
@@ -365,8 +365,8 @@ RemoteSlobrok::notifyOkRpcSrv(ManagedRpcServer *rpcsrv)
 
     _remAddPeerReq = getSupervisor()->AllocRPCRequest();
     _remAddPeerReq->SetMethodName("slobrok.admin.addPeer");
-    _remAddPeerReq->GetParams()->AddString(_exchanger._env.mySpec().c_str());
-    _remAddPeerReq->GetParams()->AddString(_exchanger._env.mySpec().c_str());
+    _remAddPeerReq->GetParams()->AddString(_exchanger.env().mySpec().c_str());
+    _remAddPeerReq->GetParams()->AddString(_exchanger.env().mySpec().c_str());
     _remote->InvokeAsync(_remAddPeerReq, 3.0, this);
     // when _remAddPeerReq is returned, our managed list is added via doAdd()
 }
@@ -380,7 +380,7 @@ RemoteSlobrok::tryConnect()
 FRT_Supervisor *
 RemoteSlobrok::getSupervisor()
 {
-    return _exchanger._env.getSupervisor();
+    return _exchanger.env().getSupervisor();
 }
 
 //-----------------------------------------------------------------------------
diff --git a/slobrok/src/vespa/slobrok/server/service_mapping.h b/slobrok/src/vespa/slobrok/server/service_mapping.h
index 6bbf910ee3e..ce2e5eec347 100644
--- a/slobrok/src/vespa/slobrok/server/service_mapping.h
+++ b/slobrok/src/vespa/slobrok/server/service_mapping.h
@@ -17,7 +17,7 @@ struct ServiceMapping {
         return name == other.name && spec == other.spec;
     }
 
-    bool operator< (const ServiceMapping &other) const {
+    bool operator< (const ServiceMapping &other) const noexcept {
         if (name < other.name) return true;
         if (other.name < name) return false;
         return spec < other.spec;
diff --git a/slobrok/src/vespa/slobrok/server/union_service_map.cpp b/slobrok/src/vespa/slobrok/server/union_service_map.cpp
index d98f8b6aa18..baf94a6fa69 100644
--- a/slobrok/src/vespa/slobrok/server/union_service_map.cpp
+++ b/slobrok/src/vespa/slobrok/server/union_service_map.cpp
@@ -10,6 +10,16 @@ namespace slobrok {
 UnionServiceMap::UnionServiceMap() = default;
 UnionServiceMap::~UnionServiceMap() = default;
 
+ServiceMappingList UnionServiceMap::currentConsensus() const {
+    ServiceMappingList result;
+    for (const auto & [ name, list ] : _mappings) {
+        if (list.size() == 1u) {
+            result.emplace_back(name, list[0].spec);
+        }
+    }
+    return result;
+}
+
 void UnionServiceMap::add(const ServiceMapping &mapping)
 {
     const vespalib::string &key = mapping.name;
diff --git a/slobrok/src/vespa/slobrok/server/union_service_map.h b/slobrok/src/vespa/slobrok/server/union_service_map.h
index 0944cb43557..d5bcbfaed94 100644
--- a/slobrok/src/vespa/slobrok/server/union_service_map.h
+++ b/slobrok/src/vespa/slobrok/server/union_service_map.h
@@ -34,6 +34,8 @@ public:
     UnionServiceMap();
     virtual ~UnionServiceMap();
 
+    ServiceMappingList currentConsensus() const;
+
     void add(const ServiceMapping &mapping) override;
     void remove(const ServiceMapping &mapping) override;
     void update(const ServiceMapping &old_mapping,