Merge pull request #19182 from vespa-engine/arnej/move-some-code

Arnej/move some code

11 files changed, 44 insertions, 84 deletions

diff --git a/slobrok/src/tests/startsome/startsome.sh b/slobrok/src/tests/startsome/startsome.sh
index 0375023bc24..a3b3158c368 100755
--- a/slobrok/src/tests/startsome/startsome.sh
+++ b/slobrok/src/tests/startsome/startsome.sh
@@ -33,7 +33,7 @@ sleep 1
 [ "${VALGRIND}" ] && sleep 9
 
 ./slobrok_rpc_info_app \
-	tcp/localhost:18481 verbose > rpc-method-list
+        tcp/localhost:18481 verbose > rpc-method-list
 
 echo port 18481:
 ${SBCMD} 18481 slobrok.callback.listNamesServed
diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
index ccfb8d3bd63..94e951ca252 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
@@ -25,9 +25,9 @@ ExchangeManager::ExchangeManager(SBEnv &env, RpcServerMap &rpcsrvmap)
 ExchangeManager::~ExchangeManager() = default;
 
 OkState
-ExchangeManager::addPartner(const std::string & name, const std::string & spec)
+ExchangeManager::addPartner(const std::string & spec)
 {
-    if (RemoteSlobrok *oldremote = lookupPartner(name)) {
+    if (RemoteSlobrok *oldremote = lookupPartner(spec)) {
         // already a partner, should be OK
         if (spec != oldremote->getSpec()) {
             return OkState(FRTE_RPC_METHOD_FAILED, "name already partner with different spec");
@@ -38,7 +38,7 @@ ExchangeManager::addPartner(const std::string & name, const std::string & spec)
         }
         return OkState();
     }
-    auto [ it, wasNew ] = _partners.emplace(name, std::make_unique<RemoteSlobrok>(name, spec, *this));
+    auto [ it, wasNew ] = _partners.emplace(spec, std::make_unique<RemoteSlobrok>(spec, spec, *this));
     LOG_ASSERT(wasNew);
     RemoteSlobrok & partner = *it->second;
     partner.tryConnect();
diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.h b/slobrok/src/vespa/slobrok/server/exchange_manager.h
index fe52155e57f..18a20274a75 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.h
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.h
@@ -93,8 +93,8 @@ public:
     RpcServerManager  &rpcServerManager() { return _rpcsrvmanager; }
     RpcServerMap      &rpcServerMap() { return _rpcsrvmap; }
 
-    OkState addPartner(const std::string & name, const std::string & spec);
-    void removePartner(const std::string & name);
+    OkState addPartner(const std::string & spec);
+    void removePartner(const std::string & spec);
     std::vector<std::string> getPartnerList();
 
     void forwardRemove(const std::string & name, const std::string & spec);
diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
index 85fe9df00d3..d867d955dac 100644
--- a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
+++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
@@ -335,7 +335,7 @@ RemoteSlobrok::maybePushMine()
         pushMine();
     } else {
         LOG(debug, "not pushing mine, as we have: remote %p r.a.p.r=%p r.l.r=%p r.a.r=%p r.r.r=%p",
-		_remote, _remAddPeerReq, _remListReq, _remAddReq, _remRemReq);
+                _remote, _remAddPeerReq, _remListReq, _remAddReq, _remRemReq);
     }
 }
 
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
index 0f0679e35ea..a9a748323f7 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
@@ -91,34 +91,6 @@ RpcServerManager::addRemReservation(const std::string & remslobrok, const std::s
 
 
 OkState
-RpcServerManager::addPeer(const std::string & remsbname, const std::string &remsbspec)
-{
-    if (remsbname == _env.mySpec()) {
-        return OkState(13, "cannot add remote slobrok with my rpcserver name");
-    }
-    return _exchanger.addPartner(remsbname, remsbspec);
-}
-
-
-OkState
-RpcServerManager::removePeer(const std::string & remsbname, const std::string & remsbspec)
-{
-    if (remsbname == _env.mySpec()) {
-        return OkState(13, "cannot remove my own rpcserver name");
-    }
-    const RemoteSlobrok *partner = _exchanger.lookupPartner(remsbname);
-    if (partner == nullptr) {
-        return OkState(0, "remote slobrok not a partner");
-    }
-    if (partner->getSpec() != remsbspec) {
-        return OkState(13, "peer registered with different spec");
-    }
-    _exchanger.removePartner(remsbname);
-    return OkState(0, "done");
-}
-
-
-OkState
 RpcServerManager::addMyReservation(const std::string & name, const std::string & spec)
 {
     OkState valid = validateName(name);
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
index 9c72fe91f18..15b674388e3 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
@@ -47,8 +47,6 @@ private:
 public:
     OkState checkPartner(const std::string & remslobrok);
 
-    OkState addPeer(const std::string & remsbname, const std::string & remsbspec);
-    OkState removePeer(const std::string & remsbname, const std::string &  remsbspec);
     OkState addRemote(const std::string & name, const std::string & spec);
 
     OkState addRemReservation(const std::string & remslobrok, const std::string & name, const std::string & spec);
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
index 4e1759f3a0d..fcaaf57570c 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
@@ -35,19 +35,6 @@ RpcServerMap::remove(const std::string & name)
 }
 
 std::vector<const NamedService *>
-RpcServerMap::lookupPattern(const char *pattern) const
-{
-    std::vector<const NamedService *> retval;
-    for (const auto & entry : _myrpcsrv_map) {
-        if (match(entry.first.c_str(), pattern)) {
-            retval.push_back(entry.second.get());
-        }
-    }
-    return retval;
-}
-
-
-std::vector<const NamedService *>
 RpcServerMap::allManaged() const
 {
     std::vector<const NamedService *> retval;
@@ -137,27 +124,6 @@ RpcServerMap::RpcServerMap() = default;
 
 RpcServerMap::~RpcServerMap() = default;
 
-bool
-RpcServerMap::match(const char *name, const char *pattern)
-{
-    LOG_ASSERT(name != nullptr);
-    LOG_ASSERT(pattern != nullptr);
-    while (*pattern != '\0') {
-        if (*name == *pattern) {
-            ++name;
-            ++pattern;
-        } else if (*pattern == '*') {
-            ++pattern;
-            while (*name != '/' && *name != '\0') {
-                ++name;
-            }
-        } else {
-            return false;
-        }
-    }
-    return (*name == *pattern);
-}
-
 void
 RpcServerMap::removeReservation(const std::string & name)
 {
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.h b/slobrok/src/vespa/slobrok/server/rpc_server_map.h
index 6af8fb79783..3d2999069ea 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_map.h
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.h
@@ -36,8 +36,6 @@ private:
     void add(NamedService *rpcsrv);
 
 public:
-    static bool match(const char *name, const char *pattern);
-
     typedef std::vector<const NamedService *> RpcSrvlist;
 
     MapSource &proxy() { return _proxy; }
@@ -45,7 +43,6 @@ public:
     ManagedRpcServer *lookupManaged(const std::string & name) const;
 
     const NamedService *    lookup(const std::string & name) const;
-    RpcSrvlist        lookupPattern(const char *pattern) const;
     RpcSrvlist        allManaged() const;
 
     void              addNew(std::unique_ptr<ManagedRpcServer> rpcsrv);
diff --git a/slobrok/src/vespa/slobrok/server/rpchooks.cpp b/slobrok/src/vespa/slobrok/server/rpchooks.cpp
index 39ef9d9a4eb..540060210ed 100644
--- a/slobrok/src/vespa/slobrok/server/rpchooks.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpchooks.cpp
@@ -65,6 +65,24 @@ void RPCHooks::reportMetrics() {
     EV_COUNT("other_reqs", _cnts.otherReqs);
 }
 
+bool RPCHooks::match(const char *name, const char *pattern) {
+    LOG_ASSERT(name != nullptr);
+    LOG_ASSERT(pattern != nullptr);
+    while (*pattern != '\0') {
+        if (*name == *pattern) {
+            ++name;
+            ++pattern;
+        } else if (*pattern == '*') {
+            ++pattern;
+            while (*name != '/' && *name != '\0') {
+                ++name;
+            }
+        } else {
+            return false;
+        }
+    }
+    return (*name == *pattern);
+}
 
 void RPCHooks::initRPC(FRT_Supervisor *supervisor) {
     _m_reporter = std::make_unique<MetricsReport>(supervisor, *this);
@@ -365,7 +383,7 @@ void RPCHooks::rpc_lookupRpcServer(FRT_RPCRequest *req) {
     auto diff = visible.makeDiffFrom(0);
     std::vector<ServiceMapping> matches;
     for (const auto & entry : diff.updated) {
-        if (RpcServerMap::match(entry.name.c_str(), rpcserverPattern)) {
+        if (match(entry.name.c_str(), rpcserverPattern)) {
             matches.push_back(entry);
         }
     }
diff --git a/slobrok/src/vespa/slobrok/server/rpchooks.h b/slobrok/src/vespa/slobrok/server/rpchooks.h
index e8f6c65ea47..b68eb9007a8 100644
--- a/slobrok/src/vespa/slobrok/server/rpchooks.h
+++ b/slobrok/src/vespa/slobrok/server/rpchooks.h
@@ -52,6 +52,8 @@ public:
     RPCHooks(SBEnv &env, RpcServerMap& rpcsrvmap, RpcServerManager& rpcsrvman);
     ~RPCHooks() override;
 
+    static bool match(const char *name, const char *pattern);
+
     void initRPC(FRT_Supervisor *supervisor);
     void reportMetrics();
     const Metrics& getMetrics() const { return _cnts; }
diff --git a/slobrok/src/vespa/slobrok/server/sbenv.cpp b/slobrok/src/vespa/slobrok/server/sbenv.cpp
index d1f60b85681..42debe1556c 100644
--- a/slobrok/src/vespa/slobrok/server/sbenv.cpp
+++ b/slobrok/src/vespa/slobrok/server/sbenv.cpp
@@ -214,7 +214,7 @@ SBEnv::setup(const std::vector<std::string> &cfg)
         std::string slobrok = cfg[i];
         discard(oldList, slobrok);
         if (slobrok != mySpec()) {
-            OkState res = _rpcsrvmanager.addPeer(slobrok, slobrok.c_str());
+            OkState res = _exchanger.addPartner(slobrok);
             if (!res.ok()) {
                 LOG(warning, "could not add peer %s: %s", slobrok.c_str(), res.errorMsg.c_str());
             } else {
@@ -223,12 +223,8 @@ SBEnv::setup(const std::vector<std::string> &cfg)
         }
     }
     for (uint32_t i = 0; i < oldList.size(); ++i) {
-        OkState res = _rpcsrvmanager.removePeer(oldList[i], oldList[i].c_str());
-        if (!res.ok()) {
-            LOG(warning, "could not remove peer %s: %s", oldList[i].c_str(), res.errorMsg.c_str());
-        } else {
-            LOG(config, "removed peer %s", oldList[i].c_str());
-        }
+        _exchanger.removePartner(oldList[i]);
+        LOG(config, "removed peer %s", oldList[i].c_str());
     }
     int64_t curGen = _configurator->getGeneration();
     vespalib::ComponentConfigProducer::Config current("slobroks", curGen, "ok");
@@ -238,6 +234,9 @@ SBEnv::setup(const std::vector<std::string> &cfg)
 OkState
 SBEnv::addPeer(const std::string &name, const std::string &spec)
 {
+    if (name != spec) {
+        return OkState(FRTE_RPC_METHOD_FAILED, "peer location brokers must have name equal to spec");
+    }
     if (spec == mySpec()) {
         return OkState(FRTE_RPC_METHOD_FAILED, "cannot add my own spec as peer");
     }
@@ -252,12 +251,15 @@ SBEnv::addPeer(const std::string &name, const std::string &spec)
                      spec.c_str(), peers.c_str());
         _partnerList.push_back(spec);
     }
-    return _rpcsrvmanager.addPeer(name, spec.c_str());
+    return _exchanger.addPartner(spec);
 }
 
 OkState
 SBEnv::removePeer(const std::string &name, const std::string &spec)
 {
+    if (name != spec) {
+        return OkState(FRTE_RPC_METHOD_FAILED, "peer location brokers must have name equal to spec");
+    }
     if (spec == mySpec()) {
         return OkState(FRTE_RPC_METHOD_FAILED, "cannot remove my own spec as peer");
     }
@@ -266,7 +268,12 @@ SBEnv::removePeer(const std::string &name, const std::string &spec)
             return OkState(FRTE_RPC_METHOD_FAILED, "configured partner list contains peer, cannot remove");
         }
     }
-    return _rpcsrvmanager.removePeer(name, spec.c_str());
+    const RemoteSlobrok *partner = _exchanger.lookupPartner(name);
+    if (partner == nullptr) {
+        return OkState(0, "remote slobrok not a partner");
+    }
+    _exchanger.removePartner(name);
+    return OkState(0, "done");
 }
 
 } // namespace slobrok