You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2016/06/29 00:33:30 UTC
[5/6] incubator-kudu git commit: locks: switch from
boost::shared_mutex to new read-write mutex
locks: switch from boost::shared_mutex to new read-write mutex
Here are the changed mutexes:
1. log allocation state lock. Only taken before and after segment
preallocation, which doesn't seem like a hot path.
2. webserver lock: Protects the handler map and footer. Almost always
uncontended, and the web UI is not a hot path anyway.
3. tablet status listener lock: Protects the last status seen. Almost always
uncontended; it's acquired often by the remote bootstrap client, but the
only other acquirer would be the web UI or ListTablets() RPC.
4. scanner manager stripe lock: Protects a particular stripe's scanner map.
This one is a hot path with contention, I think.
Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de
Reviewed-on: http://gerrit.cloudera.org:8080/3499
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Dan Burkert <da...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/e84cfdf3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/e84cfdf3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/e84cfdf3
Branch: refs/heads/master
Commit: e84cfdf3ccf78ca62912e425869fb708affbf0fe
Parents: 89b1467
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Jun 24 18:43:08 2016 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Wed Jun 29 00:32:55 2016 +0000
----------------------------------------------------------------------
src/kudu/consensus/log.cc | 5 ++---
src/kudu/consensus/log.h | 6 +++---
src/kudu/server/webserver.cc | 10 +++++-----
src/kudu/server/webserver.h | 4 ++--
src/kudu/tablet/tablet_bootstrap.cc | 2 +-
src/kudu/tablet/tablet_bootstrap.h | 6 +++---
src/kudu/tserver/scanners.cc | 14 ++++++--------
src/kudu/tserver/scanners.h | 4 ++--
8 files changed, 24 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/consensus/log.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc
index c3da641..2e2e9f5 100644
--- a/src/kudu/consensus/log.cc
+++ b/src/kudu/consensus/log.cc
@@ -18,7 +18,6 @@
#include "kudu/consensus/log.h"
#include <algorithm>
-#include <boost/thread/shared_mutex.hpp>
#include <mutex>
#include "kudu/common/wire_protocol.h"
@@ -361,7 +360,7 @@ Status Log::Init() {
}
Status Log::AsyncAllocateSegment() {
- std::lock_guard<boost::shared_mutex> lock_guard(allocation_lock_);
+ std::lock_guard<RWMutex> l(allocation_lock_);
CHECK_EQ(allocation_state_, kAllocationNotStarted);
allocation_status_.Reset();
allocation_state_ = kAllocationInProgress;
@@ -848,7 +847,7 @@ Status Log::PreAllocateNewSegment() {
// We must mark allocation as finished when returning from this method.
auto alloc_finished = MakeScopedCleanup([&] () {
- std::lock_guard<boost::shared_mutex> l(allocation_lock_);
+ std::lock_guard<RWMutex> l(allocation_lock_);
allocation_state_ = kAllocationFinished;
});
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/consensus/log.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log.h b/src/kudu/consensus/log.h
index 331b9bf..e2a3e86 100644
--- a/src/kudu/consensus/log.h
+++ b/src/kudu/consensus/log.h
@@ -18,7 +18,6 @@
#ifndef KUDU_CONSENSUS_LOG_H_
#define KUDU_CONSENSUS_LOG_H_
-#include <boost/thread/shared_mutex.hpp>
#include <map>
#include <memory>
#include <string>
@@ -33,6 +32,7 @@
#include "kudu/util/async_util.h"
#include "kudu/util/blocking_queue.h"
#include "kudu/util/locks.h"
+#include "kudu/util/rw_mutex.h"
#include "kudu/util/promise.h"
#include "kudu/util/status.h"
@@ -307,7 +307,7 @@ class Log : public RefCountedThreadSafe<Log> {
}
const SegmentAllocationState allocation_state() {
- boost::shared_lock<boost::shared_mutex> shared_lock(allocation_lock_);
+ shared_lock<RWMutex> l(allocation_lock_);
return allocation_state_;
}
@@ -387,7 +387,7 @@ class Log : public RefCountedThreadSafe<Log> {
Promise<Status> allocation_status_;
// Read-write lock to protect 'allocation_state_'.
- mutable boost::shared_mutex allocation_lock_;
+ mutable RWMutex allocation_lock_;
SegmentAllocationState allocation_state_;
scoped_refptr<MetricEntity> metric_entity_;
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/server/webserver.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index 98bb545..858e485 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -18,7 +18,6 @@
#include <algorithm>
#include <boost/algorithm/string.hpp>
-#include <boost/thread/shared_mutex.hpp>
#include <functional>
#include <gflags/gflags.h>
#include <glog/logging.h>
@@ -39,6 +38,7 @@
#include "kudu/gutil/strings/stringpiece.h"
#include "kudu/util/env.h"
#include "kudu/util/flag_tags.h"
+#include "kudu/util/locks.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/url-coding.h"
#include "kudu/util/version_info.h"
@@ -267,7 +267,7 @@ int Webserver::BeginRequestCallback(struct sq_connection* connection,
struct sq_request_info* request_info) {
PathHandler* handler;
{
- boost::shared_lock<boost::shared_mutex> lock(lock_);
+ shared_lock<RWMutex> l(lock_);
PathHandlerMap::const_iterator it = path_handlers_.find(request_info->uri);
if (it == path_handlers_.end()) {
// Let Mongoose deal with this request; returning NULL will fall through
@@ -366,7 +366,7 @@ int Webserver::RunPathHandler(const PathHandler& handler,
void Webserver::RegisterPathHandler(const string& path, const string& alias,
const PathHandlerCallback& callback, bool is_styled, bool is_on_nav_bar) {
- std::lock_guard<boost::shared_mutex> lock(lock_);
+ std::lock_guard<RWMutex> l(lock_);
auto it = path_handlers_.find(path);
if (it == path_handlers_.end()) {
it = path_handlers_.insert(
@@ -424,12 +424,12 @@ bool Webserver::static_pages_available() const {
}
void Webserver::set_footer_html(const std::string& html) {
- std::lock_guard<boost::shared_mutex> l(lock_);
+ std::lock_guard<RWMutex> l(lock_);
footer_html_ = html;
}
void Webserver::BootstrapPageFooter(stringstream* output) {
- boost::shared_lock<boost::shared_mutex> l(lock_);
+ shared_lock<RWMutex> l(lock_);
*output << "</div>\n"; // end bootstrap 'container' div
if (!footer_html_.empty()) {
*output << "<footer class=\"footer\"><div class=\"container text-muted\">";
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/server/webserver.h
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver.h b/src/kudu/server/webserver.h
index 6f19f1a..ec50568 100644
--- a/src/kudu/server/webserver.h
+++ b/src/kudu/server/webserver.h
@@ -17,13 +17,13 @@
#ifndef KUDU_UTIL_WEBSERVER_H
#define KUDU_UTIL_WEBSERVER_H
-#include <boost/thread/shared_mutex.hpp>
#include <map>
#include <string>
#include <vector>
#include "kudu/server/webserver_options.h"
#include "kudu/util/net/sockaddr.h"
+#include "kudu/util/rw_mutex.h"
#include "kudu/util/status.h"
#include "kudu/util/web_callback_registry.h"
@@ -132,7 +132,7 @@ class Webserver : public WebCallbackRegistry {
const WebserverOptions opts_;
// Lock guarding the path_handlers_ map and footer_html.
- boost::shared_mutex lock_;
+ RWMutex lock_;
// Map of path to a PathHandler containing a list of handlers for that
// path. More than one handler may register itself with a path so that many
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/tablet/tablet_bootstrap.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc
index 3b96f11..6395d4e 100644
--- a/src/kudu/tablet/tablet_bootstrap.cc
+++ b/src/kudu/tablet/tablet_bootstrap.cc
@@ -392,7 +392,7 @@ TabletStatusListener::~TabletStatusListener() {
void TabletStatusListener::StatusMessage(const string& status) {
LOG(INFO) << "T " << tablet_id() << " P " << meta_->fs_manager()->uuid() << ": "
<< status;
- std::lock_guard<boost::shared_mutex> l(lock_);
+ std::lock_guard<RWMutex> l(lock_);
last_status_ = status;
}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/tablet/tablet_bootstrap.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap.h b/src/kudu/tablet/tablet_bootstrap.h
index be3353c..1566515 100644
--- a/src/kudu/tablet/tablet_bootstrap.h
+++ b/src/kudu/tablet/tablet_bootstrap.h
@@ -17,7 +17,6 @@
#ifndef KUDU_TABLET_TABLET_BOOTSTRAP_H_
#define KUDU_TABLET_TABLET_BOOTSTRAP_H_
-#include <boost/thread/shared_mutex.hpp>
#include <memory>
#include <string>
#include <vector>
@@ -26,6 +25,7 @@
#include "kudu/consensus/log.pb.h"
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/gutil/ref_counted.h"
+#include "kudu/util/rw_mutex.h"
#include "kudu/util/status.h"
namespace kudu {
@@ -70,12 +70,12 @@ class TabletStatusListener {
const Schema& schema() const;
std::string last_status() const {
- boost::shared_lock<boost::shared_mutex> l(lock_);
+ shared_lock<RWMutex> l(lock_);
return last_status_;
}
private:
- mutable boost::shared_mutex lock_;
+ mutable RWMutex lock_;
scoped_refptr<TabletMetadata> meta_;
std::string last_status_;
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/tserver/scanners.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/scanners.cc b/src/kudu/tserver/scanners.cc
index 13c588f..74e23d1 100644
--- a/src/kudu/tserver/scanners.cc
+++ b/src/kudu/tserver/scanners.cc
@@ -16,8 +16,6 @@
// under the License.
#include "kudu/tserver/scanners.h"
-#include <boost/bind.hpp>
-#include <boost/thread/shared_mutex.hpp>
#include <gflags/gflags.h>
#include <mutex>
@@ -117,27 +115,27 @@ void ScannerManager::NewScanner(const scoped_refptr<TabletPeer>& tablet_peer,
scanner->reset(new Scanner(id, tablet_peer, requestor_string, metrics_.get()));
ScannerMapStripe& stripe = GetStripeByScannerId(id);
- std::lock_guard<boost::shared_mutex> l(stripe.lock_);
+ std::lock_guard<RWMutex> l(stripe.lock_);
success = InsertIfNotPresent(&stripe.scanners_by_id_, id, *scanner);
}
}
bool ScannerManager::LookupScanner(const string& scanner_id, SharedScanner* scanner) {
ScannerMapStripe& stripe = GetStripeByScannerId(scanner_id);
- boost::shared_lock<boost::shared_mutex> l(stripe.lock_);
+ shared_lock<RWMutex> l(stripe.lock_);
return FindCopy(stripe.scanners_by_id_, scanner_id, scanner);
}
bool ScannerManager::UnregisterScanner(const string& scanner_id) {
ScannerMapStripe& stripe = GetStripeByScannerId(scanner_id);
- std::lock_guard<boost::shared_mutex> l(stripe.lock_);
+ std::lock_guard<RWMutex> l(stripe.lock_);
return stripe.scanners_by_id_.erase(scanner_id) > 0;
}
size_t ScannerManager::CountActiveScanners() const {
size_t total = 0;
for (const ScannerMapStripe* e : scanner_maps_) {
- boost::shared_lock<boost::shared_mutex> l(e->lock_);
+ shared_lock<RWMutex> l(e->lock_);
total += e->scanners_by_id_.size();
}
return total;
@@ -145,7 +143,7 @@ size_t ScannerManager::CountActiveScanners() const {
void ScannerManager::ListScanners(std::vector<SharedScanner>* scanners) {
for (const ScannerMapStripe* stripe : scanner_maps_) {
- boost::shared_lock<boost::shared_mutex> l(stripe->lock_);
+ shared_lock<RWMutex> l(stripe->lock_);
for (const ScannerMapEntry& se : stripe->scanners_by_id_) {
scanners->push_back(se.second);
}
@@ -156,7 +154,7 @@ void ScannerManager::RemoveExpiredScanners() {
MonoDelta scanner_ttl = MonoDelta::FromMilliseconds(FLAGS_scanner_ttl_ms);
for (ScannerMapStripe* stripe : scanner_maps_) {
- std::lock_guard<boost::shared_mutex> l(stripe->lock_);
+ std::lock_guard<RWMutex> l(stripe->lock_);
for (auto it = stripe->scanners_by_id_.begin(); it != stripe->scanners_by_id_.end();) {
SharedScanner& scanner = it->second;
MonoDelta time_live =
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/e84cfdf3/src/kudu/tserver/scanners.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/scanners.h b/src/kudu/tserver/scanners.h
index e114623..d9264e2 100644
--- a/src/kudu/tserver/scanners.h
+++ b/src/kudu/tserver/scanners.h
@@ -17,7 +17,6 @@
#ifndef KUDU_TSERVER_SCANNERS_H
#define KUDU_TSERVER_SCANNERS_H
-#include <boost/thread/shared_mutex.hpp>
#include <memory>
#include <mutex>
#include <string>
@@ -36,6 +35,7 @@
#include "kudu/util/monotime.h"
#include "kudu/util/mutex.h"
#include "kudu/util/oid_generator.h"
+#include "kudu/util/rw_mutex.h"
namespace kudu {
@@ -109,7 +109,7 @@ class ScannerManager {
struct ScannerMapStripe {
// Lock protecting the scanner map.
- mutable boost::shared_mutex lock_;
+ mutable RWMutex lock_;
// Map of the currently active scanners.
ScannerMap scanners_by_id_;
};