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 2016/05/12 18:49:58 UTC
[2/2] incubator-kudu git commit: KUDU-1450. Avoid contention during
LogBlockManager startup
KUDU-1450. Avoid contention during LogBlockManager startup
This defers the tracking of block memory during LBM startup until
after each container is fully parsed and added to the block map.
In testing on a machine with a couple TB of data, this reduced
the LBM startup time from about 30 seconds down to about 10.
Change-Id: Ie680389e5e08dff89862ef32b652a9cc73f458f5
Reviewed-on: http://gerrit.cloudera.org:8080/3042
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/2c0ac84e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/2c0ac84e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/2c0ac84e
Branch: refs/heads/master
Commit: 2c0ac84e1de6c2df2768eb664cdb1f7f43e74119
Parents: 7d57b8b
Author: Todd Lipcon <to...@apache.org>
Authored: Wed May 11 19:26:39 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu May 12 18:48:34 2016 +0000
----------------------------------------------------------------------
src/kudu/fs/log_block_manager.cc | 47 +++++++++++++++++------------------
src/kudu/fs/log_block_manager.h | 3 ---
2 files changed, 23 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/2c0ac84e/src/kudu/fs/log_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index 2744ed1..2036c94 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -218,10 +218,6 @@ class LogBlockContainer {
// beginning from the position where the last written block ended.
Status Preallocate(size_t length);
- // Manipulates the block manager's memory tracker on behalf of blocks.
- void ConsumeMemory(int64_t bytes);
- void ReleaseMemory(int64_t bytes);
-
// Reads the container's metadata from disk, sanity checking and
// returning the records.
Status ReadContainerRecords(deque<BlockRecordPB>* records) const;
@@ -566,14 +562,6 @@ Status LogBlockContainer::Preallocate(size_t length) {
return data_file_->PreAllocate(total_bytes_written(), length);
}
-void LogBlockContainer::ConsumeMemory(int64_t bytes) {
- block_manager()->mem_tracker_->Consume(bytes);
-}
-
-void LogBlockContainer::ReleaseMemory(int64_t bytes) {
- block_manager()->mem_tracker_->Release(bytes);
-}
-
void LogBlockContainer::UpdateBytesWritten(int64_t more_bytes) {
DCHECK_GE(more_bytes, 0);
@@ -660,8 +648,6 @@ LogBlock::LogBlock(LogBlockContainer* container, BlockId block_id,
deleted_(false) {
DCHECK_GE(offset, 0);
DCHECK_GE(length, 0);
-
- container_->ConsumeMemory(kudu_malloc_usable_size(this));
}
static void DeleteBlockAsync(LogBlockContainer* container,
@@ -681,7 +667,6 @@ LogBlock::~LogBlock() {
container_->ExecClosure(Bind(&DeleteBlockAsync, container_, block_id_,
offset_, length_));
}
- container_->ReleaseMemory(kudu_malloc_usable_size(this));
}
void LogBlock::Delete() {
@@ -1062,6 +1047,13 @@ LogBlockManager::LogBlockManager(Env* env, const BlockManagerOptions& opts)
}
LogBlockManager::~LogBlockManager() {
+ // Release all of the memory accounted by the blocks.
+ int64_t mem = 0;
+ for (const auto& entry : blocks_by_block_id_) {
+ mem += kudu_malloc_usable_size(entry.second.get());
+ }
+ mem_tracker_->Release(mem);
+
// A LogBlock's destructor depends on its container, so all LogBlocks must be
// destroyed before their containers.
blocks_by_block_id_.clear();
@@ -1394,6 +1386,8 @@ bool LogBlockManager::AddLogBlock(LogBlockContainer* container,
int64_t length) {
lock_guard<simple_spinlock> l(&lock_);
scoped_refptr<LogBlock> lb(new LogBlock(container, block_id, offset, length));
+ mem_tracker_->Consume(kudu_malloc_usable_size(lb.get()));
+
return AddLogBlockUnlocked(lb);
}
@@ -1416,18 +1410,17 @@ bool LogBlockManager::AddLogBlockUnlocked(const scoped_refptr<LogBlock>& lb) {
scoped_refptr<LogBlock> LogBlockManager::RemoveLogBlock(const BlockId& block_id) {
lock_guard<simple_spinlock> l(&lock_);
- return RemoveLogBlockUnlocked(block_id);
-}
-
-scoped_refptr<LogBlock> LogBlockManager::RemoveLogBlockUnlocked(const BlockId& block_id) {
- DCHECK(lock_.is_locked());
-
scoped_refptr<LogBlock> result =
EraseKeyReturnValuePtr(&blocks_by_block_id_, block_id);
- if (result && metrics()) {
- metrics()->blocks_under_management->Decrement();
- metrics()->bytes_under_management->DecrementBy(result->length());
+ if (result) {
+ mem_tracker_->Release(kudu_malloc_usable_size(result.get()));
+
+ if (metrics()) {
+ metrics()->blocks_under_management->Decrement();
+ metrics()->bytes_under_management->DecrementBy(result->length());
+ }
}
+
return result;
}
@@ -1517,14 +1510,20 @@ void LogBlockManager::OpenRootPath(const string& root_path,
// the container.
{
lock_guard<simple_spinlock> l(&lock_);
+ // To avoid cacheline contention during startup, we aggregate all of the
+ // memory in a local and add it to the mem-tracker in a single increment
+ // at the end of this loop.
+ int64_t mem_usage = 0;
for (const UntrackedBlockMap::value_type& e : blocks_in_container) {
if (!AddLogBlockUnlocked(e.second)) {
LOG(FATAL) << "Found duplicate CREATE record for block " << e.first
<< " which already is alive from another container when "
<< " processing container " << container->ToString();
}
+ mem_usage += kudu_malloc_usable_size(e.second.get());
}
+ mem_tracker_->Consume(mem_usage);
AddNewContainerUnlocked(container.get());
MakeContainerAvailableUnlocked(container.release());
}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/2c0ac84e/src/kudu/fs/log_block_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager.h b/src/kudu/fs/log_block_manager.h
index 8cadb68..74d529f 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -244,9 +244,6 @@ class LogBlockManager : public BlockManager {
// already gone.
scoped_refptr<internal::LogBlock> RemoveLogBlock(const BlockId& block_id);
- // Unlocked variant of RemoveLogBlock(); must hold 'lock_'.
- scoped_refptr<internal::LogBlock> RemoveLogBlockUnlocked(const BlockId& block_id);
-
// Parse a block record, adding or removing it in 'block_map', and
// accounting for it in the metadata for 'container'.
void ProcessBlockRecord(const BlockRecordPB& record,