diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2022-03-04 16:53:42 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-04 16:53:42 +0100 |
commit | cdf6ccbe84f4f6682794ec746565365dcf38ec51 (patch) | |
tree | 55231edc83cc162b19727ac71964f940a398a739 /slobrok | |
parent | 51f12b66bcd5fcaa88a2d1c1d60136e12c476172 (diff) |
Revert "Fix Slobrok race between completion callback and scheduled task [run-systemtest]"
Diffstat (limited to 'slobrok')
-rw-r--r-- | slobrok/src/vespa/slobrok/sbregister.cpp | 74 |
1 files changed, 37 insertions, 37 deletions
diff --git a/slobrok/src/vespa/slobrok/sbregister.cpp b/slobrok/src/vespa/slobrok/sbregister.cpp index f00da4a9515..cdab4d4009d 100644 --- a/slobrok/src/vespa/slobrok/sbregister.cpp +++ b/slobrok/src/vespa/slobrok/sbregister.cpp @@ -67,8 +67,8 @@ RegisterAPI::RegisterAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config _names(), _pending(), _unreg(), - _target(nullptr), - _req(nullptr) + _target(0), + _req(0) { _configurator->poll(); if ( ! _slobrokSpecs.ok()) { @@ -82,12 +82,12 @@ RegisterAPI::RegisterAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config RegisterAPI::~RegisterAPI() { Kill(); - _configurator.reset(); - if (_req != nullptr) { + _configurator.reset(0); + if (_req != 0) { _req->Abort(); _req->SubRef(); } - if (_target != nullptr) { + if (_target != 0) { _target->SubRef(); } } @@ -96,15 +96,15 @@ RegisterAPI::~RegisterAPI() void RegisterAPI::registerName(vespalib::stringref name) { - std::lock_guard guard(_lock); - for (const auto& existing_name : _names) { - if (existing_name == name) { + std::lock_guard<std::mutex> guard(_lock); + for (uint32_t i = 0; i < _names.size(); ++i) { + if (_names[i] == name) { return; } } _busy.store(true, std::memory_order_relaxed); - _names.emplace_back(name); - _pending.emplace_back(name); + _names.push_back(name); + _pending.push_back(name); discard(_unreg, name); ScheduleNow(); } @@ -113,11 +113,11 @@ RegisterAPI::registerName(vespalib::stringref name) void RegisterAPI::unregisterName(vespalib::stringref name) { - std::lock_guard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); _busy.store(true, std::memory_order_relaxed); discard(_names, name); discard(_pending, name); - _unreg.emplace_back(name); + _unreg.push_back(name); ScheduleNow(); } @@ -125,7 +125,6 @@ RegisterAPI::unregisterName(vespalib::stringref name) void RegisterAPI::handleReqDone() { - std::lock_guard guard(_lock); if (_reqDone) { _reqDone = false; if (_req->IsError()) { @@ -134,10 +133,10 @@ RegisterAPI::handleReqDone() _req->GetErrorMessage(), _req->GetErrorCode()); // unexpected error; close our connection to this // slobrok server and try again with a fresh slate - if (_target != nullptr) { + if (_target != 0) { _target->SubRef(); } - _target = nullptr; + _target = 0; _busy.store(true, std::memory_order_relaxed); } else { LOG(warning, "%s(%s -> %s) failed: %s", @@ -147,7 +146,7 @@ RegisterAPI::handleReqDone() _req->GetErrorMessage()); } } else { - if (_logOnSuccess && _pending.empty() && !_names.empty()) { + if (_logOnSuccess && (_pending.size() == 0) && (_names.size() > 0)) { LOG(info, "[RPC @ %s] registering %s with location broker %s completed successfully", createSpec(_orb).c_str(), _names[0].c_str(), _currSlobrok.c_str()); _logOnSuccess = false; @@ -156,7 +155,7 @@ RegisterAPI::handleReqDone() _backOff.reset(); } _req->SubRef(); - _req = nullptr; + _req = 0; } } @@ -164,29 +163,29 @@ RegisterAPI::handleReqDone() void RegisterAPI::handleReconnect() { - if (_configurator->poll() && _target != nullptr) { + if (_configurator->poll() && _target != 0) { if (! _slobrokSpecs.contains(_currSlobrok)) { vespalib::string cps = _slobrokSpecs.logString(); LOG(warning, "[RPC @ %s] location broker %s removed, will disconnect and use one of: %s", createSpec(_orb).c_str(), _currSlobrok.c_str(), cps.c_str()); _target->SubRef(); - _target = nullptr; + _target = 0; } } - if (_target == nullptr) { + if (_target == 0) { _logOnSuccess = true; _currSlobrok = _slobrokSpecs.nextSlobrokSpec(); - if (!_currSlobrok.empty()) { + if (_currSlobrok.size() > 0) { // try next possible server. _target = _orb.GetTarget(_currSlobrok.c_str()); } { - std::lock_guard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); // now that we have a new connection, we need to // immediately re-register everything. _pending = _names; } - if (_target == nullptr) { + if (_target == 0) { // we have tried all possible servers. // start from the top after a delay, // possibly with a warning. @@ -212,13 +211,13 @@ RegisterAPI::handlePending() bool reg = false; vespalib::string name; { - std::lock_guard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); // pop off the todo stack, unregister has priority - if (!_unreg.empty()) { + if (_unreg.size() > 0) { name = _unreg.back(); _unreg.pop_back(); unreg = true; - } else if (!_pending.empty()) { + } else if (_pending.size() > 0) { name = _pending.back(); _pending.pop_back(); reg = true; @@ -245,7 +244,7 @@ RegisterAPI::handlePending() } else { // nothing more to do right now; schedule to re-register all // names after a long delay. - std::lock_guard guard(_lock); + std::lock_guard<std::mutex> guard(_lock); _pending = _names; LOG(debug, "done, reschedule in 30s"); _busy.store(false, std::memory_order_relaxed); @@ -257,25 +256,23 @@ void RegisterAPI::PerformTask() { handleReqDone(); - if (_req != nullptr) { + if (_req != 0) { LOG(debug, "req in progress"); return; // current request still in progress, don't start anything new } handleReconnect(); // still no connection? - if (_target == nullptr) return; + if (_target == 0) return; handlePending(); } void -RegisterAPI::RequestDone([[maybe_unused]] FRT_RPCRequest *req) +RegisterAPI::RequestDone(FRT_RPCRequest *req) { - { - std::lock_guard guard(_lock); - LOG_ASSERT(req == _req && !_reqDone); - _reqDone = true; - } + LOG_ASSERT(req == _req && !_reqDone); + (void) req; + _reqDone = true; ScheduleNow(); } @@ -299,13 +296,16 @@ RegisterAPI::RPCHooks::RPCHooks(RegisterAPI &owner) } -RegisterAPI::RPCHooks::~RPCHooks() = default; +RegisterAPI::RPCHooks::~RPCHooks() +{ +} + void RegisterAPI::RPCHooks::rpc_listNamesServed(FRT_RPCRequest *req) { FRT_Values &dst = *req->GetReturn(); - std::lock_guard guard(_owner._lock); + std::lock_guard<std::mutex> guard(_owner._lock); FRT_StringValue *names = dst.AddStringArray(_owner._names.size()); for (uint32_t i = 0; i < _owner._names.size(); ++i) { dst.SetString(&names[i], _owner._names[i].c_str()); |