kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/3] kudu git commit: KUDU-2193 (part 1): switch to a waiting mutex in TSTabletManager
Date Wed, 01 Nov 2017 05:00:19 GMT
KUDU-2193 (part 1): switch to a waiting mutex in TSTabletManager

The TSTabletManager map is sometimes held for a long time. Given that, a
sleeping mutex is more appropriate than a spinlock.

Change-Id: I763abddd74d8b1dabb618318dc84256b533077e3
Reviewed-on: http://gerrit.cloudera.org:8080/8345
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/94bf6996
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/94bf6996
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/94bf6996

Branch: refs/heads/master
Commit: 94bf699645120bbe2a0e9f3d3a33ff65b8214dce
Parents: 0bd8800
Author: Todd Lipcon <todd@apache.org>
Authored: Thu Oct 19 16:46:36 2017 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Nov 1 04:43:50 2017 +0000

----------------------------------------------------------------------
 src/kudu/tserver/ts_tablet_manager.cc | 40 +++++++++++++++---------------
 src/kudu/tserver/ts_tablet_manager.h  |  9 ++++---
 2 files changed, 25 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/94bf6996/src/kudu/tserver/ts_tablet_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index 56a9478..5ff61d5 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -299,7 +299,7 @@ Status TSTabletManager::Init() {
   for (const scoped_refptr<TabletMetadata>& meta : metas) {
     scoped_refptr<TransitionInProgressDeleter> deleter;
     {
-      std::lock_guard<rw_spinlock> lock(lock_);
+      std::lock_guard<RWMutex> lock(lock_);
       CHECK_OK(StartTabletStateTransitionUnlocked(meta->tablet_id(), "opening tablet",
&deleter));
     }
 
@@ -310,7 +310,7 @@ Status TSTabletManager::Init() {
   }
 
   {
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     state_ = MANAGER_RUNNING;
   }
 
@@ -322,7 +322,7 @@ Status TSTabletManager::WaitForAllBootstrapsToFinish() {
 
   open_tablet_pool_->Wait();
 
-  shared_lock<rw_spinlock> l(lock_);
+  shared_lock<RWMutex> l(lock_);
   for (const TabletMap::value_type& entry : tablet_map_) {
     if (entry.second->state() == tablet::FAILED) {
       return entry.second->error();
@@ -350,7 +350,7 @@ Status TSTabletManager::CreateNewTablet(const string& table_id,
   {
     // acquire the lock in exclusive mode as we'll add a entry to the
     // transition_in_progress_ set if the lookup fails.
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     TRACE("Acquired tablet manager lock");
 
     // Sanity check that the tablet isn't already registered.
@@ -475,7 +475,7 @@ void TSTabletManager::StartTabletCopy(
   boost::optional<string> transition;
   {
     // Lock must be dropped before executing callbacks.
-    shared_lock<rw_spinlock> lock(lock_);
+    shared_lock<RWMutex> lock(lock_);
     auto* t = FindOrNull(transition_in_progress_, tablet_id);
     if (t) {
       transition = *t;
@@ -539,7 +539,7 @@ void TSTabletManager::RunTabletCopy(
   bool replacing_tablet = false;
   scoped_refptr<TransitionInProgressDeleter> deleter;
   {
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     if (LookupTabletUnlocked(tablet_id, &old_replica)) {
       meta = old_replica->tablet_metadata();
       replacing_tablet = true;
@@ -757,7 +757,7 @@ Status TSTabletManager::DeleteTablet(
   {
     // Acquire the lock in exclusive mode as we'll add a entry to the
     // transition_in_progress_ map.
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     TRACE("Acquired tablet manager lock");
     RETURN_NOT_OK(CheckRunningUnlocked(error_code));
 
@@ -833,7 +833,7 @@ Status TSTabletManager::DeleteTablet(
   // Only DELETED tablets are fully shut down and removed from the tablet map.
   if (delete_type == TABLET_DATA_DELETED) {
     replica->Shutdown();
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     RETURN_NOT_OK(CheckRunningUnlocked(error_code));
     CHECK_EQ(1, tablet_map_.erase(tablet_id)) << tablet_id;
     InsertOrDie(&perm_deleted_tablet_ids_, tablet_id);
@@ -863,7 +863,7 @@ Status TSTabletManager::StartTabletStateTransitionUnlocked(
     const string& tablet_id,
     const string& reason,
     scoped_refptr<TransitionInProgressDeleter>* deleter) {
-  DCHECK(lock_.is_write_locked());
+  lock_.AssertAcquiredForWriting();
   if (ContainsKey(perm_deleted_tablet_ids_, tablet_id)) {
     // When a table is deleted, the master sends a DeleteTablet() RPC to every
     // replica of every tablet with the TABLET_DATA_DELETED parameter, which
@@ -1000,7 +1000,7 @@ void TSTabletManager::OpenTablet(const scoped_refptr<TabletReplica>&
replica,
 
 void TSTabletManager::Shutdown() {
   {
-    std::lock_guard<rw_spinlock> lock(lock_);
+    std::lock_guard<RWMutex> lock(lock_);
     switch (state_) {
       case MANAGER_QUIESCING: {
         VLOG(1) << "Tablet manager shut down already in progress..";
@@ -1040,7 +1040,7 @@ void TSTabletManager::Shutdown() {
   }
 
   {
-    std::lock_guard<rw_spinlock> l(lock_);
+    std::lock_guard<RWMutex> l(lock_);
     // We don't expect anyone else to be modifying the map after we start the
     // shut down process.
     CHECK_EQ(tablet_map_.size(), replicas_to_shutdown.size())
@@ -1054,7 +1054,7 @@ void TSTabletManager::Shutdown() {
 void TSTabletManager::RegisterTablet(const std::string& tablet_id,
                                      const scoped_refptr<TabletReplica>& replica,
                                      RegisterTabletReplicaMode mode) {
-  std::lock_guard<rw_spinlock> lock(lock_);
+  std::lock_guard<RWMutex> lock(lock_);
   // If we are replacing a tablet replica, we delete the existing one first.
   if (mode == REPLACEMENT_REPLICA && tablet_map_.erase(tablet_id) != 1) {
     LOG(FATAL) << "Unable to remove previous tablet replica " << tablet_id <<
": not registered!";
@@ -1070,7 +1070,7 @@ void TSTabletManager::RegisterTablet(const std::string& tablet_id,
 
 bool TSTabletManager::LookupTablet(const string& tablet_id,
                                    scoped_refptr<TabletReplica>* replica) const {
-  shared_lock<rw_spinlock> l(lock_);
+  shared_lock<RWMutex> l(lock_);
   return LookupTabletUnlocked(tablet_id, replica);
 }
 
@@ -1097,7 +1097,7 @@ const NodeInstancePB& TSTabletManager::NodeInstance() const {
 }
 
 void TSTabletManager::GetTabletReplicas(vector<scoped_refptr<TabletReplica> >*
replicas) const {
-  shared_lock<rw_spinlock> l(lock_);
+  shared_lock<RWMutex> l(lock_);
   AppendValuesFromMap(tablet_map_, replicas);
 }
 
@@ -1111,7 +1111,7 @@ void TSTabletManager::MarkTabletDirty(const std::string& tablet_id,
const std::s
 
 int TSTabletManager::GetNumLiveTablets() const {
   int count = 0;
-  shared_lock<rw_spinlock> l(lock_);
+  shared_lock<RWMutex> l(lock_);
   for (const auto& entry : tablet_map_) {
     tablet::TabletStatePB state = entry.second->state();
     if (state == tablet::BOOTSTRAPPING ||
@@ -1151,7 +1151,7 @@ void TSTabletManager::CreateReportedTabletPB(const string& tablet_id,
 }
 
 void TSTabletManager::PopulateFullTabletReport(TabletReportPB* report) const {
-  shared_lock<rw_spinlock> shared_lock(lock_);
+  shared_lock<RWMutex> shared_lock(lock_);
   for (const auto& e : tablet_map_) {
     CreateReportedTabletPB(e.first, e.second, report->add_updated_tablets());
   }
@@ -1159,7 +1159,7 @@ void TSTabletManager::PopulateFullTabletReport(TabletReportPB* report)
const {
 
 void TSTabletManager::PopulateIncrementalTabletReport(TabletReportPB* report,
                                                       const vector<string>& tablet_ids)
const {
-  shared_lock<rw_spinlock> shared_lock(lock_);
+  shared_lock<RWMutex> shared_lock(lock_);
   for (const auto& id : tablet_ids) {
     const scoped_refptr<tablet::TabletReplica>* replica =
         FindOrNull(tablet_map_, id);
@@ -1291,7 +1291,7 @@ void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
 
 int TSTabletManager::RefreshTabletStateCacheAndReturnCount(tablet::TabletStatePB st) {
   MonoDelta period = MonoDelta::FromMilliseconds(FLAGS_tablet_state_walk_min_period_ms);
-  std::lock_guard<rw_spinlock> lock(lock_);
+  std::lock_guard<RWMutex> lock(lock_);
   if (last_walked_ + period < MonoTime::Now()) {
     // Old cache: regenerate counts.
     tablet_state_counts_.clear();
@@ -1304,11 +1304,11 @@ int TSTabletManager::RefreshTabletStateCacheAndReturnCount(tablet::TabletStatePB
 }
 
 TransitionInProgressDeleter::TransitionInProgressDeleter(
-    TransitionInProgressMap* map, rw_spinlock* lock, string entry)
+    TransitionInProgressMap* map, RWMutex* lock, string entry)
     : in_progress_(map), lock_(lock), entry_(std::move(entry)) {}
 
 TransitionInProgressDeleter::~TransitionInProgressDeleter() {
-  std::lock_guard<rw_spinlock> lock(*lock_);
+  std::lock_guard<RWMutex> lock(*lock_);
   CHECK(in_progress_->erase(entry_));
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/94bf6996/src/kudu/tserver/ts_tablet_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.h b/src/kudu/tserver/ts_tablet_manager.h
index 1b5f24d..6c9cd28 100644
--- a/src/kudu/tserver/ts_tablet_manager.h
+++ b/src/kudu/tserver/ts_tablet_manager.h
@@ -39,6 +39,7 @@
 #include "kudu/util/locks.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/monotime.h"
+#include "kudu/util/rw_mutex.h"
 #include "kudu/util/status.h"
 
 namespace boost {
@@ -292,7 +293,7 @@ class TSTabletManager : public tserver::TabletReplicaLookupIf {
                                  int64_t last_logged_term);
 
   TSTabletManagerStatePB state() const {
-    shared_lock<rw_spinlock> l(lock_);
+    shared_lock<RWMutex> l(lock_);
     return state_;
   }
 
@@ -315,7 +316,7 @@ class TSTabletManager : public tserver::TabletReplicaLookupIf {
   // Lock protecting tablet_map_, dirty_tablets_, state_,
   // transition_in_progress_, perm_deleted_tablet_ids_,
   // tablet_state_counts_, and last_walked_.
-  mutable rw_spinlock lock_;
+  mutable RWMutex lock_;
 
   // Map from tablet ID to tablet
   TabletMap tablet_map_;
@@ -357,7 +358,7 @@ class TSTabletManager : public tserver::TabletReplicaLookupIf {
 // when tablet bootstrap, create, and delete operations complete.
 class TransitionInProgressDeleter : public RefCountedThreadSafe<TransitionInProgressDeleter>
{
  public:
-  TransitionInProgressDeleter(TransitionInProgressMap* map, rw_spinlock* lock,
+  TransitionInProgressDeleter(TransitionInProgressMap* map, RWMutex* lock,
                               std::string entry);
 
  private:
@@ -365,7 +366,7 @@ class TransitionInProgressDeleter : public RefCountedThreadSafe<TransitionInProg
   ~TransitionInProgressDeleter();
 
   TransitionInProgressMap* const in_progress_;
-  rw_spinlock* const lock_;
+  RWMutex* const lock_;
   const std::string entry_;
 };
 


Mime
View raw message