From 1e2fcd12193ba3aae204e9aa401dc62b7570f621 Mon Sep 17 00:00:00 2001
From: Arne Juul <arnej@verizonmedia.com>
Date: Mon, 30 Aug 2021 07:01:53 +0000
Subject: diff with helper function

---
 .../src/vespa/slobrok/server/exchange_manager.cpp  | 62 ++++++++++++----------
 .../src/vespa/slobrok/server/exchange_manager.h    |  2 +
 2 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
index 4d34d6ecb6b..e5aa595cd7b 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
@@ -10,6 +10,8 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".slobrok.server.exchange_manager");
 
+using namespace vespalib;
+
 namespace slobrok {
 
 //-----------------------------------------------------------------------------
@@ -105,44 +107,50 @@ ExchangeManager::lookupPartner(const std::string & name) const {
     return (found == _partners.end()) ? nullptr : found->second.get();
 }
 
-void
-ExchangeManager::healthCheck()
+vespalib::string
+ExchangeManager::diffLists(const ServiceMappingList &lhs, const ServiceMappingList &rhs)
 {
-    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::string result;
+    auto visitor = overload
         {
-            [&](vespalib::visit_ranges_first, const auto &m) {
-                LOG(warning, "new world only: %s->%s", m.name.c_str(), m.spec.c_str());
+            [&result](visit_ranges_first, const auto &m) {
+                result.append("\nmissing: ").append(m.name).append("->").append(m.spec);
             },
-            [&](vespalib::visit_ranges_second, const auto &m) {
-                LOG(warning, "old world only: %s->%s", m.name.c_str(), m.spec.c_str());
+            [&result](visit_ranges_second, const auto &m) {
+                result.append("\nextra: ").append(m.name).append("->").append(m.spec);
             },
-            [&](vespalib::visit_ranges_both, const auto &, const auto &) {}
+            [](visit_ranges_both, const auto &, const auto &) {}
         };
-        vespalib::visit_ranges(visitor,
-                               newWorldList.begin(), newWorldList.end(),
-                               oldWorldList.begin(), oldWorldList.end());
+    visit_ranges(visitor, lhs.begin(), lhs.end(), rhs.begin(), rhs.end());
+    return result;
+}
+
+void
+ExchangeManager::healthCheck()
+{
+    auto oldWorldServices = env().rpcServerMap().allManaged();
+    ServiceMappingList oldWorldList;
+    for (const auto *nsp : oldWorldServices) {
+        oldWorldList.emplace_back(nsp->getName(), nsp->getSpec());
+    }
+    std::sort(oldWorldList.begin(), oldWorldList.end());
+    auto newWorldList = env().consensusMap().currentConsensus();
+    vespalib::string diff = diffLists(oldWorldList, newWorldList);
+    if (! diff.empty()) {
+        LOG(warning, "Diff from old world rpcServerMap to new world consensus map: %s",
+            diff.c_str());
     }
     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
+        if (remoteList.size() != 0) {
+            diff = diffLists(newWorldList, remoteList);
+            if (! diff.empty()) {
+                LOG(warning, "Diff from consensus map to peer slobrok mirror: %s",
+                    diff.c_str());
+            }
         }
     }
     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 6bb89fc0d8d..fe52155e57f 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.h
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.h
@@ -81,6 +81,8 @@ private:
     RpcServerManager  &_rpcsrvmanager;
     RpcServerMap      &_rpcsrvmap;
 
+    vespalib::string diffLists(const ServiceMappingList &lhs, const ServiceMappingList &rhs);
+
 public:
     ExchangeManager(const ExchangeManager &) = delete;
     ExchangeManager &operator=(const ExchangeManager &) = delete;
-- 
cgit v1.2.3