You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/10/12 18:27:01 UTC
kudu git commit: KUDU-2055 [part 4]: refactor BM to remove
BlockManager::DeleteBlock
Repository: kudu
Updated Branches:
refs/heads/master 9583cad6e -> f373af07d
KUDU-2055 [part 4]: refactor BM to remove BlockManager::DeleteBlock
This patch refactors block manager to use fs::BlockDeletionTransaction
instead of BlockManager::DeleteBlock for block deletions, in order to
simplify the interfaces of block manager and have a uniform path for
block deletions.
Change-Id: Ied2ab44f33bce29706f8f41acc3d468582edad6e
Reviewed-on: http://gerrit.cloudera.org:8080/8219
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f373af07
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f373af07
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f373af07
Branch: refs/heads/master
Commit: f373af07da4995872d37c6817f214bc39f9d8e4d
Parents: 9583cad
Author: hahao <ha...@cloudera.com>
Authored: Tue Oct 10 17:53:16 2017 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Oct 12 18:26:49 2017 +0000
----------------------------------------------------------------------
src/kudu/fs/block_manager-stress-test.cc | 9 ++-
src/kudu/fs/block_manager-test.cc | 32 ++++++++---
src/kudu/fs/block_manager.h | 8 ---
src/kudu/fs/file_block_manager.cc | 3 +-
src/kudu/fs/file_block_manager.h | 12 +++-
src/kudu/fs/fs_manager.cc | 6 --
src/kudu/fs/log_block_manager-test.cc | 60 +++++++++++++++-----
src/kudu/fs/log_block_manager.cc | 1 +
src/kudu/fs/log_block_manager.h | 13 ++++-
src/kudu/tablet/tablet_metadata.cc | 20 +++----
src/kudu/tools/kudu-tool-test.cc | 7 ++-
src/kudu/tools/tool_action_fs.cc | 23 ++++++--
.../tserver/tablet_copy_source_session-test.cc | 8 ++-
13 files changed, 140 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 53c10d7..06be125 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -88,6 +88,7 @@ DEFINE_string(block_manager_paths, "", "Comma-separated list of paths to "
"test path");
using std::string;
+using std::shared_ptr;
using std::unique_ptr;
using std::unordered_map;
using std::vector;
@@ -448,6 +449,8 @@ void BlockManagerStressTest<T>::DeleterThread() {
while (!ShouldStop(tight_loop)) {
// Grab a block at random.
BlockId to_delete;
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
{
std::unique_lock<simple_spinlock> l(lock_);
// If we only have a small number of live blocks, don't delete any.
@@ -472,8 +475,10 @@ void BlockManagerStressTest<T>::DeleterThread() {
}
// And delete it.
- CHECK_OK(bm_->DeleteBlock(to_delete));
- num_blocks_deleted++;
+ deletion_transaction->AddDeletedBlock(to_delete);
+ vector<BlockId> deleted;
+ CHECK_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
+ num_blocks_deleted += deleted.size();
}
total_blocks_deleted_.IncrementBy(num_blocks_deleted);
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 552a533..68a71df 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -40,6 +40,7 @@
#include "kudu/fs/fs.pb.h"
#include "kudu/fs/fs_report.h"
#include "kudu/fs/log_block_manager.h"
+#include "kudu/gutil/basictypes.h"
#include "kudu/gutil/bind.h"
#include "kudu/gutil/casts.h"
#include "kudu/gutil/gscoped_ptr.h"
@@ -456,7 +457,12 @@ TYPED_TEST(BlockManagerTest, EndToEndTest) {
LOG(INFO) << "Block memory footprint: " << read_block->memory_footprint();
// Delete the block.
- ASSERT_OK(this->bm_->DeleteBlock(written_block->id()));
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ deletion_transaction->AddDeletedBlock(written_block->id());
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
+ ASSERT_EQ(1, deleted.size());
ASSERT_TRUE(this->bm_->OpenBlock(written_block->id(), nullptr)
.IsNotFound());
}
@@ -495,7 +501,12 @@ TYPED_TEST(BlockManagerTest, ReadAfterDeleteTest) {
// Open it for reading, then delete it. Subsequent opens should fail.
unique_ptr<ReadableBlock> read_block;
ASSERT_OK(this->bm_->OpenBlock(written_block->id(), &read_block));
- ASSERT_OK(this->bm_->DeleteBlock(written_block->id()));
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ deletion_transaction->AddDeletedBlock(written_block->id());
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
+ ASSERT_EQ(1, deleted.size());
ASSERT_TRUE(this->bm_->OpenBlock(written_block->id(), nullptr)
.IsNotFound());
@@ -655,7 +666,12 @@ TYPED_TEST(BlockManagerTest, PersistenceTest) {
ASSERT_OK(this->bm_->CreateBlock(this->test_block_opts_, &written_block3));
ASSERT_OK(written_block3->Append(test_data));
ASSERT_OK(written_block3->Close());
- ASSERT_OK(this->bm_->DeleteBlock(written_block3->id()));
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ deletion_transaction->AddDeletedBlock(written_block3->id());
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
+ ASSERT_EQ(1, deleted.size());
// Reopen the block manager. This may read block metadata from disk.
//
@@ -940,15 +956,14 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailure) {
int num_deleted = 0;
int num_deleted_attempts = 0;
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
for (auto it = ids.begin(); it != ids.end();) {
// TODO(adar): the lbm removes a block from its block map even if the
// on-disk deletion fails. When that's fixed, update this code to
// erase() only if s.ok().
- Status s = this->bm_->DeleteBlock(*it);
+ deletion_transaction->AddDeletedBlock(*it);
it = ids.erase(it);
- if (s.ok()) {
- num_deleted++;
- }
num_deleted_attempts++;
// Skip every other block.
@@ -956,6 +971,9 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailure) {
it++;
}
}
+ vector<BlockId> deleted;
+ ignore_result(deletion_transaction->CommitDeletedBlocks(&deleted));
+ num_deleted += deleted.size();
LOG(INFO) << Substitute("Successfully deleted $0 blocks on $1 attempts",
num_deleted, num_deleted_attempts);
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/fs/block_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager.h b/src/kudu/fs/block_manager.h
index 641479e..cbe36e0 100644
--- a/src/kudu/fs/block_manager.h
+++ b/src/kudu/fs/block_manager.h
@@ -247,14 +247,6 @@ class BlockManager {
virtual Status OpenBlock(const BlockId& block_id,
std::unique_ptr<ReadableBlock>* block) = 0;
- // Deletes an existing block, allowing its space to be reclaimed by the
- // filesystem. The change is immediately made durable.
- //
- // Blocks may be deleted while they are open for reading or writing;
- // the actual deletion will take place after the last open reader or
- // writer is closed.
- virtual Status DeleteBlock(const BlockId& block_id) = 0;
-
// Constructs a block creation transaction to group a set of block creation
// operations and closes the registered blocks together.
virtual std::unique_ptr<BlockCreationTransaction> NewCreationTransaction() = 0;
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 b3e6dbe..6694b88 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -319,7 +319,7 @@ Status FileWritableBlock::Close() {
Status FileWritableBlock::Abort() {
RETURN_NOT_OK(Close(NO_SYNC));
- return block_manager()->DeleteBlock(id());
+ return block_manager_->DeleteBlock(id());
}
BlockManager* FileWritableBlock::block_manager() const {
@@ -612,6 +612,7 @@ void FileBlockDeletionTransaction::AddDeletedBlock(BlockId block) {
}
Status FileBlockDeletionTransaction::CommitDeletedBlocks(std::vector<BlockId>* deleted) {
+ deleted->clear();
Status first_failure;
for (BlockId block : deleted_blocks_) {
Status s = fbm_->DeleteBlock(block);
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 fd3535b..bc114a1 100644
--- a/src/kudu/fs/file_block_manager.h
+++ b/src/kudu/fs/file_block_manager.h
@@ -45,6 +45,7 @@ class FsErrorManager;
struct FsReport;
namespace internal {
+class FileBlockDeletionTransaction;
class FileBlockLocation;
class FileReadableBlock;
class FileWritableBlock;
@@ -86,8 +87,6 @@ class FileBlockManager : public BlockManager {
Status OpenBlock(const BlockId& block_id,
std::unique_ptr<ReadableBlock>* block) override;
- Status DeleteBlock(const BlockId& block_id) override;
-
std::unique_ptr<BlockCreationTransaction> NewCreationTransaction() override;
std::shared_ptr<BlockDeletionTransaction> NewDeletionTransaction() override;
@@ -97,10 +96,19 @@ class FileBlockManager : public BlockManager {
FsErrorManager* error_manager() override { return error_manager_; }
private:
+ friend class internal::FileBlockDeletionTransaction;
friend class internal::FileBlockLocation;
friend class internal::FileReadableBlock;
friend class internal::FileWritableBlock;
+ // Deletes an existing block, allowing its space to be reclaimed by the
+ // filesystem. The change is immediately made durable.
+ //
+ // Blocks may be deleted while they are open for reading or writing;
+ // the actual deletion will take place after the last open reader or
+ // writer is closed.
+ Status DeleteBlock(const BlockId& block_id);
+
// Synchronizes the metadata for a block with the given location.
Status SyncMetadata(const internal::FileBlockLocation& location);
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index a0c78c6..deeb52a 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -649,12 +649,6 @@ Status FsManager::OpenBlock(const BlockId& block_id, unique_ptr<ReadableBlock>*
return block_manager_->OpenBlock(block_id, block);
}
-Status FsManager::DeleteBlock(const BlockId& block_id) {
- CHECK(!read_only_);
-
- return block_manager_->DeleteBlock(block_id);
-}
-
bool FsManager::BlockExists(const BlockId& block_id) const {
unique_ptr<ReadableBlock> block;
return block_manager_->OpenBlock(block_id, &block).ok();
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/fs/log_block_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index b3f76f7..5dc5099 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -66,6 +66,7 @@
using kudu::pb_util::ReadablePBContainerFile;
using std::set;
using std::string;
+using std::shared_ptr;
using std::unique_ptr;
using std::unordered_map;
using std::unordered_set;
@@ -320,7 +321,11 @@ TEST_F(LogBlockManagerTest, MetricsTest) {
ASSERT_NO_FATAL_FAILURE(CheckLogMetrics(new_entity, 10 * 1024, 11, 10, 10));
// Delete a block. Its contents should no longer be under management.
- ASSERT_OK(bm_->DeleteBlock(saved_id));
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ bm_->NewDeletionTransaction();
+ deletion_transaction->AddDeletedBlock(saved_id);
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
ASSERT_NO_FATAL_FAILURE(CheckLogMetrics(new_entity, 9 * 1024, 10, 10, 10));
}
@@ -727,17 +732,22 @@ TEST_F(LogBlockManagerTest, TestContainerWithManyHoles) {
// in the container by forcing the filesystem to alternate every hole with
// a live extent.
LOG(INFO) << "Deleting every other block";
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
for (int i = 0; i < ids.size(); i += 2) {
- ASSERT_OK(bm_->DeleteBlock(ids[i]));
+ deletion_transaction->AddDeletedBlock(ids[i]);
}
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
// Delete all of the blocks belonging to the interior node. If KUDU-1508
// applies, this should corrupt the filesystem.
LOG(INFO) << Substitute("Deleting remaining blocks up to block number $0",
last_interior_node_block_number);
for (int i = 1; i < last_interior_node_block_number; i += 2) {
- ASSERT_OK(bm_->DeleteBlock(ids[i]));
+ deletion_transaction->AddDeletedBlock(ids[i]);
}
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
}
TEST_F(LogBlockManagerTest, TestParseKernelRelease) {
@@ -932,14 +942,18 @@ TEST_F(LogBlockManagerTest, TestMisalignedBlocksFuzz) {
ASSERT_EQ(container_name, mb.container);
}
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
// Delete about half of them, chosen randomly.
vector<BlockId> block_ids;
ASSERT_OK(bm_->GetAllBlockIds(&block_ids));
for (const auto& id : block_ids) {
if (rand() % 2) {
- ASSERT_OK(bm_->DeleteBlock(id));
+ deletion_transaction->AddDeletedBlock(id);
}
}
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
// Wait for the block manager to punch out all of the holes. It's easiest to
// do this by reopening it; shutdown will wait for outstanding hole punches.
@@ -1235,7 +1249,11 @@ TEST_F(LogBlockManagerTest, TestDeleteDeadContainersAtStartup) {
// Delete the one block and reopen it again. The container files should have
// been deleted.
- ASSERT_OK(bm_->DeleteBlock(block->id()));
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ deletion_transaction->AddDeletedBlock(block->id());
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
ASSERT_OK(ReopenBlockManager());
ASSERT_FALSE(env_->FileExists(data_file_name));
ASSERT_FALSE(env_->FileExists(metadata_file_name));
@@ -1316,15 +1334,21 @@ TEST_F(LogBlockManagerTest, TestDeleteFromContainerAfterMetadataCompaction) {
// Create many container with a bunch of blocks, half of which are deleted.
vector<BlockId> block_ids;
- for (int i = 0; i < 1000; i++) {
- unique_ptr<WritableBlock> block;
- ASSERT_OK(bm_->CreateBlock(test_block_opts_, &block));
- ASSERT_OK(block->Close());
- if (i % 2 == 1) {
- ASSERT_OK(bm_->DeleteBlock(block->id()));
- } else {
- block_ids.emplace_back(block->id());
+ {
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ for (int i = 0; i < 1000; i++) {
+ unique_ptr<WritableBlock> block;
+ ASSERT_OK(bm_->CreateBlock(test_block_opts_, &block));
+ ASSERT_OK(block->Close());
+ if (i % 2 == 1) {
+ deletion_transaction->AddDeletedBlock(block->id());
+ } else {
+ block_ids.emplace_back(block->id());
+ }
}
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
}
// Reopen the block manager. This will cause it to compact all of the metadata
@@ -1338,8 +1362,14 @@ TEST_F(LogBlockManagerTest, TestDeleteFromContainerAfterMetadataCompaction) {
// we have file_cache capacity, this will also generate a mix of cache hits,
// misses, and re-insertions.
std::random_shuffle(block_ids.begin(), block_ids.end());
- for (const BlockId& b : block_ids) {
- ASSERT_OK(bm_->DeleteBlock(b));
+ {
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ this->bm_->NewDeletionTransaction();
+ for (const BlockId &b : block_ids) {
+ deletion_transaction->AddDeletedBlock(b);
+ }
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
}
// Reopen to make sure that the metadata can be properly loaded and
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 dcb0e32..5f3d365 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -1208,6 +1208,7 @@ void LogBlockDeletionTransaction::AddDeletedBlock(BlockId block) {
}
Status LogBlockDeletionTransaction::CommitDeletedBlocks(std::vector<BlockId>* deleted) {
+ deleted->clear();
Status first_failure;
for (BlockId block : deleted_blocks_) {
Status s = lbm_->DeleteBlock(block);
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/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 ea4d4ae..4b0f7b2 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -57,6 +57,7 @@ struct FsReport;
namespace internal {
class LogBlock;
class LogBlockContainer;
+class LogBlockDeletionTransaction;
class LogWritableBlock;
struct LogBlockManagerMetrics;
@@ -178,8 +179,6 @@ class LogBlockManager : public BlockManager {
Status OpenBlock(const BlockId& block_id,
std::unique_ptr<ReadableBlock>* block) override;
- Status DeleteBlock(const BlockId& block_id) override;
-
std::unique_ptr<BlockCreationTransaction> NewCreationTransaction() override;
std::shared_ptr<BlockDeletionTransaction> NewDeletionTransaction() override;
@@ -191,6 +190,7 @@ class LogBlockManager : public BlockManager {
private:
FRIEND_TEST(LogBlockManagerTest, TestAbortBlock);
FRIEND_TEST(LogBlockManagerTest, TestCloseFinalizedBlock);
+ FRIEND_TEST(LogBlockManagerTest, TestCompactFullContainerMetadataAtStartup);
FRIEND_TEST(LogBlockManagerTest, TestFinalizeBlock);
FRIEND_TEST(LogBlockManagerTest, TestLIFOContainerSelection);
FRIEND_TEST(LogBlockManagerTest, TestLookupBlockLimit);
@@ -199,6 +199,7 @@ class LogBlockManager : public BlockManager {
FRIEND_TEST(LogBlockManagerTest, TestReuseBlockIds);
friend class internal::LogBlockContainer;
+ friend class internal::LogBlockDeletionTransaction;
friend class internal::LogWritableBlock;
// Type for the actual block map used to store all live blocks.
@@ -237,6 +238,14 @@ class LogBlockManager : public BlockManager {
std::string,
std::vector<BlockRecordPB>> BlockRecordsByContainerMap;
+ // Deletes an existing block, allowing its space to be reclaimed by the
+ // filesystem. The change is immediately made durable.
+ //
+ // Blocks may be deleted while they are open for reading or writing;
+ // the actual deletion will take place after the last open reader or
+ // writer is closed.
+ Status DeleteBlock(const BlockId& block_id);
+
// Adds an as of yet unseen container to this block manager.
//
// Must be called with 'lock_' held.
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/tablet/tablet_metadata.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_metadata.cc b/src/kudu/tablet/tablet_metadata.cc
index ff1904a..873eb10 100644
--- a/src/kudu/tablet/tablet_metadata.cc
+++ b/src/kudu/tablet/tablet_metadata.cc
@@ -31,6 +31,7 @@
#include "kudu/consensus/opid.pb.h"
#include "kudu/consensus/opid_util.h"
#include "kudu/fs/block_id.h"
+#include "kudu/fs/block_manager.h"
#include "kudu/fs/data_dirs.h"
#include "kudu/fs/fs.pb.h"
#include "kudu/fs/fs_manager.h"
@@ -61,6 +62,8 @@ TAG_FLAG(enable_tablet_orphaned_block_deletion, runtime);
using base::subtle::Barrier_AtomicIncrement;
using kudu::consensus::MinimumOpId;
using kudu::consensus::OpId;
+using kudu::fs::BlockManager;
+using kudu::fs::BlockDeletionTransaction;
using kudu::pb_util::SecureDebugString;
using kudu::pb_util::SecureShortDebugString;
using std::memory_order_relaxed;
@@ -465,19 +468,14 @@ void TabletMetadata::DeleteOrphanedBlocks(const vector<BlockId>& blocks) {
return;
}
- vector<BlockId> deleted;
+ BlockManager* bm = fs_manager()->block_manager();
+ shared_ptr<BlockDeletionTransaction> transaction = bm->NewDeletionTransaction();
for (const BlockId& b : blocks) {
- Status s = fs_manager()->DeleteBlock(b);
- // If we get NotFound, then the block was actually successfully
- // deleted before. So, we can remove it from our orphaned block list
- // as if it was a success.
- if (!s.ok() && !s.IsNotFound()) {
- WARN_NOT_OK(s, Substitute("Could not delete block $0", b.ToString()));
- continue;
- }
-
- deleted.push_back(b);
+ transaction->AddDeletedBlock(b);
}
+ vector<BlockId> deleted;
+ WARN_NOT_OK(transaction->CommitDeletedBlocks(&deleted),
+ "not all orphaned blocks were deleted");
// Remove the successfully-deleted blocks from the set.
{
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 7590d32..28054cb 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -134,6 +134,7 @@ using consensus::OpId;
using consensus::RECEIVED_OPID;
using consensus::ReplicateRefPtr;
using consensus::ReplicateMsg;
+using fs::BlockDeletionTransaction;
using fs::FsReport;
using fs::WritableBlock;
using itest::ExternalMiniClusterFsInspector;
@@ -624,10 +625,12 @@ TEST_F(ToolTest, TestFsCheck) {
FsManager fs(env_, kTestDir);
FsReport report;
ASSERT_OK(fs.Open(&report));
+ std::shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ fs.block_manager()->NewDeletionTransaction();
for (int i = 0; i < block_ids.size(); i += 2) {
- ASSERT_OK(fs.DeleteBlock(block_ids[i]));
- missing_ids.push_back(block_ids[i]);
+ deletion_transaction->AddDeletedBlock(block_ids[i]);
}
+ deletion_transaction->CommitDeletedBlocks(&missing_ids);
}
NO_FATALS(RunFsCheck(Substitute("fs check --fs_wal_dir=$0", kTestDir),
block_ids.size() / 2, kTabletId, missing_ids, 0));
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/tools/tool_action_fs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_fs.cc b/src/kudu/tools/tool_action_fs.cc
index 139387e..444b62e 100644
--- a/src/kudu/tools/tool_action_fs.cc
+++ b/src/kudu/tools/tool_action_fs.cc
@@ -65,11 +65,13 @@ namespace tools {
using cfile::CFileReader;
using cfile::CFileIterator;
using cfile::ReaderOptions;
+using fs::BlockDeletionTransaction;
using fs::FsReport;
using fs::ReadableBlock;
using std::cout;
using std::endl;
using std::string;
+using std::shared_ptr;
using std::unique_ptr;
using std::unordered_map;
using std::vector;
@@ -139,6 +141,11 @@ Status Check(const RunnerContext& /*context*/) {
// Add orphaned blocks to the report after attempting to repair them.
report.orphaned_block_check.emplace();
+ shared_ptr<BlockDeletionTransaction> deletion_transaction;
+ if (FLAGS_repair) {
+ deletion_transaction = fs_manager.block_manager()->NewDeletionTransaction();
+ }
+ vector<BlockId> deleted;
for (const auto& id : orphaned_block_ids) {
// Opening a block isn't free, but the number of orphaned blocks shouldn't
// be extraordinarily high.
@@ -151,14 +158,20 @@ Status Check(const RunnerContext& /*context*/) {
fs::OrphanedBlockCheck::Entry entry(id, size);
if (FLAGS_repair) {
- Status s = fs_manager.DeleteBlock(id);
- WARN_NOT_OK(s, "Could not delete orphaned block");
- if (s.ok()) {
- entry.repaired = true;
- }
+ deletion_transaction->AddDeletedBlock(id);
}
report.orphaned_block_check->entries.emplace_back(entry);
}
+
+ if (FLAGS_repair) {
+ WARN_NOT_OK(deletion_transaction->CommitDeletedBlocks(&deleted),
+ "Could not delete orphaned blocks");
+ BlockIdSet deleted_set(deleted.begin(), deleted.end());
+ for (auto& entry : report.orphaned_block_check->entries) {
+ if (ContainsKey(deleted_set, entry.block_id)) entry.repaired = true;
+ }
+ }
+
return report.PrintAndCheckForFatalErrors();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/f373af07/src/kudu/tserver/tablet_copy_source_session-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_source_session-test.cc b/src/kudu/tserver/tablet_copy_source_session-test.cc
index f0cd053..2447805 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -91,6 +91,7 @@ using consensus::ConsensusMetadataManager;
using consensus::RaftConfigPB;
using consensus::RaftPeerPB;
using consensus::kMinimumTerm;
+using fs::BlockDeletionTransaction;
using fs::ReadableBlock;
using log::Log;
using log::LogOptions;
@@ -362,9 +363,14 @@ TEST_F(TabletCopyTest, TestBlocksAreFetchableAfterBeingDeleted) {
}
// Delete them.
+ shared_ptr<BlockDeletionTransaction> deletion_transaction =
+ fs_manager()->block_manager()->NewDeletionTransaction();
for (const BlockId& block_id : data_blocks) {
- ASSERT_OK(fs_manager()->DeleteBlock(block_id));
+ deletion_transaction->AddDeletedBlock(block_id);
}
+ vector<BlockId> deleted;
+ ASSERT_OK(deletion_transaction->CommitDeletedBlocks(&deleted));
+ ASSERT_EQ(data_blocks.size(), deleted.size());
// Read them back.
for (const BlockId& block_id : data_blocks) {