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 <[email protected]> Reviewed-by: David Ribeiro Alves <[email protected]> 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 <[email protected]> Authored: Sat Mar 11 01:26:16 2017 -0800 Committer: Adar Dembo <[email protected]> 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 {
