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/03/24 23:12:20 UTC
[2/2] kudu git commit: diskrowset: stop using percpu_rwlock
diskrowset: stop using percpu_rwlock
In a heap analysis of a server with 34K rowsets, I see percpu_rwlocks
inside DiskRowSet taking ~105MB of memory. Each percpu_rwlock is using 48
cache lines (3072 bytes) so the multiplication works out to around the
same value as I see in practice.
Although switching to a normal rwlock might increase contention
marginally, it's probably worth it for the 100MB memory win. If we start
to see contention here in a workload we can figure out other ways to
avoid taking the locks, but my guess is that the improved cache
locality of the lock being stored inline in the DRS class will outweigh
any contention-related issue.
Change-Id: I9e210eb3e8fc13d807f6630e68c68d64902b1cc4
Reviewed-on: http://gerrit.cloudera.org:8080/6472
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/286de539
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/286de539
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/286de539
Branch: refs/heads/master
Commit: 286de539213602f502f0a25fddde27f97ab9665b
Parents: c3953ad
Author: Todd Lipcon <to...@apache.org>
Authored: Fri Mar 24 12:56:18 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Mar 24 23:12:03 2017 +0000
----------------------------------------------------------------------
src/kudu/tablet/diskrowset.cc | 20 ++++++++++----------
src/kudu/tablet/diskrowset.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/286de539/src/kudu/tablet/diskrowset.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc
index c9646e8..fa72927 100644
--- a/src/kudu/tablet/diskrowset.cc
+++ b/src/kudu/tablet/diskrowset.cc
@@ -551,7 +551,7 @@ Status DiskRowSet::MajorCompactDeltaStoresWithColumnIds(const vector<ColumnId>&
mem_trackers_.tablet_tracker,
&new_base));
{
- std::lock_guard<percpu_rwlock> lock(component_lock_);
+ std::lock_guard<rw_spinlock> lock(component_lock_);
CHECK_OK(compaction->UpdateDeltaTracker(delta_tracker_.get()));
base_data_.swap(new_base);
}
@@ -566,7 +566,7 @@ Status DiskRowSet::NewMajorDeltaCompaction(const vector<ColumnId>& col_ids,
HistoryGcOpts history_gc_opts,
gscoped_ptr<MajorDeltaCompaction>* out) const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
const Schema* schema = &rowset_metadata_->tablet_schema();
@@ -594,7 +594,7 @@ Status DiskRowSet::NewRowIterator(const Schema *projection,
OrderMode /*order*/,
gscoped_ptr<RowwiseIterator>* out) const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
shared_ptr<CFileSet::Iterator> base_iter(base_data_->NewIterator(projection));
gscoped_ptr<ColumnwiseIterator> col_iter;
@@ -618,7 +618,7 @@ Status DiskRowSet::MutateRow(Timestamp timestamp,
ProbeStats* stats,
OperationResultPB* result) {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
rowid_t row_idx;
RETURN_NOT_OK(base_data_->FindRow(probe, &row_idx, stats));
@@ -640,7 +640,7 @@ Status DiskRowSet::CheckRowPresent(const RowSetKeyProbe &probe,
bool* present,
ProbeStats* stats) const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
rowid_t row_idx;
RETURN_NOT_OK(base_data_->CheckRowPresent(probe, present, &row_idx, stats));
@@ -658,7 +658,7 @@ Status DiskRowSet::CheckRowPresent(const RowSetKeyProbe &probe,
Status DiskRowSet::CountRows(rowid_t *count) const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
return base_data_->CountRows(count);
}
@@ -666,25 +666,25 @@ Status DiskRowSet::CountRows(rowid_t *count) const {
Status DiskRowSet::GetBounds(std::string* min_encoded_key,
std::string* max_encoded_key) const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
return base_data_->GetBounds(min_encoded_key, max_encoded_key);
}
uint64_t DiskRowSet::EstimateBaseDataDiskSize() const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
return base_data_->EstimateOnDiskSize();
}
uint64_t DiskRowSet::EstimateDeltaDiskSize() const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
return delta_tracker_->EstimateOnDiskSize();
}
uint64_t DiskRowSet::EstimateOnDiskSize() const {
DCHECK(open_);
- shared_lock<rw_spinlock> l(component_lock_.get_lock());
+ shared_lock<rw_spinlock> l(component_lock_);
return base_data_->EstimateOnDiskSize() + delta_tracker_->EstimateOnDiskSize();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/286de539/src/kudu/tablet/diskrowset.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/diskrowset.h b/src/kudu/tablet/diskrowset.h
index 8d0313b..be14c7d 100644
--- a/src/kudu/tablet/diskrowset.h
+++ b/src/kudu/tablet/diskrowset.h
@@ -420,7 +420,7 @@ class DiskRowSet : public RowSet {
TabletMemTrackers mem_trackers_;
// Base data for this rowset.
- mutable percpu_rwlock component_lock_;
+ mutable rw_spinlock component_lock_;
std::shared_ptr<CFileSet> base_data_;
gscoped_ptr<DeltaTracker> delta_tracker_;