From 93028a1dab332fdcc6a74beac3446b1a40b28c06 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 27 Sep 2018 23:44:53 +0200 Subject: Modernise code and use smart pointers and favor std containers over homegrown specialized classes. --- slobrok/src/vespa/slobrok/server/cmd.cpp | 8 +- slobrok/src/vespa/slobrok/server/cmd.h | 9 +- .../src/vespa/slobrok/server/exchange_manager.cpp | 129 ++++++++------------- .../src/vespa/slobrok/server/exchange_manager.h | 61 ++++------ slobrok/src/vespa/slobrok/server/history.cpp | 2 +- slobrok/src/vespa/slobrok/server/history.h | 3 +- .../vespa/slobrok/server/managed_rpc_server.cpp | 30 ++--- .../src/vespa/slobrok/server/managed_rpc_server.h | 10 +- slobrok/src/vespa/slobrok/server/named_service.cpp | 3 +- slobrok/src/vespa/slobrok/server/named_service.h | 6 +- .../src/vespa/slobrok/server/remote_slobrok.cpp | 105 ++++++++--------- slobrok/src/vespa/slobrok/server/remote_slobrok.h | 13 +-- slobrok/src/vespa/slobrok/server/reserved_name.h | 2 +- .../vespa/slobrok/server/rpc_server_manager.cpp | 98 +++++++--------- .../src/vespa/slobrok/server/rpc_server_manager.h | 45 +++---- .../src/vespa/slobrok/server/rpc_server_map.cpp | 42 +++---- slobrok/src/vespa/slobrok/server/rpc_server_map.h | 12 +- slobrok/src/vespa/slobrok/server/rpchooks.cpp | 24 ++-- slobrok/src/vespa/slobrok/server/rpcmirror.cpp | 8 +- slobrok/src/vespa/slobrok/server/sbenv.cpp | 14 +-- slobrok/src/vespa/slobrok/server/sbenv.h | 2 +- slobrok/src/vespa/slobrok/server/selfcheck.cpp | 2 +- slobrok/src/vespa/slobrok/server/visible_map.cpp | 17 +-- slobrok/src/vespa/slobrok/server/visible_map.h | 5 +- 24 files changed, 273 insertions(+), 377 deletions(-) diff --git a/slobrok/src/vespa/slobrok/server/cmd.cpp b/slobrok/src/vespa/slobrok/server/cmd.cpp index 63a59eb0a8d..0e476c9bfe4 100644 --- a/slobrok/src/vespa/slobrok/server/cmd.cpp +++ b/slobrok/src/vespa/slobrok/server/cmd.cpp @@ -30,14 +30,14 @@ public: const std::string spec; FRT_RPCRequest * const registerRequest; - RegRpcSrvData(SBEnv &e, const char *n, const char *s, FRT_RPCRequest *r) + RegRpcSrvData(SBEnv &e, const std::string &n, const std::string &s, FRT_RPCRequest *r) : _state(RDC_INIT), env(e), name(n), spec(s), registerRequest(r) {} }; RegRpcSrvCommand RegRpcSrvCommand::makeRegRpcSrvCmd(SBEnv &env, - const char *name, const char *spec, + const std::string &name, const std::string &spec, FRT_RPCRequest *req) { RegRpcSrvData *data = new RegRpcSrvData(env, name, spec, req); @@ -45,7 +45,7 @@ RegRpcSrvCommand::makeRegRpcSrvCmd(SBEnv &env, } RegRpcSrvCommand -RegRpcSrvCommand::makeRemRemCmd(SBEnv &env, const char *name, const char *spec) +RegRpcSrvCommand::makeRemRemCmd(SBEnv &env, const std::string & name, const std::string &spec) { RegRpcSrvData *data = new RegRpcSrvData(env, name, spec, nullptr); data->_state = RegRpcSrvData::XCH_IGNORE; @@ -99,7 +99,7 @@ RegRpcSrvCommand::doneHandler(OkState result) LOG(spam, "phase addManaged(%s,%s)", _data->name.c_str(), _data->spec.c_str()); _data->_state = RegRpcSrvData::CHK_RPCSRV; - _data->env._rpcsrvmanager.addManaged(_data->name.c_str(), _data->spec.c_str(), *this); + _data->env._rpcsrvmanager.addManaged(_data->name, _data->spec.c_str(), *this); return; } else if (_data->_state == RegRpcSrvData::CHK_RPCSRV) { LOG(spam, "phase doAdd(%s,%s)", _data->name.c_str(), _data->spec.c_str()); diff --git a/slobrok/src/vespa/slobrok/server/cmd.h b/slobrok/src/vespa/slobrok/server/cmd.h index 744d874eed0..32c26967b01 100644 --- a/slobrok/src/vespa/slobrok/server/cmd.h +++ b/slobrok/src/vespa/slobrok/server/cmd.h @@ -40,13 +40,8 @@ public: return *this; } - static RegRpcSrvCommand makeRegRpcSrvCmd(SBEnv &env, - const char *name, - const char *spec, - FRT_RPCRequest *req); - static RegRpcSrvCommand makeRemRemCmd(SBEnv &env, - const char *name, - const char *spec); + static RegRpcSrvCommand makeRegRpcSrvCmd(SBEnv &env, const std::string &name, const std::string &spec, FRT_RPCRequest *req); + static RegRpcSrvCommand makeRemRemCmd(SBEnv &env, const std::string &name, const std::string &spec); }; //----------------------------------------------------------------------------- diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp index f3e06d83282..6e14eb073bb 100644 --- a/slobrok/src/vespa/slobrok/server/exchange_manager.cpp +++ b/slobrok/src/vespa/slobrok/server/exchange_manager.cpp @@ -13,23 +13,23 @@ namespace slobrok { //----------------------------------------------------------------------------- ExchangeManager::ExchangeManager(SBEnv &env, RpcServerMap &rpcsrvmap) - : _partners(NULL), + : _partners(), _env(env), _rpcsrvmanager(env._rpcsrvmanager), _rpcsrvmap(rpcsrvmap) { } +ExchangeManager::~ExchangeManager() = default; OkState -ExchangeManager::addPartner(const char *name, const char *spec) +ExchangeManager::addPartner(const std::string & name, const std::string & spec) { - RemoteSlobrok *oldremote = _partners[name]; - if (oldremote != NULL) { + RemoteSlobrok *oldremote = lookupPartner(name); + if (oldremote != nullptr) { // already a partner, should be OK - if (strcmp(spec, oldremote->getSpec()) != 0) { - return OkState(FRTE_RPC_METHOD_FAILED, - "name already partner with different spec"); + if (spec != oldremote->getSpec()) { + return OkState(FRTE_RPC_METHOD_FAILED, "name already partner with different spec"); } // this is probably a good time to try connecting again if (! oldremote->isConnected()) { @@ -38,100 +38,81 @@ ExchangeManager::addPartner(const char *name, const char *spec) return OkState(); } - RemoteSlobrok *partner = new RemoteSlobrok(name, spec, *this); - LOG_ASSERT(_partners.isSet(name) == false); - _partners.set(name, partner); - partner->tryConnect(); + LOG_ASSERT(_partners.find(name) == _partners.end()); + auto newPartner = std::make_unique(name, spec, *this); + RemoteSlobrok & partner = *newPartner; + _partners[name] = std::move(newPartner); + partner.tryConnect(); return OkState(); } - void -ExchangeManager::removePartner(const char *name) +ExchangeManager::removePartner(const std::string & name) { // assuming checks already done - RemoteSlobrok *oldremote = _partners.remove(name); - LOG_ASSERT(oldremote != NULL); - delete oldremote; + auto oldremote = std::move(_partners[name]); + LOG_ASSERT(oldremote); + _partners.erase(name); } - std::vector ExchangeManager::getPartnerList() { std::vector partnerList; - HashMap::Iterator itr = _partners.iterator(); - for (; itr.valid(); itr.next()) { - partnerList.push_back(std::string(itr.value()->getSpec())); + for (const auto & entry : _partners) { + partnerList.push_back(entry.second->getSpec()); } return partnerList; } void -ExchangeManager::forwardRemove(const char *name, const char *spec) +ExchangeManager::forwardRemove(const std::string & name, const std::string & spec) { - RegRpcSrvCommand remremhandler - = RegRpcSrvCommand::makeRemRemCmd(_env, name, spec); - WorkPackage *package = new WorkPackage(WorkPackage::OP_REMOVE, - name, spec, *this, - remremhandler); - HashMap::Iterator it = _partners.iterator(); - while (it.valid()) { - RemoteSlobrok *partner = it.value(); - package->addItem(partner); - it.next(); + RegRpcSrvCommand remremhandler = RegRpcSrvCommand::makeRemRemCmd(_env, name, spec); + WorkPackage *package = new WorkPackage(WorkPackage::OP_REMOVE, name, spec, *this, remremhandler); + for (const auto & entry : _partners) { + package->addItem(entry.second.get()); } package->expedite(); } void -ExchangeManager::doAdd(const char *name, const char *spec, - RegRpcSrvCommand rdc) +ExchangeManager::doAdd(const std::string &name, const std::string &spec, RegRpcSrvCommand rdc) { - HashMap::Iterator it = - _partners.iterator(); + WorkPackage *package = new WorkPackage(WorkPackage::OP_DOADD, name, spec, *this, rdc); - WorkPackage *package = - new WorkPackage(WorkPackage::OP_DOADD, name, spec, *this, rdc); - - while (it.valid()) { - RemoteSlobrok *partner = it.value(); - package->addItem(partner); - it.next(); + for (const auto & entry : _partners) { + package->addItem(entry.second.get()); } package->expedite(); } void -ExchangeManager::wantAdd(const char *name, const char *spec, - RegRpcSrvCommand rdc) +ExchangeManager::wantAdd(const std::string &name, const std::string &spec, RegRpcSrvCommand rdc) { - WorkPackage *package = - new WorkPackage(WorkPackage::OP_WANTADD, name, spec, *this, rdc); - HashMap::Iterator it = _partners.iterator(); - while (it.valid()) { - RemoteSlobrok *partner = it.value(); - package->addItem(partner); - it.next(); + WorkPackage *package = new WorkPackage(WorkPackage::OP_WANTADD, name, spec, *this, rdc); + for (const auto & entry : _partners) { + package->addItem(entry.second.get()); } package->expedite(); } +RemoteSlobrok * +ExchangeManager::lookupPartner(const std::string & name) const { + auto found = _partners.find(name); + return (found == _partners.end()) ? nullptr : found->second.get(); +} void ExchangeManager::healthCheck() { - int i=0; - HashMap::Iterator it = _partners.iterator(); - while (it.valid()) { - RemoteSlobrok *partner = it.value(); + for (const auto & entry : _partners) { + RemoteSlobrok *partner = entry.second.get(); partner->healthCheck(); - it.next(); - i++; } - LOG(debug, "ExchangeManager::healthCheck for %d partners", i); + LOG(debug, "ExchangeManager::healthCheck for %ld partners", _partners.size()); } //----------------------------------------------------------------------------- @@ -152,8 +133,7 @@ ExchangeManager::WorkPackage::WorkItem::RequestDone(FRT_RPCRequest *req) if (!req->IsError() && strcmp(answer.GetTypeString(), "is") == 0) { if (answer[0]._intval32 != 0) { - LOG(warning, "request denied: %s [%d]", - answer[1]._string._str, answer[0]._intval32); + LOG(warning, "request denied: %s [%d]", answer[1]._string._str, answer[0]._intval32); denied = true; } else { LOG(spam, "request approved"); @@ -163,7 +143,7 @@ ExchangeManager::WorkPackage::WorkItem::RequestDone(FRT_RPCRequest *req) // XXX tell remslob? } req->SubRef(); - _pendingReq = NULL; + _pendingReq = nullptr; _pkg.doneItem(denied); } @@ -175,17 +155,15 @@ ExchangeManager::WorkPackage::WorkItem::expedite() ExchangeManager::WorkPackage::WorkItem::~WorkItem() { - if (_pendingReq != NULL) { + if (_pendingReq != nullptr) { _pendingReq->Abort(); - LOG_ASSERT(_pendingReq == NULL); + LOG_ASSERT(_pendingReq == nullptr); } } -ExchangeManager::WorkPackage::WorkPackage(op_type op, - const char *name, const char *spec, - ExchangeManager &exchanger, - RegRpcSrvCommand donehandler) +ExchangeManager::WorkPackage::WorkPackage(op_type op, const std::string & name, const std::string & spec, + ExchangeManager &exchanger, RegRpcSrvCommand donehandler) : _work(), _doneCnt(0), _numDenied(0), @@ -197,13 +175,7 @@ ExchangeManager::WorkPackage::WorkPackage(op_type op, { } -ExchangeManager::WorkPackage::~WorkPackage() -{ - for (size_t i = 0; i < _work.size(); i++) { - delete _work[i]; - _work[i] = NULL; - } -} +ExchangeManager::WorkPackage::~WorkPackage() = default; void ExchangeManager::WorkPackage::doneItem(bool denied) @@ -240,15 +212,14 @@ ExchangeManager::WorkPackage::addItem(RemoteSlobrok *partner) } else if (_optype == OP_DOADD) { r->SetMethodName("slobrok.internal.doAdd"); } - r->GetParams()->AddString(_exchanger._env.mySpec()); + r->GetParams()->AddString(_exchanger._env.mySpec().c_str()); r->GetParams()->AddString(_name.c_str()); r->GetParams()->AddString(_spec.c_str()); - WorkItem *item = new WorkItem(*this, partner, r); - _work.push_back(item); + _work.push_back(std::make_unique(*this, partner, r)); LOG(spam, "added %s(%s,%s,%s) for %s to workpackage", - r->GetMethodName(), _exchanger._env.mySpec(), - _name.c_str(), _spec.c_str(), partner->getName()); + r->GetMethodName(), _exchanger._env.mySpec().c_str(), + _name.c_str(), _spec.c_str(), partner->getName().c_str()); } diff --git a/slobrok/src/vespa/slobrok/server/exchange_manager.h b/slobrok/src/vespa/slobrok/server/exchange_manager.h index 4d7bf4cff15..ea335cbb3c0 100644 --- a/slobrok/src/vespa/slobrok/server/exchange_manager.h +++ b/slobrok/src/vespa/slobrok/server/exchange_manager.h @@ -1,14 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include -#include - -#include #include "ok_state.h" #include "cmd.h" #include "remote_slobrok.h" +#include +#include +#include + namespace slobrok { //----------------------------------------------------------------------------- @@ -17,8 +17,6 @@ class SBEnv; class RpcServerMap; class RpcServerManager; -using vespalib::HashMap; - //----------------------------------------------------------------------------- /** @@ -31,10 +29,8 @@ using vespalib::HashMap; class ExchangeManager { private: - ExchangeManager(const ExchangeManager &); // Not used - ExchangeManager &operator=(const ExchangeManager &); // Not used - - HashMap _partners; + using PartnerMap = std::unordered_map>; + PartnerMap _partners; class WorkPackage; @@ -48,27 +44,21 @@ private: class WorkPackage { private: - WorkPackage(const WorkPackage&); // not used - WorkPackage& operator= (const WorkPackage&); // not used - class WorkItem: public FRT_IRequestWait { private: WorkPackage &_pkg; FRT_RPCRequest *_pendingReq; RemoteSlobrok *_remslob; - - WorkItem(const WorkItem&); // not used - WorkItem& operator= (const WorkItem&); // not used public: void expedite(); void RequestDone(FRT_RPCRequest *req) override; - WorkItem(WorkPackage &pkg, - RemoteSlobrok *rem, - FRT_RPCRequest *req); + WorkItem(WorkPackage &pkg, RemoteSlobrok *rem, FRT_RPCRequest *req); + WorkItem(const WorkItem&) = delete; + WorkItem& operator= (const WorkItem&) = delete; ~WorkItem(); }; - std::vector _work; + std::vector> _work; size_t _doneCnt; size_t _numDenied; RegRpcSrvCommand _donehandler; @@ -81,41 +71,36 @@ private: void addItem(RemoteSlobrok *partner); void doneItem(bool denied); void expedite(); - WorkPackage(op_type op, - const char *name, const char *spec, - ExchangeManager &exchanger, - RegRpcSrvCommand donehandler); + WorkPackage(const WorkPackage&) = delete; + WorkPackage& operator= (const WorkPackage&) = delete; + WorkPackage(op_type op, const std::string & name, const std::string & spec, + ExchangeManager &exchanger, RegRpcSrvCommand donehandler); ~WorkPackage(); }; public: + ExchangeManager(const ExchangeManager &) = delete; + ExchangeManager &operator=(const ExchangeManager &) = delete; ExchangeManager(SBEnv &env, RpcServerMap &rpcsrvmap); - ~ExchangeManager() {} + ~ExchangeManager(); SBEnv &_env; RpcServerManager &_rpcsrvmanager; RpcServerMap &_rpcsrvmap; - OkState addPartner(const char *name, const char *spec); - void removePartner(const char *name); + OkState addPartner(const std::string & name, const std::string & spec); + void removePartner(const std::string & name); std::vector getPartnerList(); - void registerFrom(RemoteSlobrok *partner); - void reregisterTo(RemoteSlobrok *partner); + void forwardRemove(const std::string & name, const std::string & spec); - void forwardRemove(const char *name, const char *spec); - - void wantAdd(const char *name, const char *spec, RegRpcSrvCommand rdc); - void doAdd(const char *name, const char *spec, RegRpcSrvCommand rdc); - - RemoteSlobrok *lookupPartner(const char *name) const { - return _partners[name]; - } + void wantAdd(const std::string & name, const std::string & spec, RegRpcSrvCommand rdc); + void doAdd(const std::string & name, const std::string & spec, RegRpcSrvCommand rdc); + RemoteSlobrok *lookupPartner(const std::string & name) const; void healthCheck(); }; //----------------------------------------------------------------------------- } // namespace slobrok - diff --git a/slobrok/src/vespa/slobrok/server/history.cpp b/slobrok/src/vespa/slobrok/server/history.cpp index 9b4fcedba8b..f23b0cbb757 100644 --- a/slobrok/src/vespa/slobrok/server/history.cpp +++ b/slobrok/src/vespa/slobrok/server/history.cpp @@ -22,7 +22,7 @@ History::verify() const } void -History::add(const char *name, vespalib::GenCnt gen) +History::add(const std::string &name, vespalib::GenCnt gen) { HistoryEntry h; _entries.push_back(h); diff --git a/slobrok/src/vespa/slobrok/server/history.h b/slobrok/src/vespa/slobrok/server/history.h index 2790d467b1a..ca90c927dc0 100644 --- a/slobrok/src/vespa/slobrok/server/history.h +++ b/slobrok/src/vespa/slobrok/server/history.h @@ -18,12 +18,11 @@ private: std::vector _entries; - typedef std::vector::iterator iter_t; typedef std::vector::const_iterator citer_t; void verify() const; public: - void add(const char *name, vespalib::GenCnt gen); + void add(const std::string &name, vespalib::GenCnt gen); bool has(vespalib::GenCnt gen) const; diff --git a/slobrok/src/vespa/slobrok/server/managed_rpc_server.cpp b/slobrok/src/vespa/slobrok/server/managed_rpc_server.cpp index 925df0e9f57..065dac04304 100644 --- a/slobrok/src/vespa/slobrok/server/managed_rpc_server.cpp +++ b/slobrok/src/vespa/slobrok/server/managed_rpc_server.cpp @@ -12,14 +12,14 @@ namespace slobrok { //----------------------------------------------------------------------------- -ManagedRpcServer::ManagedRpcServer(const char *name, - const char *spec, +ManagedRpcServer::ManagedRpcServer(const std::string & name, + const std::string & spec, IRpcServerManager &manager) : NamedService(name, spec), _mmanager(manager), _monitor(*this, *manager.getSupervisor()), - _monitoredServer(NULL), - _checkServerReq(NULL) + _monitoredServer(nullptr), + _checkServerReq(nullptr) { } @@ -27,10 +27,10 @@ ManagedRpcServer::ManagedRpcServer(const char *name, void ManagedRpcServer::healthCheck() { - if (_monitoredServer == NULL) { + if (_monitoredServer == nullptr) { _monitoredServer = _mmanager.getSupervisor()->GetTarget(_spec.c_str()); } - if (_checkServerReq == NULL) { + if (_checkServerReq == nullptr) { _checkServerReq = _mmanager.getSupervisor()->AllocRPCRequest(); _checkServerReq->SetMethodName("slobrok.callback.listNamesServed"); _monitoredServer->InvokeAsync(_checkServerReq, 25.0, this); @@ -49,14 +49,14 @@ void ManagedRpcServer::cleanupMonitor() { _monitor.disable(); - if (_monitoredServer != NULL) { + if (_monitoredServer != nullptr) { _monitoredServer->SubRef(); - _monitoredServer = NULL; + _monitoredServer = nullptr; } - if (_checkServerReq != NULL) { + if (_checkServerReq != nullptr) { _checkServerReq->Abort(); // _checkServerReq cleared by RequestDone Method - LOG_ASSERT(_checkServerReq == NULL); + LOG_ASSERT(_checkServerReq == nullptr); } } @@ -90,9 +90,9 @@ ManagedRpcServer::RequestDone(FRT_RPCRequest *req) FRT_Values &answer = *(req->GetReturn()); if (req->GetErrorCode() == FRTE_RPC_ABORT) { - LOG(debug, "rpcserver[%s].check aborted", getName()); + LOG(debug, "rpcserver[%s].check aborted", getName().c_str()); req->SubRef(); - _checkServerReq = NULL; + _checkServerReq = nullptr; return; } @@ -111,18 +111,18 @@ ManagedRpcServer::RequestDone(FRT_RPCRequest *req) errmsg = "checkServer failed validation"; } req->SubRef(); - _checkServerReq = NULL; + _checkServerReq = nullptr; cleanupMonitor(); _mmanager.notifyFailedRpcSrv(this, errmsg); return; } // start monitoring connection to server - LOG_ASSERT(_monitoredServer != NULL); + LOG_ASSERT(_monitoredServer != nullptr); _monitor.enable(_monitoredServer); req->SubRef(); - _checkServerReq = NULL; + _checkServerReq = nullptr; _mmanager.notifyOkRpcSrv(this); } diff --git a/slobrok/src/vespa/slobrok/server/managed_rpc_server.h b/slobrok/src/vespa/slobrok/server/managed_rpc_server.h index e8bb8b6e2d8..072aebd7850 100644 --- a/slobrok/src/vespa/slobrok/server/managed_rpc_server.h +++ b/slobrok/src/vespa/slobrok/server/managed_rpc_server.h @@ -25,14 +25,10 @@ class ManagedRpcServer: public NamedService, public FRT_IRequestWait, public IMonitoredServer { -private: - ManagedRpcServer(const ManagedRpcServer&); // Not used - ManagedRpcServer& operator=(const ManagedRpcServer&); // Not used - public: - ManagedRpcServer(const char *name, - const char *spec, - IRpcServerManager &manager); + ManagedRpcServer(const ManagedRpcServer&) = delete; + ManagedRpcServer& operator=(const ManagedRpcServer&) = delete; + ManagedRpcServer(const std::string & name, const std::string & spec, IRpcServerManager &manager); ~ManagedRpcServer(); void healthCheck(); diff --git a/slobrok/src/vespa/slobrok/server/named_service.cpp b/slobrok/src/vespa/slobrok/server/named_service.cpp index 59c2c31dbdc..c1aeb83a8a5 100644 --- a/slobrok/src/vespa/slobrok/server/named_service.cpp +++ b/slobrok/src/vespa/slobrok/server/named_service.cpp @@ -9,8 +9,7 @@ namespace slobrok { //----------------------------------------------------------------------------- -NamedService::NamedService(const char *name, - const char *spec) +NamedService::NamedService(const std::string & name, const std::string & spec) : _name(name), _spec(spec) { diff --git a/slobrok/src/vespa/slobrok/server/named_service.h b/slobrok/src/vespa/slobrok/server/named_service.h index d08b5079ef2..712e340400c 100644 --- a/slobrok/src/vespa/slobrok/server/named_service.h +++ b/slobrok/src/vespa/slobrok/server/named_service.h @@ -25,11 +25,11 @@ public: NamedService(const NamedService &) = delete; NamedService &operator=(const NamedService &) = delete; - NamedService(const char *name, const char *spec); + NamedService(const std::string & name, const std::string & spec); virtual ~NamedService(); - const char *getName() const { return _name.c_str(); } - const char *getSpec() const { return _spec.c_str(); } + const std::string & getName() const { return _name; } + const std::string & getSpec() const { return _spec; } }; //----------------------------------------------------------------------------- diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp index bc33fc900ec..345a96ec9f1 100644 --- a/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp +++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.cpp @@ -25,18 +25,18 @@ class IgnoreReqDone: public FRT_IRequestWait //----------------------------------------------------------------------------- -RemoteSlobrok::RemoteSlobrok(const char *name, const char *spec, +RemoteSlobrok::RemoteSlobrok(const std::string &name, const std::string &spec, ExchangeManager &manager) : _exchanger(manager), _rpcsrvmanager(manager._rpcsrvmanager), - _remote(NULL), + _remote(nullptr), _rpcserver(name, spec, *this), _reconnecter(getSupervisor()->GetScheduler(), *this), _failCnt(0), - _remAddPeerReq(NULL), - _remListReq(NULL), - _remAddReq(NULL), - _remRemReq(NULL), + _remAddPeerReq(nullptr), + _remListReq(nullptr), + _remAddReq(nullptr), + _remRemReq(nullptr), _pending() { _rpcserver.healthCheck(); @@ -48,21 +48,21 @@ RemoteSlobrok::~RemoteSlobrok() { _reconnecter.disable(); - if (_remote != NULL) { + if (_remote != nullptr) { _remote->SubRef(); - _remote = NULL; + _remote = nullptr; } - if (_remAddPeerReq != NULL) { + if (_remAddPeerReq != nullptr) { _remAddPeerReq->Abort(); } - if (_remListReq != NULL) { + if (_remListReq != nullptr) { _remListReq->Abort(); } - if (_remAddReq != NULL) { + if (_remAddReq != nullptr) { _remAddReq->Abort(); } - if (_remRemReq != NULL) { + if (_remRemReq != nullptr) { _remRemReq->Abort(); } // _rpcserver destructor called automatically @@ -72,8 +72,8 @@ RemoteSlobrok::~RemoteSlobrok() void RemoteSlobrok::doPending() { - LOG_ASSERT(_remAddReq == NULL); - LOG_ASSERT(_remRemReq == NULL); + LOG_ASSERT(_remAddReq == nullptr); + LOG_ASSERT(_remRemReq == nullptr); if (_pending.size() > 0) { NamedService *todo = _pending.front(); @@ -81,19 +81,19 @@ RemoteSlobrok::doPending() NamedService *rpcsrv = _exchanger._rpcsrvmap.lookup(todo->getName()); - if (rpcsrv == NULL) { + if (rpcsrv == nullptr) { _remRemReq = getSupervisor()->AllocRPCRequest(); _remRemReq->SetMethodName("slobrok.internal.doRemove"); - _remRemReq->GetParams()->AddString(_exchanger._env.mySpec()); - _remRemReq->GetParams()->AddString(todo->getName()); - _remRemReq->GetParams()->AddString(todo->getSpec()); + _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()); - _remAddReq->GetParams()->AddString(todo->getName()); - _remAddReq->GetParams()->AddString(rpcsrv->getSpec()); + _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); } // XXX should save this and pick up on RequestDone() @@ -127,19 +127,18 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) const char *myname = args[0]._string._str; const char *myspec = args[1]._string._str; LOG(error, "addPeer(%s, %s) on remote slobrok %s at %s: %s", - myname, myspec, getName(), getSpec(), - req->GetErrorMessage()); + myname, myspec, getName().c_str(), getSpec().c_str(), req->GetErrorMessage()); req->SubRef(); - _remAddPeerReq = NULL; + _remAddPeerReq = nullptr; goto retrylater; } req->SubRef(); - _remAddPeerReq = NULL; + _remAddPeerReq = nullptr; // next step is to ask the remote to send its list of managed names: - LOG_ASSERT(_remListReq == NULL); + LOG_ASSERT(_remListReq == nullptr); _remListReq = getSupervisor()->AllocRPCRequest(); _remListReq->SetMethodName("slobrok.internal.listManagedRpcServers"); - if (_remote != NULL) { + if (_remote != nullptr) { _remote->InvokeAsync(_remListReq, 3.0, this); } // when _remListReq is returned, our managed list is added @@ -149,19 +148,18 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) || strcmp(answer.GetTypeString(), "SS") != 0) { LOG(error, "error listing remote slobrok %s at %s: %s", - getName(), getSpec(), req->GetErrorMessage()); + getName().c_str(), getSpec().c_str(), req->GetErrorMessage()); req->SubRef(); - _remListReq = NULL; + _remListReq = nullptr; goto retrylater; } uint32_t numNames = answer.GetValue(0)._string_array._len; uint32_t numSpecs = answer.GetValue(1)._string_array._len; if (numNames != numSpecs) { - LOG(error, "inconsistent array lengths from %s at %s", - getName(), getSpec()); + LOG(error, "inconsistent array lengths from %s at %s", getName().c_str(), getSpec().c_str()); req->SubRef(); - _remListReq = NULL; + _remListReq = nullptr; goto retrylater; } FRT_StringValue *names = answer.GetValue(0)._string_array._pt; @@ -171,7 +169,7 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) _rpcsrvmanager.addRemote(names[idx]._str, specs[idx]._str); } req->SubRef(); - _remListReq = NULL; + _remListReq = nullptr; // next step is to push the ones I own: pushMine(); @@ -180,10 +178,9 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) if (req->IsError() && (req->GetErrorCode() == FRTE_RPC_CONNECTION || req->GetErrorCode() == FRTE_RPC_TIMEOUT)) { - LOG(error, "connection error adding to remote slobrok: %s", - req->GetErrorMessage()); + LOG(error, "connection error adding to remote slobrok: %s", req->GetErrorMessage()); req->SubRef(); - _remAddReq = NULL; + _remAddReq = nullptr; goto retrylater; } if (req->IsError()) { @@ -195,7 +192,7 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) _rpcsrvmanager.removeLocal(rpcsrvname, rpcsrvspec); } req->SubRef(); - _remAddReq = NULL; + _remAddReq = nullptr; doPending(); } else if (req == _remRemReq) { // handle response after pushing some remove we had pending: @@ -205,7 +202,7 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) LOG(error, "connection error adding to remote slobrok: %s", req->GetErrorMessage()); req->SubRef(); - _remRemReq = NULL; + _remRemReq = nullptr; goto retrylater; } if (req->IsError()) { @@ -213,11 +210,11 @@ RemoteSlobrok::RequestDone(FRT_RPCRequest *req) req->GetErrorMessage()); } req->SubRef(); - _remRemReq = NULL; + _remRemReq = nullptr; doPending(); } else { LOG(error, "got unknown request back in RequestDone()"); - LOG_ASSERT(req == NULL); + LOG_ASSERT(req == nullptr); } return; @@ -232,10 +229,10 @@ RemoteSlobrok::notifyFailedRpcSrv(ManagedRpcServer *rpcsrv, std::string errmsg) { if (++_failCnt > 10) { LOG(warning, "remote location broker at %s failed: %s", - rpcsrv->getSpec(), errmsg.c_str()); + rpcsrv->getSpec().c_str(), errmsg.c_str()); } else { LOG(debug, "remote location broker at %s failed: %s", - rpcsrv->getSpec(), errmsg.c_str()); + rpcsrv->getSpec().c_str(), errmsg.c_str()); } LOG_ASSERT(rpcsrv == &_rpcserver); fail(); @@ -246,9 +243,9 @@ void RemoteSlobrok::fail() { // disconnect - if (_remote != NULL) { + if (_remote != nullptr) { _remote->SubRef(); - _remote = NULL; + _remote = nullptr; } // schedule reconnect attempt _reconnecter.scheduleTryConnect(); @@ -258,13 +255,13 @@ RemoteSlobrok::fail() void RemoteSlobrok::healthCheck() { - if (_remote != NULL && - _remAddPeerReq == NULL && - _remListReq == NULL && - _remAddReq == NULL && - _remRemReq == NULL) + if (_remote != nullptr && + _remAddPeerReq == nullptr && + _remListReq == nullptr && + _remAddReq == nullptr && + _remRemReq == nullptr) { - LOG(debug, "spamming remote at %s with my names", getName()); + LOG(debug, "spamming remote at %s with my names", getName().c_str()); 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", @@ -282,8 +279,8 @@ RemoteSlobrok::notifyOkRpcSrv(ManagedRpcServer *rpcsrv) // connection was OK, so disable any pending reconnect _reconnecter.disable(); - if (_remote == NULL) { - _remote = getSupervisor()->GetTarget(getSpec()); + if (_remote == nullptr) { + _remote = getSupervisor()->GetTarget(getSpec().c_str()); } // at this point, we will do (in sequence): @@ -294,8 +291,8 @@ RemoteSlobrok::notifyOkRpcSrv(ManagedRpcServer *rpcsrv) _remAddPeerReq = getSupervisor()->AllocRPCRequest(); _remAddPeerReq->SetMethodName("slobrok.admin.addPeer"); - _remAddPeerReq->GetParams()->AddString(_exchanger._env.mySpec()); - _remAddPeerReq->GetParams()->AddString(_exchanger._env.mySpec()); + _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() } diff --git a/slobrok/src/vespa/slobrok/server/remote_slobrok.h b/slobrok/src/vespa/slobrok/server/remote_slobrok.h index 1851e57c93c..2740f409f3c 100644 --- a/slobrok/src/vespa/slobrok/server/remote_slobrok.h +++ b/slobrok/src/vespa/slobrok/server/remote_slobrok.h @@ -62,19 +62,16 @@ private: void doPending(); public: - RemoteSlobrok(const char *name, const char *spec, - ExchangeManager &manager); + RemoteSlobrok(const std::string &name, const std::string &spec, ExchangeManager &manager); ~RemoteSlobrok(); void fail(); - bool isConnected() const { return (_remote != NULL); } + bool isConnected() const { return (_remote != nullptr); } void tryConnect(); void healthCheck(); - void invokeAsync(FRT_RPCRequest *req, - double timeout, - FRT_IRequestWait *rwaiter); - const char *getName() const { return _rpcserver.getName(); } - const char *getSpec() const { return _rpcserver.getSpec(); } + 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(); } // interfaces implemented: void notifyFailedRpcSrv(ManagedRpcServer *rpcsrv, std::string errmsg) override; diff --git a/slobrok/src/vespa/slobrok/server/reserved_name.h b/slobrok/src/vespa/slobrok/server/reserved_name.h index ee533626f72..1ebed4b94dc 100644 --- a/slobrok/src/vespa/slobrok/server/reserved_name.h +++ b/slobrok/src/vespa/slobrok/server/reserved_name.h @@ -22,7 +22,7 @@ private: public: const bool isLocal; - ReservedName(const char *name, const char *spec, bool local) + ReservedName(const std::string &name, const std::string &spec, bool local) : NamedService(name, spec), _reservedTime(), isLocal(local) { _reservedTime.SetNow(); diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp index 97749de645c..e18e13d8d59 100644 --- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp +++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp @@ -24,15 +24,15 @@ RpcServerManager::RpcServerManager(SBEnv &sbenv) } static OkState -validateName(const char *rpcsrvname) +validateName(const std::string & rpcsrvname) { - const char *p = rpcsrvname; + const char *p = rpcsrvname.c_str(); while (*p != '\0') { // important: disallow '*' if (strchr("+,-./:=@[]_{}~<>" "0123456789" "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz", *p) == NULL) + "abcdefghijklmnopqrstuvwxyz", *p) == nullptr) { std::ostringstream tmp; tmp << "Illegal character '" << *p << "' ("; @@ -49,22 +49,20 @@ validateName(const char *rpcsrvname) OkState -RpcServerManager::checkPartner(const char *remslobrok) +RpcServerManager::checkPartner(const std::string & remslobrok) { - if (strcmp(remslobrok, _env.mySpec()) == 0) { + if (remslobrok == _env.mySpec()) { return OkState(13, "remote slobrok using my rpcserver name"); } RemoteSlobrok *partner = _exchanger.lookupPartner(remslobrok); - if (partner == NULL) { + if (partner == nullptr) { return OkState(13, "remote slobrok not a partner"); } return OkState(); } OkState -RpcServerManager::addRemReservation(const char *remslobrok, - const char *name, - const char *spec) +RpcServerManager::addRemReservation(const std::string & remslobrok, const std::string & name, const std::string &spec) { OkState state = checkPartner(remslobrok); if (state.failed()) return state; @@ -73,30 +71,27 @@ RpcServerManager::addRemReservation(const char *remslobrok, if (valid.failed()) return valid; NamedService *old = _rpcsrvmap.lookupManaged(name); - if (old != NULL) { - if (strcmp(old->getSpec(), spec) == 0) { + if (old != nullptr) { + if (old->getSpec() == spec) { // was alright already return OkState(0, "already registered"); } LOG(warning, "remote %s tried to register [%s -> %s] but we already have [%s -> %s] registered!", - remslobrok, name, spec, old->getName(), old->getSpec()); + remslobrok.c_str(), name.c_str(), spec.c_str(), old->getName().c_str(), old->getSpec().c_str()); return OkState(FRTE_RPC_METHOD_FAILED, "already managed by me"); } if (_rpcsrvmap.conflictingReservation(name, spec)) { - return OkState(FRTE_RPC_METHOD_FAILED, - "registration for name already in progress"); + return OkState(FRTE_RPC_METHOD_FAILED, "registration for name already in progress"); } - ReservedName *rpcsrv = new ReservedName(name, spec, false); - _rpcsrvmap.addReservation(rpcsrv); + _rpcsrvmap.addReservation(new ReservedName(name, spec, false)); return OkState(0, "done"); } OkState -RpcServerManager::addPeer(const char *remsbname, - const char *remsbspec) +RpcServerManager::addPeer(const std::string & remsbname, const std::string &remsbspec) { - if (strcmp(remsbname, _env.mySpec()) == 0) { + if (remsbname == _env.mySpec()) { return OkState(13, "cannot add remote slobrok with my rpcserver name"); } return _exchanger.addPartner(remsbname, remsbspec); @@ -104,17 +99,16 @@ RpcServerManager::addPeer(const char *remsbname, OkState -RpcServerManager::removePeer(const char *remsbname, - const char *remsbspec) +RpcServerManager::removePeer(const std::string & remsbname, const std::string & remsbspec) { - if (strcmp(remsbname, _env.mySpec()) == 0) { + if (remsbname == _env.mySpec()) { return OkState(13, "cannot remove my own rpcserver name"); } RemoteSlobrok *partner = _exchanger.lookupPartner(remsbname); - if (partner == NULL) { + if (partner == nullptr) { return OkState(0, "remote slobrok not a partner"); } - if (strcmp(partner->getSpec(), remsbspec) != 0) { + if (partner->getSpec() != remsbspec) { return OkState(13, "peer registered with different spec"); } _exchanger.removePartner(remsbname); @@ -123,20 +117,20 @@ RpcServerManager::removePeer(const char *remsbname, OkState -RpcServerManager::addMyReservation(const char *name, const char *spec) +RpcServerManager::addMyReservation(const std::string & name, const std::string & spec) { OkState valid = validateName(name); if (valid.failed()) return valid; NamedService *old = _rpcsrvmap.lookupManaged(name); - if (old != NULL) { - if (strcmp(old->getSpec(), spec) == 0) { + if (old != nullptr) { + if (old->getSpec() == spec) { // was alright already return OkState(0, "already registered"); } else { return OkState(FRTE_RPC_METHOD_FAILED, vespalib::make_string( "name %s registered (to %s), cannot register %s", - name, old->getSpec(), spec)); + name.c_str(), old->getSpec().c_str(), spec.c_str())); } } @@ -144,7 +138,7 @@ RpcServerManager::addMyReservation(const char *name, const char *spec) if (_rpcsrvmap.conflictingReservation(name, spec)) { const ReservedName * rsv = _rpcsrvmap.getReservation(name); LOG(warning, "conflicting registrations: wanted [%s -> %s] but [%s -> %s] already reserved", - name, spec, rsv->getName(), rsv->getSpec()); + name.c_str(), spec.c_str(), rsv->getName().c_str(), rsv->getSpec().c_str()); return OkState(FRTE_RPC_METHOD_FAILED, "registration for name already in progress with a different spec"); } @@ -156,8 +150,7 @@ RpcServerManager::addMyReservation(const char *name, const char *spec) OkState -RpcServerManager::addRemote(const char *name, - const char *spec) +RpcServerManager::addRemote(const std::string & name, const std::string &spec) { OkState valid = validateName(name); if (valid.failed()) return valid; @@ -166,12 +159,11 @@ RpcServerManager::addRemote(const char *name, return OkState(0, "already correct"); } NamedService *old = _rpcsrvmap.lookup(name); - if (old != NULL) { - if (strcmp(old->getSpec(), spec) != 0) { - LOG(warning, "collision on remote add: " - "name %s registered to %s locally, " + if (old != nullptr) { + if (old->getSpec() != spec) { + LOG(warning, "collision on remote add: name %s registered to %s locally, " "but another location broker wants it registered to %s", - name, old->getSpec(), spec); + name.c_str(), old->getSpec().c_str(), spec.c_str()); removeRemote(name, old->getSpec()); return OkState(13, "registered, with different spec"); } @@ -199,16 +191,15 @@ RpcServerManager::remove(ManagedRpcServer *rpcsrv) OkState -RpcServerManager::removeRemote(const char *name, - const char *spec) +RpcServerManager::removeRemote(const std::string &name, const std::string &spec) { NamedService *old = _rpcsrvmap.lookup(name); - if (old == NULL) { + if (old == nullptr) { // was alright already, remove any reservation too _rpcsrvmap.removeReservation(name); return OkState(0, "already done"); } - if (strcmp(old->getSpec(), spec) != 0) { + if (old->getSpec() != spec) { return OkState(1, "name registered, but with different spec"); } NamedService *td = _rpcsrvmap.remove(name); @@ -218,25 +209,25 @@ RpcServerManager::removeRemote(const char *name, } OkState -RpcServerManager::removeLocal(const char *name, const char *spec) +RpcServerManager::removeLocal(const std::string & name, const std::string &spec) { NamedService *td = _rpcsrvmap.lookup(name); - if (td == NULL) { + if (td == nullptr) { // already removed, nop return OkState(); } RemoteSlobrok *partner = _exchanger.lookupPartner(name); - if (partner != NULL) { + if (partner != nullptr) { return OkState(13, "cannot unregister partner slobrok"); } ManagedRpcServer *rpcsrv = _rpcsrvmap.lookupManaged(name); - if (rpcsrv == NULL) { + if (rpcsrv == nullptr) { return OkState(13, "not a local rpcserver"); } - if (strcmp(rpcsrv->getSpec(), spec) != 0) { + if (rpcsrv->getSpec() != spec) { // the client can probably ignore this "error" // or log it on level INFO? return OkState(1, "name registered, but with different spec"); @@ -250,13 +241,12 @@ RpcServerManager::removeLocal(const char *name, const char *spec) void -RpcServerManager::addManaged(const char *name, const char *spec, - RegRpcSrvCommand rdc) +RpcServerManager::addManaged(const std::string &name, const std::string &spec, RegRpcSrvCommand rdc) { ManagedRpcServer *rpcsrv = new ManagedRpcServer(name, spec, *this); _rpcsrvmap.addNew(rpcsrv); for (size_t i = 0; i < _addManageds.size(); i++) { - if (_addManageds[i].rpcsrv == NULL) { + if (_addManageds[i].rpcsrv == nullptr) { _addManageds[i].rpcsrv = rpcsrv; _addManageds[i].handler = rdc; rpcsrv->healthCheck(); @@ -272,11 +262,11 @@ RpcServerManager::addManaged(const char *name, const char *spec, bool -RpcServerManager::alreadyManaged(const char *name, const char *spec) +RpcServerManager::alreadyManaged(const std::string &name, const std::string &spec) { ManagedRpcServer *rpcsrv = _rpcsrvmap.lookupManaged(name); - if (rpcsrv != NULL) { - if (strcmp(rpcsrv->getSpec(), spec) == 0) { + if (rpcsrv != nullptr) { + if (rpcsrv->getSpec() == spec) { return true; } } @@ -312,7 +302,7 @@ RpcServerManager::notifyFailedRpcSrv(ManagedRpcServer *rpcsrv, std::string errms old = _rpcsrvmap.remove(rpcsrv->getName()); LOG_ASSERT(old == rpcsrv); LOG(info, "managed server %s at %s failed: %s", - rpcsrv->getName(), rpcsrv->getSpec(), errmsg.c_str()); + rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str(), errmsg.c_str()); logged = true; } _exchanger.forwardRemove(rpcsrv->getName(), rpcsrv->getSpec()); @@ -322,13 +312,13 @@ RpcServerManager::notifyFailedRpcSrv(ManagedRpcServer *rpcsrv, std::string errms .doneHandler(OkState(13, "failed check using listNames callback")); _addManageds[i].rpcsrv = 0; LOG(warning, "rpcserver %s at %s failed while trying to register", - rpcsrv->getName(), rpcsrv->getSpec()); + rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str()); logged = true; } } if (!logged) { LOG(warning, "unmanaged server %s at %s failed: %s", - rpcsrv->getName(), rpcsrv->getSpec(), errmsg.c_str()); + rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str(), errmsg.c_str()); } _deleteList.push_back(rpcsrv); ScheduleNow(); diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h index a01e1592690..9d3ba3c573e 100644 --- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h +++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h @@ -56,43 +56,26 @@ private: }; std::vector _addManageds; std::vector _deleteList; +public: + OkState checkPartner(const std::string & remslobrok); - RpcServerManager(const RpcServerManager &); // Not used - RpcServerManager &operator=(const RpcServerManager &); // Not used + 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); -public: - OkState checkPartner(const char *remslobrok); - - OkState addPeer(const char *remsbname, - const char *remsbspec); - OkState removePeer(const char *remsbname, - const char *remsbspec); - - OkState addLocal(const char *name, - const char *spec, - FRT_RPCRequest *req); - OkState addRemote(const char *name, - const char *spec); - - OkState addRemReservation(const char *remslobrok, - const char *name, - const char *spec); - OkState addMyReservation(const char *name, - const char *spec); - - bool alreadyManaged(const char *name, - const char *spec); - void addManaged(const char *name, - const char *spec, - RegRpcSrvCommand rdc); + OkState addRemReservation(const std::string & remslobrok, const std::string & name, const std::string & spec); + OkState addMyReservation(const std::string & name, const std::string & spec); - OkState remove(ManagedRpcServer *rpcsrv); + bool alreadyManaged(const std::string & name, const std::string & spec); + void addManaged(const std::string & name, const std::string & spec, RegRpcSrvCommand rdc); - OkState removeLocal(const char *name, const char *spec); + OkState remove(ManagedRpcServer *rpcsrv); - OkState removeRemote(const char *name, - const char *spec); + OkState removeLocal(const std::string & name, const std::string & spec); + OkState removeRemote(const std::string & name, const std::string & spec); + RpcServerManager(const RpcServerManager &) = delete; + RpcServerManager &operator=(const RpcServerManager &) = delete; RpcServerManager(SBEnv &sbenv); ~RpcServerManager(); diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp index 47fb7b2516e..34f8f7b9135 100644 --- a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp +++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp @@ -13,19 +13,19 @@ namespace slobrok { //----------------------------------------------------------------------------- ManagedRpcServer * -RpcServerMap::lookupManaged(const char *name) const { +RpcServerMap::lookupManaged(const std::string & name) const { auto found = _myrpcsrv_map.find(name); return (found == _myrpcsrv_map.end()) ? nullptr : found->second.get(); } NamedService * -RpcServerMap::lookup(const char *name) const +RpcServerMap::lookup(const std::string & name) const { return lookupManaged(name); } NamedService * -RpcServerMap::remove(const char *name) +RpcServerMap::remove(const std::string & name) { _visible_map.remove(name); auto service = std::move(_myrpcsrv_map[name]); @@ -61,7 +61,7 @@ RpcServerMap::allManaged() const void RpcServerMap::add(NamedService *rpcsrv) { - const char *name = rpcsrv->getName(); + const std::string &name = rpcsrv->getName(); LOG_ASSERT(rpcsrv != nullptr); LOG_ASSERT(_myrpcsrv_map.find(name) == _myrpcsrv_map.end()); @@ -75,7 +75,7 @@ RpcServerMap::add(NamedService *rpcsrv) void RpcServerMap::addNew(ManagedRpcServer *rpcsrv) { - const char *name = rpcsrv->getName(); + const std::string &name = rpcsrv->getName(); auto oldman = std::move(_myrpcsrv_map[name]); _myrpcsrv_map.erase(name); @@ -84,21 +84,21 @@ RpcServerMap::addNew(ManagedRpcServer *rpcsrv) const ReservedName *oldres = _reservations[name].get(); const NamedService *oldvis = _visible_map.remove(name); - const char *spec = rpcsrv->getSpec(); - const char *oldname = oldman->getName(); - const char *oldspec = oldman->getSpec(); - if (strcmp(spec, oldspec) != 0) { + const std::string &spec = rpcsrv->getSpec(); + const std::string &oldname = oldman->getName(); + const std::string &oldspec = oldman->getSpec(); + if (spec != oldspec) { LOG(warning, "internal state problem: adding [%s at %s] but already had [%s at %s]", - name, spec, oldname, oldspec); + name.c_str(), spec.c_str(), oldname.c_str(), oldspec.c_str()); if (oldvis != oldman.get()) { - const char *n = oldvis->getName(); - const char *s = oldvis->getSpec(); - LOG(warning, "BAD: different old visible: [%s at %s]", n, s); + const std::string &n = oldvis->getName(); + const std::string &s = oldvis->getSpec(); + LOG(warning, "BAD: different old visible: [%s at %s]", n.c_str(), s.c_str()); } if (oldres != nullptr) { - const char *n = oldres->getName(); - const char *s = oldres->getSpec(); - LOG(warning, "old reservation: [%s at %s]", n, s); + const std::string &n = oldres->getName(); + const std::string &s = oldres->getSpec(); + LOG(warning, "old reservation: [%s at %s]", n.c_str(), s.c_str()); } } } @@ -119,23 +119,23 @@ RpcServerMap::addReservation(ReservedName *rpcsrv) auto old = std::move(_reservations[rpcsrv->getName()]); _reservations[rpcsrv->getName()].reset(rpcsrv); LOG_ASSERT(!old - || strcmp(old->getSpec(), rpcsrv->getSpec()) == 0 + || old->getSpec() == rpcsrv->getSpec() || ! old->stillReserved()); } /** check if there is a (different) registration for this name in progress */ bool -RpcServerMap::conflictingReservation(const char *name, const char *spec) +RpcServerMap::conflictingReservation(const std::string &name, const std::string &spec) { const ReservedName *resv = _reservations[name].get(); return (resv != nullptr && resv->stillReserved() && - strcmp(resv->getSpec(), spec) != 0); + resv->getSpec() != spec); } const ReservedName * -RpcServerMap::getReservation(const char *name) const { +RpcServerMap::getReservation(const std::string &name) const { auto found = _reservations.find(name); return (found == _reservations.end()) ? nullptr : found->second.get(); } @@ -170,7 +170,7 @@ RpcServerMap::match(const char *name, const char *pattern) } void -RpcServerMap::removeReservation(const char *name) +RpcServerMap::removeReservation(const std::string & name) { _reservations.erase(name); } diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.h b/slobrok/src/vespa/slobrok/server/rpc_server_map.h index dd11a6c410e..d458df6cb1b 100644 --- a/slobrok/src/vespa/slobrok/server/rpc_server_map.h +++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.h @@ -36,21 +36,21 @@ public: VisibleMap& visibleMap() { return _visible_map; } - ManagedRpcServer *lookupManaged(const char *name) const; + ManagedRpcServer *lookupManaged(const std::string & name) const; - NamedService * lookup(const char *name) const; + NamedService * lookup(const std::string & name) const; RpcSrvlist lookupPattern(const char *pattern) const; RpcSrvlist allVisible() const; RpcSrvlist allManaged() const; void addNew(ManagedRpcServer *rpcsrv); - NamedService * remove(const char *name); + NamedService * remove(const std::string & name); void addReservation(ReservedName *rpcsrv); - bool conflictingReservation(const char *name, const char *spec); + bool conflictingReservation(const std::string & name, const std::string &spec); - const ReservedName *getReservation(const char *name) const; - void removeReservation(const char *name); + const ReservedName *getReservation(const std::string & name) const; + void removeReservation(const std::string & name); RpcServerMap(const RpcServerMap &) = delete; RpcServerMap &operator=(const RpcServerMap &) = delete; diff --git a/slobrok/src/vespa/slobrok/server/rpchooks.cpp b/slobrok/src/vespa/slobrok/server/rpchooks.cpp index 82e30a309a1..133de63c895 100644 --- a/slobrok/src/vespa/slobrok/server/rpchooks.cpp +++ b/slobrok/src/vespa/slobrok/server/rpchooks.cpp @@ -236,7 +236,7 @@ RPCHooks::rpc_listNamesServed(FRT_RPCRequest *req) { FRT_Values &dst = *req->GetReturn(); FRT_StringValue *names = dst.AddStringArray(1); - dst.SetString(names, _env.mySpec()); + dst.SetString(names, _env.mySpec().c_str()); _cnts.otherReqs++; return; } @@ -422,8 +422,8 @@ RPCHooks::rpc_lookupRpcServer(FRT_RPCRequest *req) FRT_StringValue *names = dst.AddStringArray(rpcsrvlist.size()); FRT_StringValue *specs = dst.AddStringArray(rpcsrvlist.size()); for (uint32_t i = 0; i < rpcsrvlist.size(); ++i) { - dst.SetString(&names[i], rpcsrvlist[i]->getName()); - dst.SetString(&specs[i], rpcsrvlist[i]->getSpec()); + dst.SetString(&names[i], rpcsrvlist[i]->getName().c_str()); + dst.SetString(&specs[i], rpcsrvlist[i]->getSpec().c_str()); } if (rpcsrvlist.size() < 1) { LOG(debug, "RPC: lookupRpcServers(%s) -> no match", @@ -431,7 +431,7 @@ RPCHooks::rpc_lookupRpcServer(FRT_RPCRequest *req) } else { LOG(debug, "RPC: lookupRpcServers(%s) -> %u matches, first [%s,%s]", rpcserverPattern, (unsigned int)rpcsrvlist.size(), - rpcsrvlist[0]->getName(), rpcsrvlist[0]->getSpec()); + rpcsrvlist[0]->getName().c_str(), rpcsrvlist[0]->getSpec().c_str()); } return; } @@ -446,15 +446,15 @@ RPCHooks::rpc_listManagedRpcServers(FRT_RPCRequest *req) FRT_StringValue *names = dst.AddStringArray(rpcsrvlist.size()); FRT_StringValue *specs = dst.AddStringArray(rpcsrvlist.size()); for (uint32_t i = 0; i < rpcsrvlist.size(); ++i) { - dst.SetString(&names[i], rpcsrvlist[i]->getName()); - dst.SetString(&specs[i], rpcsrvlist[i]->getSpec()); + dst.SetString(&names[i], rpcsrvlist[i]->getName().c_str()); + dst.SetString(&specs[i], rpcsrvlist[i]->getSpec().c_str()); } if (rpcsrvlist.size() < 1) { LOG(debug, "RPC: listManagedRpcServers() -> 0 managed"); } else { LOG(debug, "RPC: listManagedRpcServers() -> %u managed, first [%s,%s]", (unsigned int)rpcsrvlist.size(), - rpcsrvlist[0]->getName(), rpcsrvlist[0]->getSpec()); + rpcsrvlist[0]->getName().c_str(), rpcsrvlist[0]->getSpec().c_str()); } return; } @@ -472,8 +472,8 @@ RPCHooks::rpc_lookupManaged(FRT_RPCRequest *req) req->SetError(FRTE_RPC_METHOD_FAILED, "Not found"); } else { FRT_Values &dst = *req->GetReturn(); - dst.AddString(found->getName()); - dst.AddString(found->getSpec()); + dst.AddString(found->getName().c_str()); + dst.AddString(found->getSpec().c_str()); } return; } @@ -493,9 +493,9 @@ RPCHooks::rpc_listAllRpcServers(FRT_RPCRequest *req) int j = 0; for (uint32_t i = 0; i < mrpcsrvlist.size(); ++i, ++j) { - dst.SetString(&names[j], mrpcsrvlist[i]->getName()); - dst.SetString(&specs[j], mrpcsrvlist[i]->getSpec()); - dst.SetString(&owner[j], _env.mySpec()); + dst.SetString(&names[j], mrpcsrvlist[i]->getName().c_str()); + dst.SetString(&specs[j], mrpcsrvlist[i]->getSpec().c_str()); + dst.SetString(&owner[j], _env.mySpec().c_str()); } if (sz > 0) { diff --git a/slobrok/src/vespa/slobrok/server/rpcmirror.cpp b/slobrok/src/vespa/slobrok/server/rpcmirror.cpp index af5139f043c..46d7f70288b 100644 --- a/slobrok/src/vespa/slobrok/server/rpcmirror.cpp +++ b/slobrok/src/vespa/slobrok/server/rpcmirror.cpp @@ -38,8 +38,8 @@ MirrorFetch::completeReq() FRT_StringValue *names = dst.AddStringArray(sz); FRT_StringValue *specs = dst.AddStringArray(sz); for (uint32_t i = 0; i < rpcsrvlist.size(); ++i) { - dst.SetString(&names[i], rpcsrvlist[i]->getName()); - dst.SetString(&specs[i], rpcsrvlist[i]->getSpec()); + dst.SetString(&names[i], rpcsrvlist[i]->getName().c_str()); + dst.SetString(&specs[i], rpcsrvlist[i]->getSpec().c_str()); } if (sz > 0) { LOG(debug, "mirrorFetch %p -> %u, last [%s,%s]", @@ -145,8 +145,8 @@ IncrementalFetch::completeReq() FRT_StringValue *names = dst.AddStringArray(sz); FRT_StringValue *specs = dst.AddStringArray(sz); for (uint32_t i = 0; i < sz; ++i) { - dst.SetString(&names[i], diff.updated[i]->getName()); - dst.SetString(&specs[i], diff.updated[i]->getSpec()); + dst.SetString(&names[i], diff.updated[i]->getName().c_str()); + dst.SetString(&specs[i], diff.updated[i]->getSpec().c_str()); } dst.AddInt32(newgen.getAsInt()); diff --git a/slobrok/src/vespa/slobrok/server/sbenv.cpp b/slobrok/src/vespa/slobrok/server/sbenv.cpp index 35af0daf24a..163113de7b9 100644 --- a/slobrok/src/vespa/slobrok/server/sbenv.cpp +++ b/slobrok/src/vespa/slobrok/server/sbenv.cpp @@ -212,20 +212,18 @@ SBEnv::setup(const std::vector &cfg) std::string slobrok = cfg[i]; discard(oldList, slobrok); if (slobrok != mySpec()) { - OkState res = _rpcsrvmanager.addPeer(slobrok.c_str(), slobrok.c_str()); + OkState res = _rpcsrvmanager.addPeer(slobrok, slobrok.c_str()); if (!res.ok()) { - LOG(warning, "could not add peer %s: %s", slobrok.c_str(), - res.errorMsg.c_str()); + LOG(warning, "could not add peer %s: %s", slobrok.c_str(), res.errorMsg.c_str()); } else { LOG(config, "added peer %s", slobrok.c_str()); } } } for (uint32_t i = 0; i < oldList.size(); ++i) { - OkState res = _rpcsrvmanager.removePeer(oldList[i].c_str(), oldList[i].c_str()); + 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()); + 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()); } @@ -252,7 +250,7 @@ SBEnv::addPeer(const std::string &name, const std::string &spec) spec.c_str(), peers.c_str()); return OkState(FRTE_RPC_METHOD_FAILED, "configured partner list does not contain peer. configured peers = " + peers); } - return _rpcsrvmanager.addPeer(name.c_str(), spec.c_str()); + return _rpcsrvmanager.addPeer(name, spec.c_str()); } OkState @@ -266,7 +264,7 @@ 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.c_str(), spec.c_str()); + return _rpcsrvmanager.removePeer(name, spec.c_str()); } } // namespace slobrok diff --git a/slobrok/src/vespa/slobrok/server/sbenv.h b/slobrok/src/vespa/slobrok/server/sbenv.h index 8c897c9c999..223a8b7e9ee 100644 --- a/slobrok/src/vespa/slobrok/server/sbenv.h +++ b/slobrok/src/vespa/slobrok/server/sbenv.h @@ -73,7 +73,7 @@ public: ExchangeManager _exchanger; RpcServerMap _rpcsrvmap; - const char *mySpec() const { return _me->getSpec(); } + const std::string & mySpec() const { return _me->getSpec(); } bool isSuspended() const { return false; } bool isShuttingDown() const { return _shuttingDown; } diff --git a/slobrok/src/vespa/slobrok/server/selfcheck.cpp b/slobrok/src/vespa/slobrok/server/selfcheck.cpp index 7d1ca23957a..a5e6870882e 100644 --- a/slobrok/src/vespa/slobrok/server/selfcheck.cpp +++ b/slobrok/src/vespa/slobrok/server/selfcheck.cpp @@ -42,7 +42,7 @@ SelfCheck::PerformTask() const NamedService *r = mrpcsrvlist[i]; ManagedRpcServer *m = _rpcsrvmap.lookupManaged(r->getName()); LOG_ASSERT(r == m); - LOG(debug, "managed: %s -> %s", m->getName(), m->getSpec()); + LOG(debug, "managed: %s -> %s", m->getName().c_str(), m->getSpec().c_str()); m->healthCheck(); } // reschedule in 1-2 seconds: diff --git a/slobrok/src/vespa/slobrok/server/visible_map.cpp b/slobrok/src/vespa/slobrok/server/visible_map.cpp index 9508af74c9e..a9257dbb391 100644 --- a/slobrok/src/vespa/slobrok/server/visible_map.cpp +++ b/slobrok/src/vespa/slobrok/server/visible_map.cpp @@ -55,24 +55,11 @@ VisibleMap::removeUpdateListener(IUpdateListener *l) //----------------------------------------------------------------------------- NamedService * -VisibleMap::lookup(const char *name) const { +VisibleMap::lookup(const std::string &name) const { auto found = _map.find(name); return (found == _map.end()) ? nullptr : found->second; } -std::vector -VisibleMap::lookupPattern(const char *pattern) const -{ - std::vector retval; - for (const auto & entry : _map) { - if (match(entry.first.c_str(), pattern)) { - retval.push_back(entry.second); - } - } - return retval; -} - - std::vector VisibleMap::allVisible() const { @@ -99,7 +86,7 @@ VisibleMap::addNew(NamedService *rpcsrv) NamedService * -VisibleMap::remove(const char *name) { +VisibleMap::remove(const std::string &name) { NamedService *d = _map[name]; _map.erase(name); diff --git a/slobrok/src/vespa/slobrok/server/visible_map.h b/slobrok/src/vespa/slobrok/server/visible_map.h index 4c7c76bbdc0..5d2440cd176 100644 --- a/slobrok/src/vespa/slobrok/server/visible_map.h +++ b/slobrok/src/vespa/slobrok/server/visible_map.h @@ -63,11 +63,10 @@ public: void removeUpdateListener(IUpdateListener *l); void addNew(NamedService *rpcsrv); - NamedService *remove(const char *name); + NamedService *remove(const std::string &name); NamedService *update(NamedService *rpcsrv); - NamedService *lookup(const char *name) const; - RpcSrvlist lookupPattern(const char *pattern) const; + NamedService *lookup(const std::string &name) const; RpcSrvlist allVisible() const; const vespalib::GenCnt& genCnt() { return _genCnt; } -- cgit v1.2.3