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:19 UTC
[1/2] kudu git commit: rowset_metadata: use flat_map for
column->block mapping
Repository: kudu
Updated Branches:
refs/heads/master e4495d662 -> 286de5392
rowset_metadata: use flat_map for column->block mapping
In a server with around 1T of data, the std::map<> here was taking
around 25MB of RAM.
Based on an experiment, a map<int,int> with 40 entries takes 1648 bytes
whereas a flat_map<int,int> (after shrink_to_fit) takes 344 bytes. So,
this should save around 20M of RAM from that server (and improve memory
locality while we're at it).
Change-Id: I1b7eaac3481672c69709009b6eacc2008027d4cd
Reviewed-on: http://gerrit.cloudera.org:8080/6470
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c3953ad0
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c3953ad0
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c3953ad0
Branch: refs/heads/master
Commit: c3953ad01e91bfd5b10d2851c7db5cdf424eec8e
Parents: e4495d6
Author: Todd Lipcon <to...@apache.org>
Authored: Fri Mar 24 12:12:52 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Mar 24 23:11:56 2017 +0000
----------------------------------------------------------------------
src/kudu/tablet/delta_compaction.cc | 3 ++-
src/kudu/tablet/diskrowset.cc | 11 +++++++----
src/kudu/tablet/rowset_metadata.cc | 8 ++++++--
src/kudu/tablet/rowset_metadata.h | 8 ++++++--
4 files changed, 21 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/c3953ad0/src/kudu/tablet/delta_compaction.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/delta_compaction.cc b/src/kudu/tablet/delta_compaction.cc
index a2b1700..5d1488d 100644
--- a/src/kudu/tablet/delta_compaction.cc
+++ b/src/kudu/tablet/delta_compaction.cc
@@ -18,6 +18,7 @@
#include "kudu/tablet/delta_compaction.h"
#include <algorithm>
+#include <map>
#include <string>
#include <vector>
@@ -306,7 +307,7 @@ Status MajorDeltaCompaction::CreateMetadataUpdate(
}
// Replace old column blocks with new ones
- RowSetMetadata::ColumnIdToBlockIdMap new_column_blocks;
+ std::map<ColumnId, BlockId> new_column_blocks;
base_data_writer_->GetFlushedBlocksByColumnId(&new_column_blocks);
// NOTE: in the case that one of the columns being compacted is deleted,
http://git-wip-us.apache.org/repos/asf/kudu/blob/c3953ad0/src/kudu/tablet/diskrowset.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc
index 69708f6..c9646e8 100644
--- a/src/kudu/tablet/diskrowset.cc
+++ b/src/kudu/tablet/diskrowset.cc
@@ -15,12 +15,16 @@
// specific language governing permissions and limitations
// under the License.
+#include "kudu/tablet/diskrowset.h"
+
#include <algorithm>
-#include <glog/logging.h>
-#include <glog/stl_logging.h>
+#include <map>
#include <mutex>
#include <vector>
+#include <glog/logging.h>
+#include <glog/stl_logging.h>
+
#include "kudu/cfile/bloomfile.h"
#include "kudu/cfile/cfile_writer.h"
#include "kudu/cfile/type_encodings.h"
@@ -34,7 +38,6 @@
#include "kudu/tablet/compaction.h"
#include "kudu/tablet/delta_compaction.h"
#include "kudu/tablet/delta_store.h"
-#include "kudu/tablet/diskrowset.h"
#include "kudu/tablet/multi_column_writer.h"
#include "kudu/util/debug/trace_event.h"
#include "kudu/util/flag_tags.h"
@@ -228,7 +231,7 @@ Status DiskRowSetWriter::FinishAndReleaseBlocks(ScopedWritableBlockCloser* close
RETURN_NOT_OK(col_writer_->FinishAndReleaseBlocks(closer));
// Put the column data blocks in the metadata.
- RowSetMetadata::ColumnIdToBlockIdMap flushed_blocks;
+ std::map<ColumnId, BlockId> flushed_blocks;
col_writer_->GetFlushedBlocksByColumnId(&flushed_blocks);
rowset_metadata_->SetColumnDataBlocks(flushed_blocks);
http://git-wip-us.apache.org/repos/asf/kudu/blob/c3953ad0/src/kudu/tablet/rowset_metadata.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/rowset_metadata.cc b/src/kudu/tablet/rowset_metadata.cc
index 6e10b53..dd853cb 100644
--- a/src/kudu/tablet/rowset_metadata.cc
+++ b/src/kudu/tablet/rowset_metadata.cc
@@ -138,9 +138,11 @@ const string RowSetMetadata::ToString() const {
return Substitute("RowSet($0)", id_);
}
-void RowSetMetadata::SetColumnDataBlocks(const ColumnIdToBlockIdMap& blocks) {
+void RowSetMetadata::SetColumnDataBlocks(const std::map<ColumnId, BlockId>& blocks_by_col_id) {
+ ColumnIdToBlockIdMap new_map(blocks_by_col_id.begin(), blocks_by_col_id.end());
+ new_map.shrink_to_fit();
std::lock_guard<LockType> l(lock_);
- blocks_by_col_id_ = blocks;
+ blocks_by_col_id_ = std::move(new_map);
}
Status RowSetMetadata::CommitRedoDeltaDataBlock(int64_t dms_id,
@@ -235,6 +237,8 @@ Status RowSetMetadata::CommitUpdate(const RowSetMetadataUpdate& update) {
}
}
+ blocks_by_col_id_.shrink_to_fit();
+
// Should only be NULL in tests.
if (tablet_metadata()) {
tablet_metadata()->AddOrphanedBlocks(removed);
http://git-wip-us.apache.org/repos/asf/kudu/blob/c3953ad0/src/kudu/tablet/rowset_metadata.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/rowset_metadata.h b/src/kudu/tablet/rowset_metadata.h
index ee664b7..5652380 100644
--- a/src/kudu/tablet/rowset_metadata.h
+++ b/src/kudu/tablet/rowset_metadata.h
@@ -22,6 +22,8 @@
#include <string>
#include <vector>
+#include <boost/container/flat_map.hpp>
+
#include "kudu/common/schema.h"
#include "kudu/fs/block_id.h"
#include "kudu/fs/fs_manager.h"
@@ -63,7 +65,9 @@ class TabletMetadata;
//
class RowSetMetadata {
public:
- typedef std::map<ColumnId, BlockId> ColumnIdToBlockIdMap;
+ // We use a flat_map to save memory, since there are lots of these metadata
+ // objects.
+ typedef boost::container::flat_map<ColumnId, BlockId> ColumnIdToBlockIdMap;
// Create a new RowSetMetadata
static Status CreateNew(TabletMetadata* tablet_metadata,
@@ -97,7 +101,7 @@ class RowSetMetadata {
adhoc_index_block_ = block_id;
}
- void SetColumnDataBlocks(const ColumnIdToBlockIdMap& blocks_by_col_id);
+ void SetColumnDataBlocks(const std::map<ColumnId, BlockId>& blocks_by_col_id);
Status CommitRedoDeltaDataBlock(int64_t dms_id, const BlockId& block_id);
[2/2] kudu git commit: diskrowset: stop using percpu_rwlock
Posted by to...@apache.org.
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_;