diff options
6 files changed, 53 insertions, 63 deletions
diff --git a/storage/src/vespa/storage/common/statusmetricconsumer.cpp b/storage/src/vespa/storage/common/statusmetricconsumer.cpp index 6d5b6b1eb2b..1c978198f5e 100644 --- a/storage/src/vespa/storage/common/statusmetricconsumer.cpp +++ b/storage/src/vespa/storage/common/statusmetricconsumer.cpp @@ -24,6 +24,7 @@ StatusMetricConsumer::StatusMetricConsumer( _manager(manager), _component(compReg, "statusmetricsconsumer"), _name(name), + _lock(), _startTime(_component.getClock().getTimeInSeconds()), _processedTime(0) { @@ -43,8 +44,7 @@ StatusMetricConsumer::updateMetrics(const MetricLockGuard & guard) } vespalib::string -StatusMetricConsumer::getReportContentType( - const framework::HttpUrlPath& path) const +StatusMetricConsumer::getReportContentType(const framework::HttpUrlPath& path) const { if (!path.hasAttribute("format")) { return "text/html"; @@ -177,7 +177,7 @@ StatusMetricConsumer::reportStatus(std::ostream& out, if (path.hasAttribute("task") && path.getAttribute("task") == "reset") { { - vespalib::MonitorGuard sync(_waiter); + std::lock_guard guard(_lock); _manager.reset(currentTime.getTime()); } if (html) { @@ -337,17 +337,9 @@ StatusMetricConsumer::reportStatus(std::ostream& out, } void -StatusMetricConsumer::waitUntilTimeProcessed(framework::SecondTime t) const +StatusMetricConsumer::waitUntilTimeProcessed(framework::SecondTime ) const { - return; // Return straight away as thread is not running now. - // This is used in unit testing to wait for internal thread to have - // generated snapshots. Wait aggressively and signal other thread to - // make it do it quick (as it uses fake timer) - vespalib::MonitorGuard sync(_waiter); - while (_processedTime < t) { - sync.signal(); - sync.wait(1); - } + return; } void @@ -389,12 +381,10 @@ StatusMetricConsumer::writeXmlTags(std::ostream& out, namespace { struct UnusedMetricPrinter : public metrics::MetricVisitor { - const std::map<metrics::Metric::String, - metrics::Metric::SP>& _usedMetrics; + const std::map<metrics::Metric::String, metrics::Metric::SP>& _usedMetrics; std::ostream& _out; - UnusedMetricPrinter(const std::map<metrics::Metric::String, - metrics::Metric::SP>& used, + UnusedMetricPrinter(const std::map<metrics::Metric::String, metrics::Metric::SP>& used, std::ostream& out) : _usedMetrics(used), _out(out) {} diff --git a/storage/src/vespa/storage/common/statusmetricconsumer.h b/storage/src/vespa/storage/common/statusmetricconsumer.h index 6f93f51cfdf..af6f07e0412 100644 --- a/storage/src/vespa/storage/common/statusmetricconsumer.h +++ b/storage/src/vespa/storage/common/statusmetricconsumer.h @@ -13,7 +13,6 @@ #include <vespa/storageframework/generic/status/statusreporter.h> #include <vespa/storageframework/generic/metric/metricupdatehook.h> #include <vespa/vespalib/util/document_runnable.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/metrics/metrics.h> #include <map> @@ -34,7 +33,7 @@ public: StorageComponentRegister&, metrics::MetricManager&, const std::string& name = "status"); - ~StatusMetricConsumer(); + ~StatusMetricConsumer() override; vespalib::string getReportContentType(const framework::HttpUrlPath&) const override; bool reportStatus(std::ostream& out, const framework::HttpUrlPath&) const override; @@ -48,11 +47,11 @@ private: typedef metrics::Metric::String String; metrics::MetricManager& _manager; - StorageComponent _component; - std::string _name; - vespalib::Monitor _waiter; - framework::SecondTime _startTime; - framework::SecondTime _processedTime; + StorageComponent _component; + std::string _name; + mutable std::mutex _lock; + framework::SecondTime _startTime; + framework::SecondTime _processedTime; void writeXmlTags(std::ostream& out, const vespalib::StringTokenizer& name, diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index cb624fb6a7f..e4563ec56ce 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -14,7 +14,6 @@ #include <vespa/storageframework/generic/status/xmlstatusreporter.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/vespalib/util/memoryusage.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/log/log.h> LOG_SETUP(".distributor-main"); @@ -25,13 +24,15 @@ namespace storage::distributor { class Distributor::Status { const DelegatedStatusRequest& _request; - vespalib::Monitor _monitor; - bool _done; + std::mutex _lock; + std::condition_variable _cond; + bool _done; public: Status(const DelegatedStatusRequest& request) noexcept : _request(request), - _monitor(), + _lock(), + _cond(), _done(false) {} @@ -46,14 +47,16 @@ public: } void notifyCompleted() { - vespalib::MonitorGuard guard(_monitor); - _done = true; - guard.broadcast(); + { + std::lock_guard guard(_lock); + _done = true; + } + _cond.notify_all(); } void waitForCompletion() { - vespalib::MonitorGuard guard(_monitor); + std::unique_lock guard(_lock); while (!_done) { - guard.wait(); + _cond.wait(guard); } } }; diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index e04bce902d4..a041ab0cfff 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -1,13 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "statemanager.h" +#include "storagemetricsset.h" #include <vespa/defaults.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/metrics/jsonwriter.h> #include <vespa/metrics/metricmanager.h> #include <vespa/storage/common/bucketoperationlogger.h> -#include <vespa/storage/config/config-stor-server.h> -#include <vespa/storage/storageserver/storagemetricsset.h> #include <vespa/storageapi/messageapi/storagemessage.h> #include <vespa/vdslib/state/cluster_state_bundle.h> #include <vespa/vespalib/io/fileutil.h> @@ -16,7 +15,6 @@ #include <vespa/vespalib/util/stringfmt.h> #include <fstream> -#include <sys/types.h> #include <unistd.h> #include <vespa/log/log.h> @@ -35,6 +33,7 @@ StateManager::StateManager(StorageComponentRegister& compReg, _component(compReg, "statemanager"), _metricManager(metricManager), _stateLock(), + _stateCond(), _listenerLock(), _nodeState(std::make_shared<lib::NodeState>(_component.getNodeType(), lib::State::INITIALIZING)), _nextNodeState(), @@ -147,7 +146,7 @@ StateManager::reportHtmlStatus(std::ostream& out, #endif { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); const auto &baseLineClusterState = _systemState->getBaselineClusterState(); out << "<h1>Current system state</h1>\n" << "<code>" << baseLineClusterState->toString(true) << "</code>\n" @@ -180,14 +179,14 @@ StateManager::thisNode() const lib::NodeState::CSP StateManager::getReportedNodeState() const { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); return _nodeState; } lib::NodeState::CSP StateManager::getCurrentNodeState() const { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); return std::make_shared<const lib::NodeState> (_systemState->getBaselineClusterState()->getNodeState(thisNode())); } @@ -195,7 +194,7 @@ StateManager::getCurrentNodeState() const std::shared_ptr<const lib::ClusterStateBundle> StateManager::getClusterStateBundle() const { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); return _systemState; } @@ -225,10 +224,10 @@ struct StateManager::ExternalStateLock : public NodeStateUpdater::Lock { explicit ExternalStateLock(StateManager& manager) noexcept : _manager(manager) {} ~ExternalStateLock() override { { - vespalib::MonitorGuard lock(_manager._stateLock); + std::lock_guard lock(_manager._stateLock); _manager._grabbedExternalLock = false; - lock.broadcast(); } + _manager._stateCond.notify_all(); _manager.notifyStateListeners(); } }; @@ -236,9 +235,9 @@ struct StateManager::ExternalStateLock : public NodeStateUpdater::Lock { NodeStateUpdater::Lock::SP StateManager::grabStateChangeLock() { - vespalib::MonitorGuard lock(_stateLock); + std::unique_lock guard(_stateLock); while (_grabbedExternalLock || _nextNodeState.get()) { - lock.wait(); + _stateCond.wait(guard); } _grabbedExternalLock = true; return std::make_shared<ExternalStateLock>(*this); @@ -247,7 +246,7 @@ StateManager::grabStateChangeLock() void StateManager::setReportedNodeState(const lib::NodeState& state) { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); if (!_grabbedExternalLock) { LOG(error, "Cannot set reported node state without first having " @@ -285,10 +284,10 @@ StateManager::notifyStateListeners() lib::NodeState::SP newState; while (true) { { - vespalib::MonitorGuard stateLock(_stateLock); + std::lock_guard guard(_stateLock); if (!_nextNodeState && !_nextSystemState) { _notifyingListeners = false; - stateLock.broadcast(); + _stateCond.notify_all(); break; // No change } if (_nextNodeState) { @@ -323,7 +322,7 @@ StateManager::notifyStateListeners() if (_nextSystemState) { enableNextClusterState(); } - stateLock.broadcast(); + _stateCond.notify_all(); } for (auto* listener : _stateListeners) { listener->handleNewState(); @@ -451,7 +450,7 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd) } std::shared_ptr<api::GetNodeStateReply> reply; { - vespalib::LockGuard lock(_stateLock); + std::unique_lock guard(_stateLock); const bool is_up_to_date = (_controllers_observed_explicit_node_state.find(cmd->getSourceIndex()) != _controllers_observed_explicit_node_state.end()); if (cmd->getExpectedState() != nullptr @@ -478,8 +477,8 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd) : cmd->getExpectedState()->toString().c_str(), _nodeState->toString().c_str()); reply = std::make_shared<api::GetNodeStateReply>(*cmd, *_nodeState); - mark_controller_as_having_observed_explicit_node_state(lock, cmd->getSourceIndex()); - lock.unlock(); + mark_controller_as_having_observed_explicit_node_state(guard, cmd->getSourceIndex()); + guard.unlock(); reply->setNodeInfo(getNodeInfo()); } } @@ -489,7 +488,8 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd) return true; } -void StateManager::mark_controller_as_having_observed_explicit_node_state(const vespalib::LockGuard &, uint16_t controller_index) { +void +StateManager::mark_controller_as_having_observed_explicit_node_state(const std::unique_lock<std::mutex> &, uint16_t controller_index) { _controllers_observed_explicit_node_state.emplace(controller_index); } @@ -497,7 +497,7 @@ void StateManager::setClusterStateBundle(const ClusterStateBundle& c) { { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); _nextSystemState = std::make_shared<const ClusterStateBundle>(c); } notifyStateListeners(); @@ -518,7 +518,7 @@ StateManager::onActivateClusterStateVersion( { auto reply = std::make_shared<api::ActivateClusterStateVersionReply>(*cmd); { - vespalib::LockGuard lock(_stateLock); + std::lock_guard lock(_stateLock); reply->setActualVersion(_systemState ? _systemState->getVersion() : 0); } sendUp(reply); @@ -553,7 +553,7 @@ StateManager::sendGetNodeStateReplies(framework::MilliSecTime olderThanTime, uin { std::vector<std::shared_ptr<api::GetNodeStateReply>> replies; { - vespalib::LockGuard guard(_stateLock); + std::unique_lock guard(_stateLock); for (auto it = _queuedStateRequests.begin(); it != _queuedStateRequests.end();) { if (node != 0xffff && node != it->second->getSourceIndex()) { ++it; @@ -628,7 +628,7 @@ StateManager::getNodeInfo() const // - the public getSystemState() need (and should) grab a lock on // _systemLock. // - getNodeInfo() (this function) always acquires the same lock. - vespalib::MonitorGuard guard(_stateLock); + std::lock_guard guard(_stateLock); stream << "cluster-state-version" << _systemState->getVersion(); _hostInfo->printReport(stream); @@ -650,7 +650,7 @@ StateManager::getNodeInfo() const void StateManager::immediately_send_get_node_state_replies() { LOG(debug, "Immediately replying to all pending GetNodeState requests"); { - vespalib::MonitorGuard guard(_stateLock); + std::lock_guard guard(_stateLock); // Next GetNodeState request from any controller will be replied to instantly _controllers_observed_explicit_node_state.clear(); } diff --git a/storage/src/vespa/storage/storageserver/statemanager.h b/storage/src/vespa/storage/storageserver/statemanager.h index 215e344e4a4..1731998c14f 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.h +++ b/storage/src/vespa/storage/storageserver/statemanager.h @@ -20,7 +20,6 @@ #include <vespa/storageframework/generic/status/htmlstatusreporter.h> #include <vespa/storageapi/message/state.h> #include <vespa/storageapi/messageapi/storagemessage.h> -#include <vespa/vespalib/util/sync.h> #include <vespa/vespalib/objects/floatingpointtype.h> #include <deque> #include <map> @@ -44,7 +43,8 @@ class StateManager : public NodeStateUpdater, { StorageComponent _component; metrics::MetricManager& _metricManager; - vespalib::Monitor _stateLock; + mutable std::mutex _stateLock; + std::condition_variable _stateCond; std::mutex _listenerLock; std::shared_ptr<lib::NodeState> _nodeState; std::shared_ptr<lib::NodeState> _nextNodeState; @@ -52,8 +52,7 @@ class StateManager : public NodeStateUpdater, std::shared_ptr<const ClusterStateBundle> _systemState; std::shared_ptr<const ClusterStateBundle> _nextSystemState; std::list<StateListener*> _stateListeners; - typedef std::pair<framework::MilliSecTime, - api::GetNodeStateCommand::SP> TimeStatePair; + typedef std::pair<framework::MilliSecTime, api::GetNodeStateCommand::SP> TimeStatePair; std::list<TimeStatePair> _queuedStateRequests; mutable std::mutex _threadLock; std::condition_variable _threadCond; @@ -107,7 +106,7 @@ private: bool sendGetNodeStateReplies( framework::MilliSecTime olderThanTime = framework::MilliSecTime(0), uint16_t index = 0xffff); - void mark_controller_as_having_observed_explicit_node_state(const vespalib::LockGuard &, uint16_t controller_index); + void mark_controller_as_having_observed_explicit_node_state(const std::unique_lock<std::mutex> &, uint16_t controller_index); lib::Node thisNode() const; diff --git a/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp b/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp index 7913cdcfe84..51afa75ea98 100644 --- a/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp +++ b/storageframework/src/vespa/storageframework/generic/thread/tickingthread.cpp @@ -2,7 +2,6 @@ #include "tickingthread.h" #include "threadpool.h" #include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/vespalib/util/sync.h> #include <cassert> namespace storage::framework { |