kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: fs: add BlockManager::GetAllBlockIds()
Date Thu, 16 Mar 2017 00:02:22 GMT
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 <adar@cloudera.com>
Reviewed-by: David Ribeiro Alves <dralves@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 <adar@cloudera.com>
Authored: Sat Mar 11 01:26:16 2017 -0800
Committer: Adar Dembo <adar@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 {


Mime
View raw message