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 2016/12/13 01:57:11 UTC
kudu git commit: KUDU-1801: catalog_manager: change TableInfo lock to
a rwlock
Repository: kudu
Updated Branches:
refs/heads/master c2c76b4c8 -> d59a9653a
KUDU-1801: catalog_manager: change TableInfo lock to a rwlock
This lock was showing a lot of contention on a 200-node cluster with
40 concurrent Impala queries. This patch does a straightforward
substitution of a rw spinlock.
I deployed this on the cluster with the same workload and measured
metrics before and after:
Before:
{
"name": "handler_latency_kudu_master_MasterService_GetTableLocations",
"total_count": 10082475,
"min": 7,
"mean": 1229.28,
"percentile_75": 2128,
"percentile_95": 3456,
"percentile_99": 5056,
"percentile_99_9": 7680,
"percentile_99_99": 12928,
"max": 56026,
"total_sum": 12394196985
},
{
"name": "handler_latency_kudu_master_MasterService_GetTableSchema",
"total_count": 8372070,
"min": 18,
"mean": 2121.11,
"percentile_75": 2928,
"percentile_95": 4352,
"percentile_99": 6112,
"percentile_99_9": 9024,
"percentile_99_99": 16896,
"max": 67530,
"total_sum": 17758104544
}
After:
{
"name": "handler_latency_kudu_master_MasterService_GetTableLocations",
"total_count": 1231543,
"min": 2,
"mean": 86.0921,
"percentile_75": 101,
"percentile_95": 139,
"percentile_99": 176,
"percentile_99_9": 258,
"percentile_99_99": 868,
"max": 364560,
"total_sum": 106026108
},
{
"name": "handler_latency_kudu_master_MasterService_GetTableSchema",
"total_count": 1164715,
"min": 2,
"mean": 554.124,
"percentile_75": 620,
"percentile_95": 784,
"percentile_99": 1056,
"percentile_99_9": 1624,
"percentile_99_99": 6176,
"max": 364211,
"total_sum": 645396044
}
Improvement for GetTableLocations:
percentile_99_9: 29.8x
percentile_99_99: 14.9x
min: 3.5x
percentile_95: 24.9x
percentile_75: 21.1x
percentile_99: 28.7x
mean: 14.3x
Improvement for GetTableSchema:
percentile_99_9: 5.6x
percentile_99_99: 2.7x
min: 9.0x
percentile_95: 5.6x
percentile_75: 4.7x
percentile_99: 5.8x
mean: 3.8x
Change-Id: Id77edbf7641275b54010514379cdfff228b408b5
Reviewed-on: http://gerrit.cloudera.org:8080/5471
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d59a9653
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d59a9653
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d59a9653
Branch: refs/heads/master
Commit: d59a9653a46d40c868c78fac6502e9a24e253731
Parents: c2c76b4
Author: Todd Lipcon <to...@apache.org>
Authored: Sun Dec 11 23:13:27 2016 +0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Dec 13 01:31:14 2016 +0000
----------------------------------------------------------------------
src/kudu/master/catalog_manager.cc | 26 +++++++++++++-------------
src/kudu/master/catalog_manager.h | 6 +++---
2 files changed, 16 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/d59a9653/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 7923162..c360730 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -3824,17 +3824,17 @@ std::string TableInfo::ToString() const {
}
bool TableInfo::RemoveTablet(const std::string& partition_key_start) {
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
return EraseKeyReturnValuePtr(&tablet_map_, partition_key_start) != nullptr;
}
void TableInfo::AddTablet(TabletInfo *tablet) {
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
AddTabletUnlocked(tablet);
}
void TableInfo::AddTablets(const vector<TabletInfo*>& tablets) {
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
for (TabletInfo *tablet : tablets) {
AddTabletUnlocked(tablet);
}
@@ -3842,7 +3842,7 @@ void TableInfo::AddTablets(const vector<TabletInfo*>& tablets) {
void TableInfo::AddRemoveTablets(const vector<scoped_refptr<TabletInfo>>& tablets_to_add,
const vector<scoped_refptr<TabletInfo>>& tablets_to_drop) {
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
for (const auto& tablet : tablets_to_drop) {
const auto& lower_bound = tablet->metadata().state().pb.partition().partition_key_start();
CHECK(EraseKeyReturnValuePtr(&tablet_map_, lower_bound) != nullptr);
@@ -3866,7 +3866,7 @@ void TableInfo::AddTabletUnlocked(TabletInfo* tablet) {
void TableInfo::GetTabletsInRange(const GetTableLocationsRequestPB* req,
vector<scoped_refptr<TabletInfo> > *ret) const {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
int max_returned_locations = req->max_returned_locations();
TableInfo::TabletInfoMap::const_iterator it, it_end;
@@ -3893,7 +3893,7 @@ void TableInfo::GetTabletsInRange(const GetTableLocationsRequestPB* req,
}
bool TableInfo::IsAlterInProgress(uint32_t version) const {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) {
if (e.second->reported_schema_version() < version) {
VLOG(3) << "Table " << table_id_ << " ALTER in progress due to tablet "
@@ -3906,7 +3906,7 @@ bool TableInfo::IsAlterInProgress(uint32_t version) const {
}
bool TableInfo::IsCreateInProgress() const {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) {
TabletMetadataLock tablet_lock(e.second, TabletMetadataLock::READ);
if (!tablet_lock.data().is_running()) {
@@ -3919,14 +3919,14 @@ bool TableInfo::IsCreateInProgress() const {
void TableInfo::AddTask(MonitoredTask* task) {
task->AddRef();
{
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
pending_tasks_.insert(task);
}
}
void TableInfo::RemoveTask(MonitoredTask* task) {
{
- std::lock_guard<simple_spinlock> l(lock_);
+ std::lock_guard<rw_spinlock> l(lock_);
pending_tasks_.erase(task);
}
@@ -3936,7 +3936,7 @@ void TableInfo::RemoveTask(MonitoredTask* task) {
}
void TableInfo::AbortTasks() {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
for (MonitoredTask* task : pending_tasks_) {
task->Abort();
}
@@ -3946,7 +3946,7 @@ void TableInfo::WaitTasksCompletion() {
int wait_time = 5;
while (1) {
{
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
if (pending_tasks_.empty()) {
break;
}
@@ -3957,7 +3957,7 @@ void TableInfo::WaitTasksCompletion() {
}
void TableInfo::GetTaskList(std::vector<scoped_refptr<MonitoredTask> > *ret) {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
for (MonitoredTask* task : pending_tasks_) {
ret->push_back(make_scoped_refptr(task));
}
@@ -3965,7 +3965,7 @@ void TableInfo::GetTaskList(std::vector<scoped_refptr<MonitoredTask> > *ret) {
void TableInfo::GetAllTablets(vector<scoped_refptr<TabletInfo> > *ret) const {
ret->clear();
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
for (const auto& e : tablet_map_) {
ret->push_back(make_scoped_refptr(e.second));
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/d59a9653/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index aa15c1b..3820e86 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -229,13 +229,13 @@ class TableInfo : public RefCountedThreadSafe<TableInfo> {
// Returns a snapshot copy of the table info's tablet map.
TabletInfoMap tablet_map() const {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
return tablet_map_;
}
// Returns the number of tablets.
int num_tablets() const {
- std::lock_guard<simple_spinlock> l(lock_);
+ shared_lock<rw_spinlock> l(lock_);
return tablet_map_.size();
}
@@ -252,7 +252,7 @@ class TableInfo : public RefCountedThreadSafe<TableInfo> {
TabletInfoMap tablet_map_;
// Protects tablet_map_ and pending_tasks_
- mutable simple_spinlock lock_;
+ mutable rw_spinlock lock_;
CowObject<PersistentTableInfo> metadata_;