From d36eda2c4c1910af412b89f10ea66a8c8c2e3ac5 Mon Sep 17 00:00:00 2001 From: xuchaojie Date: Thu, 8 Jul 2021 17:02:38 +0800 Subject: [PATCH] chunkserver : fix bug in trash when collecting chunknum miss chunk in raft_snapshot dir --- src/chunkserver/trash.cpp | 32 ++++++++------- test/chunkserver/trash_test.cpp | 69 +++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/chunkserver/trash.cpp b/src/chunkserver/trash.cpp index e81b98d37b..14f7f858d0 100644 --- a/src/chunkserver/trash.cpp +++ b/src/chunkserver/trash.cpp @@ -306,27 +306,31 @@ bool Trash::IsWALFile(const std::string &fileName) { } uint32_t Trash::CountChunkNumInCopyset(const std::string ©setPath) { - auto count = [&](const std::string& path) { - std::vector chunks; - localFileSystem_->List(path, &chunks); + std::vector files; + if (0 != localFileSystem_->List(copysetPath, &files)) { + LOG(ERROR) << "Trash failed to list files in " << copysetPath; + return 0; + } - uint32_t chunkNum = 0; - for (auto& chunk : chunks) { + // Traverse subdirectories + uint32_t chunkNum = 0; + for (auto &file : files) { + std::string filePath = copysetPath + "/" + file; + bool isDir = localFileSystem_->DirExists(filePath); + if (!isDir) { // valid: chunkfile, snapshotfile, walfile - if (!(IsChunkOrSnapShotFile(chunk) || IsWALFile(chunk))) { + if (!(IsChunkOrSnapShotFile(file) || + IsWALFile(file))) { LOG(WARNING) << "Trash find a illegal file:" - << chunk << " in " << path - << ", filename: " << chunk; + << file << " in " << copysetPath; continue; } ++chunkNum; + } else { + chunkNum += CountChunkNumInCopyset(filePath); } - - return chunkNum; - }; - - return count(copysetPath + "/" + RAFT_DATA_DIR) - + count(copysetPath + "/" + RAFT_LOG_DIR); + } + return chunkNum; } } // namespace chunkserver diff --git a/test/chunkserver/trash_test.cpp b/test/chunkserver/trash_test.cpp index 6631ea8c5a..3c7975d571 100644 --- a/test/chunkserver/trash_test.cpp +++ b/test/chunkserver/trash_test.cpp @@ -557,7 +557,6 @@ TEST_F(TrashTest, recycle_copyset_dir_list_err) { EXPECT_CALL(*lfs, Mkdir(trashPath)).WillOnce(Return(0)); EXPECT_CALL(*lfs, Rename(dirPath, _, 0)).WillOnce(Return(0)); EXPECT_CALL(*lfs, List(_, _)) - .WillOnce(Return(-1)) .WillOnce(Return(-1)); ASSERT_EQ(0, trash->RecycleCopySet(dirPath)); } @@ -571,7 +570,6 @@ TEST_F(TrashTest, recycle_copyset_dir_ok) { EXPECT_CALL(*lfs, Mkdir(trashPath)).WillOnce(Return(0)); EXPECT_CALL(*lfs, Rename(dirPath, _, 0)).WillOnce(Return(0)); EXPECT_CALL(*lfs, List(_, _)) - .WillOnce(Return(0)) .WillOnce(Return(0)); ASSERT_EQ(0, trash->RecycleCopySet(dirPath)); } @@ -586,6 +584,7 @@ TEST_F(TrashTest, DISABLED_test_concurrenct) { TEST_F(TrashTest, test_chunk_num_statistic) { std::string trashPath = "./runlog/trash_test0/trash"; std::vector copysets{"4294967493.55555", "4294967494.55555"}; + std::vector dirs{"data", "log"}; std::vector chunks1{"chunk_100", "chunk_101"}; std::vector chunks2{"chunk_200_snap_1", "abc"}; std::vector logfiles1{ @@ -594,14 +593,30 @@ TEST_F(TrashTest, test_chunk_num_statistic) { std::vector logfiles2{}; // (1) chunk exists when init + // trash/ + // 4294967493.55555/ + // data/ + // chunk_100, chunk_101 +2 + // log/ + // curve_log_10086_10087, +1 + // curve_log_inprogress_10088, +1 + // log_10083_10084, + // log_inprogress_10085 + // 4294967494.55555/ + // data/ + // chunk_200_snap_1, abc +1 + // log/ + using item4list = struct{ std::string subdir; std::vector& names; }; std::vector action4List{ { "", copysets }, + { "/4294967493.55555", dirs}, { "/4294967493.55555/data", chunks1 }, { "/4294967493.55555/log", logfiles1 }, + { "/4294967494.55555", dirs}, { "/4294967494.55555/data", chunks2 }, { "/4294967494.55555/log", logfiles2 }, }; @@ -611,24 +626,64 @@ TEST_F(TrashTest, test_chunk_num_statistic) { .WillOnce(DoAll(SetArgPointee<1>(it.names), Return(0))); } + EXPECT_CALL(*lfs, DirExists(_)) + .WillOnce(Return(true)) // data + .WillOnce(Return(false)) // chunk_100 + .WillOnce(Return(false)) // chunk_101 + .WillOnce(Return(true)) // log + .WillOnce(Return(false)) // curve_log_10086_10087 + .WillOnce(Return(false)) // curve_log_inprogress_10088_10088 + .WillOnce(Return(false)) // log_10083_10084 + .WillOnce(Return(false)) // log_inprogress_10085 + .WillOnce(Return(true)) // data + .WillOnce(Return(false)) // chunk_200_snap_1 + .WillOnce(Return(false)) // abc + .WillOnce(Return(true)); // log + trash->Init(ops); ASSERT_EQ(5, trash->GetChunkNum()); // (2) recycle copyset - std::string copysetDir = "./runlog/trash_test0/copysets/4294967495"; + // trash_test0/copysets/4294967495/ + // data/ + // chunk_333, chunk_444 +2 + // log/ + // curve_log_10086_10087 +1 + // raft_snapshot/temp/data/ + // chunk_10000 +1 + // + std::string copysetDir = "/copysets/4294967495"; + std::vector dirs2{"data", "log", "raft_snapshot"}; EXPECT_CALL(*lfs, DirExists(_)) .WillOnce(Return(true)) + .WillOnce(Return(false)) + .WillOnce(Return(true)) // data + .WillOnce(Return(false)) + .WillOnce(Return(false)) + .WillOnce(Return(true)) // log + .WillOnce(Return(false)) + .WillOnce(Return(true)) // raft_snapshot + .WillOnce(Return(true)) // temp + .WillOnce(Return(true)) // data .WillOnce(Return(false)); - std::string trashedCopysetDir; + + std::string trashedCopysetDir = "/trash_test0/copysets/4294967495"; EXPECT_CALL(*lfs, Rename(copysetDir, _, 0)) .WillOnce(DoAll(SaveArg<1>(&trashedCopysetDir), Return(0))); std::vector chunks3{"chunk_333", "chunk_444"}; std::vector logfiles3{"curve_log_10086_10087"}; + std::vector temp{"temp"}; + std::vector data{"data"}; + std::vector chunks4{"chunk_10000"}; EXPECT_CALL(*lfs, List(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(dirs2), Return(0))) .WillOnce(DoAll(SetArgPointee<1>(chunks3), Return(0))) - .WillOnce(DoAll(SetArgPointee<1>(logfiles3), Return(0))); + .WillOnce(DoAll(SetArgPointee<1>(logfiles3), Return(0))) + .WillOnce(DoAll(SetArgPointee<1>(temp), Return(0))) + .WillOnce(DoAll(SetArgPointee<1>(data), Return(0))) + .WillOnce(DoAll(SetArgPointee<1>(chunks4), Return(0))); ASSERT_EQ(0, trash->RecycleCopySet(copysetDir)); - ASSERT_EQ(8, trash->GetChunkNum()); + ASSERT_EQ(9, trash->GetChunkNum()); // (3) delete eligible copyset std::string trashedCopysetName = @@ -702,7 +757,7 @@ TEST_F(TrashTest, test_chunk_num_statistic) { .Times(0); trash->DeleteEligibleFileInTrash(); - ASSERT_EQ(6, trash->GetChunkNum()); + ASSERT_EQ(7, trash->GetChunkNum()); } } // namespace chunkserver