kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject kudu git commit: KUDU-1801: catalog_manager: change TableInfo lock to a rwlock
Date Tue, 13 Dec 2016 01:57:11 GMT
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 <adar@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 <todd@apache.org>
Authored: Sun Dec 11 23:13:27 2016 +0700
Committer: Todd Lipcon <todd@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_;
 


Mime
View raw message