This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 4cb68284c fix(ut): fix flaky unit test test_rename_path_while_writing
(#1630)
4cb68284c is described below
commit 4cb68284cf4931b0ade34c31ce18156ad618d158
Author: Yingchun Lai <[email protected]>
AuthorDate: Wed Oct 11 15:41:17 2023 +0800
fix(ut): fix flaky unit test test_rename_path_while_writing (#1630)
https://github.com/apache/incubator-pegasus/issues/1631
Add retry logic for test `HDFSClientTest.test_rename_path_while_writing`,
fix an
used-after-free ASAN issue and remove tar files after downloading from
artifacts
to reduce space consumption.
---
.github/workflows/lint_and_test_cpp.yaml | 4 ++
src/block_service/test/hdfs_service_test.cpp | 79 ++++++++++++++++------------
src/test_util/test_util.h | 2 +-
3 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/.github/workflows/lint_and_test_cpp.yaml
b/.github/workflows/lint_and_test_cpp.yaml
index 6c125727c..f3dc28d38 100644
--- a/.github/workflows/lint_and_test_cpp.yaml
+++ b/.github/workflows/lint_and_test_cpp.yaml
@@ -247,6 +247,7 @@ jobs:
- name: Tar files
run: |
tar -zxvf release__builder.tar
+ rm -f release__builder.tar
- name: Unit Testing
run: |
export
LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
@@ -386,6 +387,7 @@ jobs:
- name: Tar files
run: |
tar -zxvf release_address_builder.tar
+ rm -f release_address_builder.tar
- name: Unit Testing
run: |
export
LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
@@ -527,6 +529,7 @@ jobs:
# - name: Tar files
# run: |
# tar -zxvf release_undefined_builder.tar
+# rm -f release_undefined_builder.tar
# - name: Unit Testing
# run: |
# export
LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
@@ -639,6 +642,7 @@ jobs:
- name: Tar files
run: |
tar -zxvf release_jemalloc_builder.tar
+ rm -f release_jemalloc_builder.tar
- name: Unit Testing
run: |
export
LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
diff --git a/src/block_service/test/hdfs_service_test.cpp
b/src/block_service/test/hdfs_service_test.cpp
index d7b33a8da..0eaec29e8 100644
--- a/src/block_service/test/hdfs_service_test.cpp
+++ b/src/block_service/test/hdfs_service_test.cpp
@@ -69,11 +69,21 @@ DEFINE_TASK_CODE(LPC_TEST_HDFS, TASK_PRIORITY_HIGH,
dsn::THREAD_POOL_DEFAULT)
class HDFSClientTest : public pegasus::encrypt_data_test_base
{
protected:
+ HDFSClientTest() : pegasus::encrypt_data_test_base()
+ {
+ for (int i = 0; i < FLAGS_num_test_file_lines; ++i) {
+ _local_test_data += "test";
+ }
+ }
+
void generate_test_file(const std::string &filename);
void write_test_files_async(const std::string &local_test_path,
int total_files,
int *success_count,
task_tracker *tracker);
+
+private:
+ std::string _local_test_data;
};
void HDFSClientTest::generate_test_file(const std::string &filename)
@@ -97,29 +107,24 @@ void HDFSClientTest::write_test_files_async(const
std::string &local_test_path,
task_tracker *tracker)
{
dsn::utils::filesystem::create_directory(local_test_path);
- std::string local_test_data;
- for (int i = 0; i < FLAGS_num_test_file_lines; ++i) {
- local_test_data += "test";
- }
for (int i = 0; i < total_files; ++i) {
- tasking::enqueue(
- LPC_TEST_HDFS, tracker, [&local_test_path, &local_test_data, i,
success_count]() {
- // mock the writing process in hdfs_file_object::download().
- std::string test_file_name = local_test_path + "/test_file_" +
std::to_string(i);
- auto s = rocksdb::WriteStringToFile(rocksdb::Env::Default(),
-
rocksdb::Slice(local_test_data),
- test_file_name,
- /* should_sync */ true);
- if (s.ok()) {
- ++(*success_count);
- } else {
- CHECK(s.IsIOError(), "{}", s.ToString());
- auto pos1 = s.ToString().find(
- "IO error: No such file or directory: While open a
file for appending: ");
- auto pos2 = s.ToString().find("IO error: While appending
to file: ");
- CHECK(pos1 == 0 || pos2 == 0, "{}", s.ToString());
- }
- });
+ tasking::enqueue(LPC_TEST_HDFS, tracker, [this, &local_test_path, i,
success_count]() {
+ // mock the writing process in hdfs_file_object::download().
+ std::string test_file_name = local_test_path + "/test_file_" +
std::to_string(i);
+ auto s = rocksdb::WriteStringToFile(rocksdb::Env::Default(),
+
rocksdb::Slice(_local_test_data),
+ test_file_name,
+ /* should_sync */ true);
+ if (s.ok()) {
+ ++(*success_count);
+ } else {
+ CHECK(s.IsIOError(), "{}", s.ToString());
+ auto pos1 = s.ToString().find(
+ "IO error: No such file or directory: While open a file
for appending: ");
+ auto pos2 = s.ToString().find("IO error: While appending to
file: ");
+ CHECK(pos1 == 0 || pos2 == 0, "{}", s.ToString());
+ }
+ });
}
}
@@ -443,17 +448,21 @@ TEST_P(HDFSClientTest, test_rename_path_while_writing)
std::string kLocalTestPath = "test_dir";
const int kTotalFiles = 100;
- task_tracker tracker;
- int success_count = 0;
- write_test_files_async(kLocalTestPath, kTotalFiles, &success_count,
&tracker);
- usleep(100);
-
- std::string kLocalRenamedTestPath = "rename_dir." +
std::to_string(dsn_now_ms());
- // Rename succeed but some files write failed.
- ASSERT_TRUE(dsn::utils::filesystem::rename_path(kLocalTestPath,
kLocalRenamedTestPath));
- tracker.cancel_outstanding_tasks();
- // Generally, we can assume partial files are written failed.
- // It maybe flaky, please retry if it failed.
- ASSERT_GT(success_count, 0) << success_count;
- ASSERT_LT(success_count, kTotalFiles) << success_count;
+ // The test is flaky, retry if it failed in 300 seconds.
+ ASSERT_IN_TIME_WITH_FIXED_INTERVAL(
+ [&] {
+ task_tracker tracker;
+ int success_count = 0;
+ write_test_files_async(kLocalTestPath, kTotalFiles,
&success_count, &tracker);
+ usleep(100);
+
+ std::string kLocalRenamedTestPath = "rename_dir." +
std::to_string(dsn_now_ms());
+ // Rename succeed but some files write failed.
+ ASSERT_TRUE(dsn::utils::filesystem::rename_path(kLocalTestPath,
kLocalRenamedTestPath));
+ tracker.cancel_outstanding_tasks();
+ // Generally, we can assume partial files are written failed.
+ ASSERT_GT(success_count, 0) << success_count;
+ ASSERT_LT(success_count, kTotalFiles) << success_count;
+ },
+ 300);
}
diff --git a/src/test_util/test_util.h b/src/test_util/test_util.h
index 762d15436..debeb7fa9 100644
--- a/src/test_util/test_util.h
+++ b/src/test_util/test_util.h
@@ -59,7 +59,7 @@ void create_local_test_file(const std::string &full_name,
dsn::replication::file
#define ASSERT_IN_TIME_WITH_FIXED_INTERVAL(expr, sec)
\
do {
\
- AssertEventually(expr, sec, WaitBackoff::NONE);
\
+ AssertEventually(expr, sec, ::pegasus::WaitBackoff::NONE);
\
NO_PENDING_FATALS();
\
} while (0)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
