This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 145d34b11 KUDU-3462 [UT] Fix a flaky test in log_block_manager-test
145d34b11 is described below
commit 145d34b11bc6d0d91401d237bf021d37c3e51632
Author: Yingchun Lai <[email protected]>
AuthorDate: Sat Aug 17 10:57:53 2024 +0800
KUDU-3462 [UT] Fix a flaky test in log_block_manager-test
This patch improves the determinacy of test
TestContainerBlockLimitingByMetadataSizeWithCompaction,
also adds some comments to explain the reason.
Change-Id: I913d99a6bbbfb16df1d1730701cab09222fc5c8c
Reviewed-on: http://gerrit.cloudera.org:8080/21686
Tested-by: Yingchun Lai <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/fs/log_block_manager-test.cc | 66 +++++++++++++++++++++++++----------
src/kudu/fs/log_block_manager.cc | 7 ++++
src/kudu/fs/log_block_manager.h | 8 +++++
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/src/kudu/fs/log_block_manager-test.cc
b/src/kudu/fs/log_block_manager-test.cc
index 50070e98e..a4ee00928 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -1349,6 +1349,8 @@ TEST_P(LogBlockManagerNativeMetaTest,
TestContainerBlockLimitingByMetadataSizeWi
const int kNumBlocks = 2000;
const int kNumThreads = 10;
const double kLiveBlockRatio = 0.1;
+ // Make sure the ratio values are set properly for the tests.
+ ASSERT_LT(FLAGS_log_container_metadata_size_before_compact_ratio, 1 -
kLiveBlockRatio);
// Creates and deletes some blocks.
auto create_and_delete_blocks = [&]() {
@@ -1363,15 +1365,16 @@ TEST_P(LogBlockManagerNativeMetaTest,
TestContainerBlockLimitingByMetadataSizeWi
}
// Deletes 'kNumBlocks * (1 - kLiveBlockRatio)' blocks.
- shared_ptr<BlockDeletionTransaction> deletion_transaction =
- bm_->NewDeletionTransaction();
+ std::mt19937 gen(SeedRandom());
+ std::shuffle(ids.begin(), ids.end(), gen);
+ ids.resize(ids.size() * (1 - kLiveBlockRatio));
+ auto deletion_transaction = bm_->NewDeletionTransaction();
for (const auto& id : ids) {
- if (rand() % 100 < 100 * kLiveBlockRatio) {
- continue;
- }
deletion_transaction->AddDeletedBlock(id);
}
- CHECK_OK(deletion_transaction->CommitDeletedBlocks(nullptr));
+ vector<BlockId> deleted_blocks;
+ CHECK_OK(deletion_transaction->CommitDeletedBlocks(&deleted_blocks));
+ CHECK_EQ(ids.size(), deleted_blocks.size());
};
// Create a thread pool to create and delete blocks.
@@ -1391,27 +1394,51 @@ TEST_P(LogBlockManagerNativeMetaTest,
TestContainerBlockLimitingByMetadataSizeWi
// Define a small value to make metadata easy to be full.
FLAGS_log_container_metadata_max_size = 32 * 1024;
NO_FATALS(mt_create_and_delete_blocks());
+
+ // After creating and deleting blocks, compaction will be triggered. All the
containers are left
+ // in the state of not satisfy compaction.
+ for (const auto &[_, container] : bm_->all_containers_by_name_) {
+
ASSERT_FALSE(LogBlockManagerNativeMeta::ContainerShouldCompactForTests(container.get()));
+ }
vector<string> metadata_files;
NO_FATALS(GetContainerMetadataFiles(&metadata_files));
+ int small_file_count = 0;
for (const auto& metadata_file : metadata_files) {
uint64_t file_size;
NO_FATALS(env_->GetFileSize(metadata_file, &file_size));
- ASSERT_GE(FLAGS_log_container_metadata_max_size *
- FLAGS_log_container_metadata_size_before_compact_ratio,
- file_size);
+ // When calculate compaction condition, we use metadata_file_->Offset(),
it is the encryption
+ // header size and real data size. So we need to add the encryption header
size here.
+ if (FLAGS_log_container_metadata_max_size *
+ FLAGS_log_container_metadata_size_before_compact_ratio >
+ Env::Default()->GetEncryptionHeaderSize() + file_size) {
+ small_file_count++;
+ }
}
+ // Most of the metadata files are smaller than the threshold.
+ // NOTE: It's possible that some metadata files are larger than the
threshold. When creating and
+ // deleting blocks, we delete 90% of the blocks, but they are randomly
selected, the delete ratio
+ // for a container may be less than (1 - kLiveBlockRatio), which means the
container will not be
+ // compacted.
+ ASSERT_GT(small_file_count, 0);
// Reopen and test again.
ASSERT_OK(ReopenBlockManager());
NO_FATALS(mt_create_and_delete_blocks());
+ for (const auto &[_, container] : bm_->all_containers_by_name_) {
+
ASSERT_FALSE(LogBlockManagerNativeMeta::ContainerShouldCompactForTests(container.get()));
+ }
NO_FATALS(GetContainerMetadataFiles(&metadata_files));
+ small_file_count = 0;
for (const auto& metadata_file : metadata_files) {
uint64_t file_size;
NO_FATALS(env_->GetFileSize(metadata_file, &file_size));
- ASSERT_GE(FLAGS_log_container_metadata_max_size *
- FLAGS_log_container_metadata_size_before_compact_ratio,
- file_size);
+ if (FLAGS_log_container_metadata_max_size *
+ FLAGS_log_container_metadata_size_before_compact_ratio >
+ Env::Default()->GetEncryptionHeaderSize() + file_size) {
+ small_file_count++;
+ }
}
+ ASSERT_GT(small_file_count, 0);
// Now remove the limit and create more blocks. They should go into existing
// containers, which are now no longer full.
@@ -1419,17 +1446,18 @@ TEST_P(LogBlockManagerNativeMetaTest,
TestContainerBlockLimitingByMetadataSizeWi
ASSERT_OK(ReopenBlockManager());
NO_FATALS(mt_create_and_delete_blocks());
NO_FATALS(GetContainerMetadataFiles(&metadata_files));
- bool exist_larger_one = false;
+ small_file_count = 0;
for (const auto& metadata_file : metadata_files) {
- uint64_t file_size = 0;
+ uint64_t file_size;
NO_FATALS(env_->GetFileSize(metadata_file, &file_size));
- if (file_size > FLAGS_log_container_metadata_max_size *
-
FLAGS_log_container_metadata_size_before_compact_ratio) {
- exist_larger_one = true;
- break;
+ if (FLAGS_log_container_metadata_max_size *
+ FLAGS_log_container_metadata_size_before_compact_ratio >
+ Env::Default()->GetEncryptionHeaderSize() + file_size) {
+ small_file_count++;
}
}
- ASSERT_TRUE(exist_larger_one);
+ // There must be some files larger than the threshold.
+ ASSERT_GT(metadata_files.size() - small_file_count, 0);
}
TEST_P(LogBlockManagerTest, TestMisalignedBlocksFuzz) {
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index b18cd55db..20047628d 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -864,6 +864,8 @@ class LogBlockContainerNativeMeta final : public
LogBlockContainer {
void PostWorkOfBlocksDeleted() override;
private:
+ friend class fs::LogBlockManagerNativeMeta;
+
// Performs sanity checks on it and removing it from 'live_blocks' and
// 'live_block_records', adds it to 'dead_blocks'.
//
@@ -3848,6 +3850,11 @@ Status LogBlockManagerNativeMeta::DoRepair(
return Status::OK();
}
+bool LogBlockManagerNativeMeta::ContainerShouldCompactForTests(
+ LogBlockContainer* container) {
+ return down_cast<LogBlockContainerNativeMeta*>(container)->ShouldCompact();
+}
+
Status LogBlockManager::Repair(
Dir* dir,
FsReport* report,
diff --git a/src/kudu/fs/log_block_manager.h b/src/kudu/fs/log_block_manager.h
index e42ffe6c0..84083b4d4 100644
--- a/src/kudu/fs/log_block_manager.h
+++ b/src/kudu/fs/log_block_manager.h
@@ -221,6 +221,8 @@ class LogBlockManager : public BlockManager {
BlockManagerOptions opts,
std::string tenant_id);
+ FRIEND_TEST(LogBlockManagerNativeMetaTest,
+ TestContainerBlockLimitingByMetadataSizeWithCompaction);
FRIEND_TEST(LogBlockManagerNativeMetaTest, TestMetadataTruncation);
FRIEND_TEST(LogBlockManagerTest, TestAbortBlock);
FRIEND_TEST(LogBlockManagerTest, TestCloseFinalizedBlock);
@@ -592,6 +594,12 @@ class LogBlockManagerNativeMeta : public LogBlockManager {
FsReport* report,
const ContainerBlocksByName& low_live_block_containers,
const ContainersByName& containers_by_name) override;
+
+private:
+ FRIEND_TEST(LogBlockManagerNativeMetaTest,
+ TestContainerBlockLimitingByMetadataSizeWithCompaction);
+
+ static bool ContainerShouldCompactForTests(internal::LogBlockContainer*
container);
};
#if !defined(NO_ROCKSDB)
