You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2018/03/30 21:16:07 UTC
[3/3] kudu git commit: meta_cache: improve multi-threaded scalability
meta_cache: improve multi-threaded scalability
When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().
This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.
This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).
Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Reviewed-on: http://gerrit.cloudera.org:8080/9760
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/4ebfbe41
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/4ebfbe41
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/4ebfbe41
Branch: refs/heads/master
Commit: 4ebfbe4182d7b9ccbe0bed1b2a3746467c42f05f
Parents: e36a674
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Mar 22 10:55:27 2018 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Mar 30 21:06:44 2018 +0000
----------------------------------------------------------------------
src/kudu/client/meta_cache.cc | 13 +++++--------
src/kudu/client/meta_cache.h | 9 +++++----
src/kudu/util/locks.h | 20 +++++++++++++++-----
3 files changed, 25 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/4ebfbe41/src/kudu/client/meta_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/meta_cache.cc b/src/kudu/client/meta_cache.cc
index aa77741..32f96b2 100644
--- a/src/kudu/client/meta_cache.cc
+++ b/src/kudu/client/meta_cache.cc
@@ -201,12 +201,10 @@ void RemoteTablet::Refresh(const TabletServerMap& tservers,
}
void RemoteTablet::MarkStale() {
- std::lock_guard<simple_spinlock> l(lock_);
stale_ = true;
}
bool RemoteTablet::stale() const {
- std::lock_guard<simple_spinlock> l(lock_);
return stale_;
}
@@ -820,7 +818,7 @@ Status MetaCache::ProcessLookupResponse(const LookupRpc& rpc,
MonoTime expiration_time = MonoTime::Now() +
MonoDelta::FromMilliseconds(rpc.resp().ttl_millis());
- std::lock_guard<rw_spinlock> l(lock_);
+ std::lock_guard<percpu_rwlock> l(lock_);
TabletMap& tablets_by_key = LookupOrInsert(&tablets_by_table_and_key_,
rpc.table_id(), TabletMap());
@@ -961,7 +959,7 @@ Status MetaCache::ProcessLookupResponse(const LookupRpc& rpc,
bool MetaCache::LookupEntryByKeyFastPath(const KuduTable* table,
const string& partition_key,
MetaCacheEntry* entry) {
- shared_lock<rw_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_.get_lock());
const TabletMap* tablets = FindOrNull(tablets_by_table_and_key_, table->id());
if (PREDICT_FALSE(!tablets)) {
// No cache available for this table.
@@ -1013,7 +1011,7 @@ Status MetaCache::DoFastPathLookup(const KuduTable* table,
void MetaCache::ClearNonCoveredRangeEntries(const std::string& table_id) {
VLOG(3) << "Clearing non-covered range entries of table " << table_id;
- std::lock_guard<rw_spinlock> l(lock_);
+ std::lock_guard<percpu_rwlock> l(lock_);
TabletMap* tablets = FindOrNull(tablets_by_table_and_key_, table_id);
if (PREDICT_FALSE(!tablets)) {
@@ -1032,7 +1030,7 @@ void MetaCache::ClearNonCoveredRangeEntries(const std::string& table_id) {
void MetaCache::ClearCache() {
VLOG(3) << "Clearing cache";
- std::lock_guard<rw_spinlock> l(lock_);
+ std::lock_guard<percpu_rwlock> l(lock_);
STLDeleteValues(&ts_cache_);
tablets_by_id_.clear();
tablets_by_table_and_key_.clear();
@@ -1068,10 +1066,9 @@ void MetaCache::LookupTabletByKey(const KuduTable* table,
void MetaCache::MarkTSFailed(RemoteTabletServer* ts,
const Status& status) {
LOG(INFO) << "Marking tablet server " << ts->ToString() << " as failed.";
- shared_lock<rw_spinlock> l(lock_);
-
Status ts_status = status.CloneAndPrepend("TS failed");
+ shared_lock<rw_spinlock> l(lock_.get_lock());
// TODO: replace with a ts->tablet multimap for faster lookup?
for (const auto& tablet : tablets_by_id_) {
// We just loop on all tablets; if a tablet does not have a replica on this
http://git-wip-us.apache.org/repos/asf/kudu/blob/4ebfbe41/src/kudu/client/meta_cache.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/meta_cache.h b/src/kudu/client/meta_cache.h
index 34232bc..fafd84b 100644
--- a/src/kudu/client/meta_cache.h
+++ b/src/kudu/client/meta_cache.h
@@ -19,6 +19,7 @@
#ifndef KUDU_CLIENT_META_CACHE_H
#define KUDU_CLIENT_META_CACHE_H
+#include <atomic>
#include <map>
#include <memory>
#include <set>
@@ -257,9 +258,9 @@ class RemoteTablet : public RefCountedThreadSafe<RemoteTablet> {
const std::string tablet_id_;
const Partition partition_;
- // All non-const members are protected by 'lock_'.
- mutable simple_spinlock lock_;
- bool stale_;
+ std::atomic<bool> stale_;
+
+ mutable simple_spinlock lock_; // Protects replicas_.
std::vector<RemoteReplica> replicas_;
DISALLOW_COPY_AND_ASSIGN(RemoteTablet);
@@ -461,7 +462,7 @@ class MetaCache : public RefCountedThreadSafe<MetaCache> {
KuduClient* client_;
- rw_spinlock lock_;
+ percpu_rwlock lock_;
// Cache of Tablet Server locations: TS UUID -> RemoteTabletServer*.
//
http://git-wip-us.apache.org/repos/asf/kudu/blob/4ebfbe41/src/kudu/util/locks.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/locks.h b/src/kudu/util/locks.h
index 51467b9..f70955c 100644
--- a/src/kudu/util/locks.h
+++ b/src/kudu/util/locks.h
@@ -132,9 +132,12 @@ class rw_spinlock {
// A reader-writer lock implementation which is biased for use cases where
// the write lock is taken infrequently, but the read lock is used often.
//
-// Internally, this creates N underlying mutexes, one per CPU. When a thread
-// wants to lock in read (shared) mode, it locks only its own CPU's mutex. When it
-// wants to lock in write (exclusive) mode, it locks all CPU's mutexes.
+// Internally, this creates N underlying reader-writer locks, one per CPU. When a thread
+// wants to lock in read (shared) mode, it locks only its own CPU's lock in read
+// mode. When it wants to lock in write (exclusive) mode, it locks all CPUs' rwlocks in
+// write mode. The use of reader-writer locks ensures that, even if a thread gets
+// preempted when holding one of the per-CPU locks in read mode, the next thread
+// scheduled onto that CPU will not need to block on the first thread.
//
// This means that in the read-mostly case, different readers will not cause any
// cacheline contention.
@@ -144,14 +147,14 @@ class rw_spinlock {
//
// // Lock shared:
// {
-// boost::shared_lock<rw_spinlock> lock(mylock.get_lock());
+// kudu::shared_lock<rw_spinlock> lock(mylock.get_lock());
// ...
// }
//
// // Lock exclusive:
//
// {
-// boost::lock_guard<percpu_rwlock> lock(mylock);
+// std::lock_guard<percpu_rwlock> lock(mylock);
// ...
// }
class percpu_rwlock {
@@ -207,6 +210,13 @@ class percpu_rwlock {
return false;
}
+ bool is_write_locked() const {
+ for (int i = 0; i < n_cpus_; i++) {
+ if (!locks_[i].lock.is_write_locked()) return false;
+ }
+ return true;
+ }
+
void lock() {
for (int i = 0; i < n_cpus_; i++) {
locks_[i].lock.lock();