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 2017/11/01 05:00:19 UTC
[2/3] kudu git commit: KUDU-2193 (part 1): switch to a waiting mutex
in TSTabletManager
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 <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/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 <to...@apache.org>
Authored: Thu Oct 19 16:46:36 2017 -0700
Committer: Todd Lipcon <to...@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_;
};