we must now be prepared for multiple notifyOk calls

* ManagedRpcServer now schedules its own health checks,
  so we must handle repeated OK notifications.

3 files changed, 11 insertions, 10 deletions

diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
index 6636e832b61..c4a4acd1ced 100644
--- a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp
@@ -108,9 +108,8 @@ ExchangeManager::lookupPartner(const std::string & name) const {
 void
 ExchangeManager::healthCheck()
 {
-    for (const auto & entry : _partners) {
-        RemoteSlobrok *partner = entry.second.get();
-        partner->healthCheck();
+    for (const auto & [ name, partner ] : _partners) {
+        partner->maybePushMine();
     }
     LOG(debug, "ExchangeManager::healthCheck for %ld partners", _partners.size());
 }
diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
index aa72e88d895..d50aee21a29 100644
--- a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
+++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp
@@ -62,8 +62,8 @@ RemoteSlobrok::~RemoteSlobrok()
 void
 RemoteSlobrok::doPending()
 {
-    LOG_ASSERT(_remAddReq == nullptr);
-    LOG_ASSERT(_remRemReq == nullptr);
+    if (_remAddReq != nullptr) return;
+    if (_remRemReq != nullptr) return;
 
     if (_remote == nullptr) return;
 
@@ -162,7 +162,7 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req)
         _remListReq = nullptr;
 
         // next step is to push the ones I own:
-        if (_remote != nullptr) pushMine();
+        maybePushMine();
     } else if (req == _remAddReq) {
         // handle response after pushing some name that we managed:
         if (req->IsError() && (req->GetErrorCode() == FRTE_RPC_CONNECTION ||
@@ -241,7 +241,7 @@ RemoteSlobrok::fail()
 
 
 void
-RemoteSlobrok::healthCheck()
+RemoteSlobrok::maybePushMine()
 {
     if (_remote != nullptr &&
         _remAddPeerReq == nullptr &&
@@ -267,9 +267,11 @@ RemoteSlobrok::notifyOkRpcSrv(ManagedRpcServer *rpcsrv)
     // connection was OK, so disable any pending reconnect
     _reconnecter.disable();
 
-    if (_remote == nullptr) {
-        _remote = getSupervisor()->GetTarget(getSpec().c_str());
+    if (_remote != nullptr) {
+        // the rest here should only be done on first notifyOk
+        return;
     }
+    _remote = getSupervisor()->GetTarget(getSpec().c_str());
 
     // at this point, we will do (in sequence):
     // ask peer to connect to us too;
diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.h b/slobrok/src/vespa/slobrok/server/remote_slobrok.h
index a5d17e0452b..fa627bbd821 100644
--- a/slobrok/src/vespa/slobrok/server/remote_slobrok.h
+++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.h
@@ -66,7 +66,7 @@ public:
     void fail();
     bool isConnected() const { return (_remote != nullptr); }
     void tryConnect();
-    void healthCheck();
+    void maybePushMine();
     void invokeAsync(FRT_RPCRequest *req, double timeout, FRT_IRequestWait *rwaiter);
     const std::string & getName() const { return _rpcserver.getName(); }
     const std::string & getSpec() const { return _rpcserver.getSpec(); }