You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/03/16 00:02:22 UTC
kudu git commit: fs: add BlockManager::GetAllBlockIds()
Repository: kudu
Updated Branches:
refs/heads/master f4caa6371 -> b42e9e519
fs: add BlockManager::GetAllBlockIds()
This method will be used in a poor man's data block GC. It's simple enough
to implement in the LBM, where all blocks are known, but more complicated in
the FBM.
Change-Id: I20e8ccf6e8a2deba88fcf5598fb404a1186b8262
Reviewed-on: http://gerrit.cloudera.org:8080/6360
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/b42e9e51
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b42e9e51
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b42e9e51
Branch: refs/heads/master
Commit: b42e9e519e8411f3be7d121c5976d45c27f438a6
Parents: f4caa63
Author: Adar Dembo <ad...@cloudera.com>
Authored: Sat Mar 11 01:26:16 2017 -0800
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Thu Mar 16 00:02:02 2017 +0000
----------------------------------------------------------------------
src/kudu/fs/block_id.h | 3 +
src/kudu/fs/block_manager-stress-test.cc | 2 +-
src/kudu/fs/block_manager-test.cc | 53 ++++++++++++++--
src/kudu/fs/block_manager.h | 11 +++-
src/kudu/fs/file_block_manager.cc | 74 ++++++++++++++++++++++
src/kudu/fs/file_block_manager.h | 2 +
src/kudu/fs/log_block_manager.cc | 6 +-
src/kudu/fs/log_block_manager.h | 5 +-
src/kudu/tablet/compaction-test.cc | 13 +++-
src/kudu/tablet/rowset_metadata.cc | 4 +-
src/kudu/tablet/tablet_metadata.h | 2 +-
src/kudu/tserver/tablet_copy_source_session.h | 6 +-
src/kudu/util/env.h | 2 +-
13 files changed, 161 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/block_id.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_id.h b/src/kudu/fs/block_id.h
index c0d3601..847ab4c 100644
--- a/src/kudu/fs/block_id.h
+++ b/src/kudu/fs/block_id.h
@@ -19,6 +19,7 @@
#include <iosfwd>
#include <string>
+#include <unordered_set>
#include <vector>
#include <glog/logging.h>
@@ -101,5 +102,7 @@ struct BlockIdEqual {
}
};
+typedef std::unordered_set<BlockId, BlockIdHash, BlockIdEqual> BlockIdSet;
+
} // namespace kudu
#endif /* KUDU_FS_BLOCK_ID_H */
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/block_manager-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-stress-test.cc b/src/kudu/fs/block_manager-stress-test.cc
index d9d1ee7..70827e5 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -191,7 +191,7 @@ class BlockManagerStressTest : public KuduTest {
//
// Each entry is a block id and the number of in-progress openers. To delete
// a block, there must be no openers.
- unordered_map<BlockId, int, BlockIdHash> written_blocks_;
+ unordered_map<BlockId, int, BlockIdHash, BlockIdEqual> written_blocks_;
// Protects written_blocks_.
simple_spinlock lock_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/block_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 600ce05..c726127 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+#include <algorithm>
#include <memory>
#include <unordered_map>
#include <unordered_set>
@@ -856,7 +857,9 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
created_blocks.push_back(last_block_id);
ASSERT_OK(writer->Close());
}
- ASSERT_EQ(4, bm_->CountBlocksForTests());
+ vector<BlockId> block_ids;
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(4, block_ids.size());
gscoped_ptr<ReadableBlock> block;
ASSERT_OK(bm_->OpenBlock(last_block_id, &block));
ASSERT_OK(block->Close());
@@ -892,7 +895,8 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
shared_ptr<MemTracker>(),
{ this->test_dir_ },
false));
- ASSERT_EQ(4, bm_->CountBlocksForTests());
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(4, block_ids.size());
ASSERT_OK(bm_->OpenBlock(last_block_id, &block));
ASSERT_OK(block->Close());
@@ -904,7 +908,8 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
// metadata file of the originally-written container, since we append a
// delete record to the metadata.
ASSERT_OK(bm_->DeleteBlock(created_blocks[0]));
- ASSERT_EQ(3, bm_->CountBlocksForTests());
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(3, block_ids.size());
ASSERT_OK(env_->GetFileSize(metadata_path, &cur_meta_size));
good_meta_size = cur_meta_size;
@@ -917,7 +922,8 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
created_blocks.push_back(last_block_id);
ASSERT_OK(writer->Close());
}
- ASSERT_EQ(4, bm_->CountBlocksForTests());
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(4, block_ids.size());
ASSERT_OK(env_->GetFileSize(metadata_path, &cur_meta_size));
ASSERT_GT(cur_meta_size, good_meta_size);
uint64_t prev_good_meta_size = good_meta_size; // Store previous size.
@@ -947,7 +953,8 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
ASSERT_OK(env_->GetFileSize(metadata_path, &cur_meta_size));
ASSERT_EQ(good_meta_size, cur_meta_size);
- ASSERT_EQ(3, bm_->CountBlocksForTests());
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(3, block_ids.size());
Status s = bm_->OpenBlock(last_block_id, &block);
ASSERT_TRUE(s.IsNotFound()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "Can't find block");
@@ -961,7 +968,8 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
ASSERT_OK(writer->Close());
}
- ASSERT_EQ(4, bm_->CountBlocksForTests());
+ ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
+ ASSERT_EQ(4, block_ids.size());
ASSERT_OK(bm_->OpenBlock(last_block_id, &block));
ASSERT_OK(block->Close());
@@ -1217,7 +1225,7 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailedWrites) {
// 2. Try to delete every other block.
// 3. Read and test every block.
// 4. Restart the block manager, forcing the on-disk metadata to be reloaded.
- unordered_set<BlockId, BlockIdHash> ids;
+ BlockIdSet ids;
for (int attempt = 0; attempt < kNumTries; attempt++) {
int num_created = 0;
for (int i = 0; i < kNumBlockTries; i++) {
@@ -1263,6 +1271,37 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailedWrites) {
}
}
+TYPED_TEST(BlockManagerTest, TestGetAllBlockIds) {
+ vector<BlockId> ids;
+ for (int i = 0; i < 100; i++) {
+ gscoped_ptr<WritableBlock> block;
+ ASSERT_OK(this->bm_->CreateBlock(&block));
+ ASSERT_OK(block->Close());
+ ids.push_back(block->id());
+ }
+
+ // The file block manager should skip these; they shouldn't appear in
+ // 'retrieved_ids' below.
+ for (const auto& s : { string("abcde"), // not numeric
+ string("12345"), // not a real block ID
+ ids.begin()->ToString() }) { // not in a block directory
+ unique_ptr<WritableFile> writer;
+ ASSERT_OK(this->env_->NewWritableFile(
+ JoinPathSegments(this->test_dir_, s), &writer));
+ ASSERT_OK(writer->Close());
+ }
+
+ vector<BlockId> retrieved_ids;
+ ASSERT_OK(this->bm_->GetAllBlockIds(&retrieved_ids));
+
+ // Sort the two collections before the comparison as GetAllBlockIds() does
+ // not guarantee a deterministic order.
+ std::sort(ids.begin(), ids.end(), BlockIdCompare());
+ std::sort(retrieved_ids.begin(), retrieved_ids.end(), BlockIdCompare());
+
+ ASSERT_EQ(ids, retrieved_ids);
+}
+
TEST_F(LogBlockManagerTest, TestContainerWithManyHoles) {
// This is a regression test of sorts for KUDU-1508, though it doesn't
// actually fail if the fix is missing; it just corrupts the filesystem.
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/block_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager.h b/src/kudu/fs/block_manager.h
index 06b09f5..b2cf1c5 100644
--- a/src/kudu/fs/block_manager.h
+++ b/src/kudu/fs/block_manager.h
@@ -19,8 +19,8 @@
#define KUDU_FS_BLOCK_MANAGER_H
#include <cstddef>
+#include <cstdint>
#include <memory>
-#include <stdint.h>
#include <string>
#include <vector>
@@ -238,6 +238,15 @@ class BlockManager {
//
// On success, guarantees that outstanding data is durable.
virtual Status CloseBlocks(const std::vector<WritableBlock*>& blocks) = 0;
+
+ // Retrieves the IDs of all blocks under management by this block manager.
+ // These include ReadableBlocks as well as WritableBlocks.
+ //
+ // Returned block IDs are not guaranteed to be in any particular order,
+ // nor is the order guaranteed to be deterministic. Furthermore, if
+ // concurrent operations are ongoing, some of the blocks themselves may not
+ // even exist after the call.
+ virtual Status GetAllBlockIds(std::vector<BlockId>* block_ids) = 0;
};
// Closes a group of blocks.
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/file_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/file_block_manager.cc b/src/kudu/fs/file_block_manager.cc
index 6d2003d..33086ce 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -23,6 +23,7 @@
#include "kudu/fs/block_manager_metrics.h"
#include "kudu/fs/data_dirs.h"
+#include "kudu/gutil/strings/numbers.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/atomic.h"
#include "kudu/util/env.h"
@@ -675,5 +676,78 @@ Status FileBlockManager::CloseBlocks(const vector<WritableBlock*>& blocks) {
return Status::OK();
}
+namespace {
+
+Status GetAllBlockIdsForDataDirCb(DataDir* dd,
+ vector<BlockId>* block_ids,
+ Env::FileType file_type,
+ const string& dirname,
+ const string& basename) {
+ if (file_type != Env::FILE_TYPE) {
+ // Skip directories.
+ return Status::OK();
+ }
+
+ uint64_t numeric_id;
+ if (!safe_strtou64(basename, &numeric_id)) {
+ // Skip files with non-numerical names.
+ return Status::OK();
+ }
+
+ // Verify that this block ID look-alike is, in fact, a block ID.
+ //
+ // We could also verify its contents, but that'd be quite expensive.
+ BlockId block_id(numeric_id);
+ internal::FileBlockLocation loc(
+ internal::FileBlockLocation::FromBlockId(dd, block_id));
+ if (loc.GetFullPath() != JoinPathSegments(dirname, basename)) {
+ return Status::OK();
+ }
+
+ block_ids->push_back(block_id);
+ return Status::OK();
+}
+
+void GetAllBlockIdsForDataDir(Env* env,
+ DataDir* dd,
+ vector<BlockId>* block_ids,
+ Status* status) {
+ *status = env->Walk(dd->dir(), Env::PRE_ORDER,
+ Bind(&GetAllBlockIdsForDataDirCb, dd, block_ids));
+}
+
+} // anonymous namespace
+
+Status FileBlockManager::GetAllBlockIds(vector<BlockId>* block_ids) {
+ const auto& dds = dd_manager_.data_dirs();
+ block_ids->clear();
+
+ // The FBM does not maintain block listings in memory, so off we go to the
+ // filesystem. The search is parallelized across data directories.
+ vector<vector<BlockId>> block_id_vecs(dds.size());
+ vector<Status> statuses(dds.size());
+ for (int i = 0; i < dds.size(); i++) {
+ dds[i]->ExecClosure(Bind(&GetAllBlockIdsForDataDir,
+ env_,
+ dds[i].get(),
+ &block_id_vecs[i],
+ &statuses[i]));
+ }
+ for (const auto& dd : dd_manager_.data_dirs()) {
+ dd->WaitOnClosures();
+ }
+
+ // A failure on any data directory is fatal.
+ for (const auto& s : statuses) {
+ RETURN_NOT_OK(s);
+ }
+
+ // Collect the results into 'blocks'.
+ for (const auto& ids : block_id_vecs) {
+ block_ids->insert(block_ids->begin(), ids.begin(), ids.end());
+ }
+ return Status::OK();
+}
+
} // namespace fs
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/fs/file_block_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/file_block_manager.h b/src/kudu/fs/file_block_manager.h
index 23806e1..f4581c2 100644
--- a/src/kudu/fs/file_block_manager.h
+++ b/src/kudu/fs/file_block_manager.h
@@ -94,6 +94,8 @@ class FileBlockManager : public BlockManager {
virtual Status CloseBlocks(const std::vector<WritableBlock*>& blocks) OVERRIDE;
+ virtual Status GetAllBlockIds(std::vector<BlockId>* block_ids) OVERRIDE;
+
private:
friend class internal::FileBlockLocation;
friend class internal::FileReadableBlock;
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/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 22eb08d..30da40b 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -1481,9 +1481,11 @@ Status LogBlockManager::CloseBlocks(const std::vector<WritableBlock*>& blocks) {
return Status::OK();
}
-int64_t LogBlockManager::CountBlocksForTests() const {
+Status LogBlockManager::GetAllBlockIds(vector<BlockId>* block_ids) {
std::lock_guard<simple_spinlock> l(lock_);
- return blocks_by_block_id_.size();
+ block_ids->assign(open_block_ids_.begin(), open_block_ids_.end());
+ AppendKeysFromMap(blocks_by_block_id_, block_ids);
+ return Status::OK();
}
void LogBlockManager::AddNewContainerUnlocked(LogBlockContainer* container) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/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 97501f3..1295871 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -180,8 +180,7 @@ class LogBlockManager : public BlockManager {
virtual Status CloseBlocks(const std::vector<WritableBlock*>& blocks) OVERRIDE;
- // Return the number of blocks stored in the block manager.
- int64_t CountBlocksForTests() const;
+ virtual Status GetAllBlockIds(std::vector<BlockId>* block_ids) OVERRIDE;
private:
FRIEND_TEST(LogBlockManagerTest, TestLookupBlockLimit);
@@ -317,7 +316,7 @@ class LogBlockManager : public BlockManager {
//
// Together with blocks_by_block_id's keys, used to prevent collisions
// when creating new anonymous blocks.
- std::unordered_set<BlockId, BlockIdHash> open_block_ids_;
+ BlockIdSet open_block_ids_;
// Holds (and owns) all containers loaded from disk.
std::vector<internal::LogBlockContainer*> all_containers_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/tablet/compaction-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/compaction-test.cc b/src/kudu/tablet/compaction-test.cc
index 55035d7..427a1cd 100644
--- a/src/kudu/tablet/compaction-test.cc
+++ b/src/kudu/tablet/compaction-test.cc
@@ -1077,11 +1077,18 @@ TEST_F(TestCompaction, TestEmptyFlushDoesntLeakBlocks) {
fs::LogBlockManager* lbm = down_cast<fs::LogBlockManager*>(
harness_->fs_manager()->block_manager());
- int64_t before_count = lbm->CountBlocksForTests();
+ vector<BlockId> before_block_ids;
+ ASSERT_OK(lbm->GetAllBlockIds(&before_block_ids));
ASSERT_OK(tablet()->Flush());
- int64_t after_count = lbm->CountBlocksForTests();
+ vector<BlockId> after_block_ids;
+ ASSERT_OK(lbm->GetAllBlockIds(&after_block_ids));
- ASSERT_EQ(after_count, before_count);
+ // Sort the two collections before the comparison as GetAllBlockIds() does
+ // not guarantee a deterministic order.
+ std::sort(before_block_ids.begin(), before_block_ids.end(), BlockIdCompare());
+ std::sort(after_block_ids.begin(), after_block_ids.end(), BlockIdCompare());
+
+ ASSERT_EQ(after_block_ids, before_block_ids);
}
} // namespace tablet
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/tablet/rowset_metadata.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/rowset_metadata.cc b/src/kudu/tablet/rowset_metadata.cc
index f0e80d0..6e10b53 100644
--- a/src/kudu/tablet/rowset_metadata.cc
+++ b/src/kudu/tablet/rowset_metadata.cc
@@ -191,8 +191,8 @@ Status RowSetMetadata::CommitUpdate(const RowSetMetadataUpdate& update) {
}
// Remove undo blocks.
- std::unordered_set<BlockId, BlockIdHash> undos_to_remove(update.remove_undo_blocks_.begin(),
- update.remove_undo_blocks_.end());
+ BlockIdSet undos_to_remove(update.remove_undo_blocks_.begin(),
+ update.remove_undo_blocks_.end());
int64_t num_removed = 0;
auto iter = undo_delta_blocks_.begin();
while (iter != undo_delta_blocks_.end()) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/tablet/tablet_metadata.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_metadata.h b/src/kudu/tablet/tablet_metadata.h
index 383bae4..7a03a2a 100644
--- a/src/kudu/tablet/tablet_metadata.h
+++ b/src/kudu/tablet/tablet_metadata.h
@@ -328,7 +328,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
std::vector<Schema*> old_schemas_;
// Protected by 'data_lock_'.
- std::unordered_set<BlockId, BlockIdHash, BlockIdEqual> orphaned_blocks_;
+ BlockIdSet orphaned_blocks_;
// The current state of tablet copy for the tablet.
TabletDataState tablet_data_state_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/tserver/tablet_copy_source_session.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_source_session.h b/src/kudu/tserver/tablet_copy_source_session.h
index aaf0043..d51a024 100644
--- a/src/kudu/tserver/tablet_copy_source_session.h
+++ b/src/kudu/tserver/tablet_copy_source_session.h
@@ -144,7 +144,11 @@ class TabletCopySourceSession : public RefCountedThreadSafe<TabletCopySourceSess
private:
friend class RefCountedThreadSafe<TabletCopySourceSession>;
- typedef std::unordered_map<BlockId, ImmutableReadableBlockInfo*, BlockIdHash> BlockMap;
+ typedef std::unordered_map<
+ BlockId,
+ ImmutableReadableBlockInfo*,
+ BlockIdHash,
+ BlockIdEqual> BlockMap;
typedef std::unordered_map<uint64_t, ImmutableRandomAccessFileInfo*> LogMap;
~TabletCopySourceSession();
http://git-wip-us.apache.org/repos/asf/kudu/blob/b42e9e51/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index bbf8f9c..20316d7 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -265,7 +265,7 @@ class Env {
//
// Returning an error won't halt the walk, but it will cause it to return
// with an error status when it's done.
- typedef Callback<Status(FileType,const std::string&, const std::string&)> WalkCallback;
+ typedef Callback<Status(FileType, const std::string&, const std::string&)> WalkCallback;
// Whether to walk directories in pre-order or post-order.
enum DirectoryOrder {