This is an automated email from the ASF dual-hosted git repository. joemcdonnell pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit b7452c66b0aacfafb0a4e6840d49d58b6f17f52b Author: Yida Wu <[email protected]> AuthorDate: Fri Sep 20 10:47:12 2024 -0700 IMPALA-11934: Adjust the timeout interval for temporary file tests This patch increases the timeout interval for temporary file test cases. Initially, the timeout was set to 2 seconds, as it was for hdfs files. However, after integrating ozone for temporary files, this timeout seems too short for ozone uploads, causing some related test cases in the ozone builds to become flaky. So, the timeout is extended to 20 seconds in this patch. Tests: Ran disk-io-mgr-test and tmp-file-mgr-test, and no failures were observed. Change-Id: I23ec44abd73426ed223aedb5d5634512861ac571 Reviewed-on: http://gerrit.cloudera.org:8080/21835 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- be/src/runtime/io/disk-io-mgr-test.cc | 47 ++++++++++++----------------------- be/src/runtime/tmp-file-mgr-test.cc | 33 +++++++++++------------- 2 files changed, 30 insertions(+), 50 deletions(-) diff --git a/be/src/runtime/io/disk-io-mgr-test.cc b/be/src/runtime/io/disk-io-mgr-test.cc index f6d88bcee..d4a723670 100644 --- a/be/src/runtime/io/disk-io-mgr-test.cc +++ b/be/src/runtime/io/disk-io-mgr-test.cc @@ -297,6 +297,19 @@ class DiskIoMgrTest : public testing::Test { scan_range->SetFileReader(move(reader_stub)); } + static bool WaitForStatus(DiskFile* disk_file, DiskFileStatus target_status) { + DCHECK(disk_file != nullptr); + // Suppose the upload should be finished in 20 seconds. + int max_wait_times = 20; + while (max_wait_times-- > 0) { + if (disk_file->GetFileStatus() == target_status) { + return true; + } + usleep(1000 * 1000); // sleep 1s each round. + } + return false; + } + ScanRange* InitRange(ObjectPool* pool, const char* file_path, int offset, int len, int disk_id, int64_t mtime, void* meta_data = nullptr, bool is_hdfs_cached = false, std::vector<ScanRange::SubRange> sub_ranges = {}) { @@ -2044,17 +2057,7 @@ TEST_F(DiskIoMgrTest, WriteToRemoteSuccess) { (*new_tmp_file_obj)->DiskBufferFile(), (*new_tmp_file_obj)->DiskFile(), file_size, disk_id, RequestType::FILE_UPLOAD, &io_mgr, u_callback)); EXPECT_OK(io_ctx->AddRemoteOperRange(upload_range)); - - int wait_times = 10; - while (true) { - if ((*new_tmp_file_obj)->DiskFile()->GetFileStatus() == DiskFileStatus::PERSISTED) { - break; - } - // Suppose the upload should be finished in two seconds. - ASSERT_TRUE(wait_times-- > 0); - usleep(200 * 1000); - } - + EXPECT_TRUE(WaitForStatus((*new_tmp_file_obj)->DiskFile(), DiskFileStatus::PERSISTED)); EXPECT_TRUE(HdfsFileExist(remote_file_path)); auto exam_fuc = [&](HistogramMetric* metric, const string& keyname) { @@ -2356,16 +2359,7 @@ TEST_F(DiskIoMgrTest, WriteToRemoteEvictLocal) { new RemoteOperRange(shared_tmp_file->DiskBufferFile(), shared_tmp_file->DiskFile(), file_size, 0, RequestType::FILE_UPLOAD, &io_mgr, u_callback)); Status add_status = io_ctx->AddRemoteOperRange(upload_range); - - int wait_times = 10; - while (true) { - if (shared_tmp_file->DiskFile()->GetFileStatus() == DiskFileStatus::PERSISTED) { - break; - } - // Suppose the upload should be finished in two seconds. - ASSERT_TRUE(wait_times-- > 0); - usleep(200 * 1000); - } + EXPECT_TRUE(WaitForStatus(shared_tmp_file->DiskFile(), DiskFileStatus::PERSISTED)); // TryEvictFile and the local buffer file should be evicted for releasing local // scratch space. @@ -2498,16 +2492,7 @@ TEST_F(DiskIoMgrTest, WriteToRemoteDiffPagesSuccess) { (*new_tmp_file_obj)->DiskBufferFile(), (*new_tmp_file_obj)->DiskFile(), block_size, disk_id, RequestType::FILE_UPLOAD, &io_mgr, u_callback)); EXPECT_OK(io_ctx->AddRemoteOperRange(upload_range)); - - int wait_times = 10; - while (true) { - if ((*new_tmp_file_obj)->DiskFile()->GetFileStatus() == DiskFileStatus::PERSISTED) { - break; - } - // Suppose the upload should be finished in two seconds. - ASSERT_TRUE(wait_times-- > 0); - usleep(200 * 1000); - } + EXPECT_TRUE(WaitForStatus((*new_tmp_file_obj)->DiskFile(), DiskFileStatus::PERSISTED)); // Assert remote file actual size is the same as the local actual size. EXPECT_TRUE(HdfsFileExist(remote_file_path)); diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc index c00d0480b..6dd49c58d 100644 --- a/be/src/runtime/tmp-file-mgr-test.cc +++ b/be/src/runtime/tmp-file-mgr-test.cc @@ -311,18 +311,18 @@ class TmpFileMgrTest : public ::testing::Test { EXPECT_EQ(end, search->second.end); } - /// Helper to wait for the disk file changing to specific status. Will timeout after 2 + /// Helper to wait for the disk file changing to specific status. Will timeout after 20 /// seconds. - static void WaitForDiskFileStatus(DiskFile* file, DiskFileStatus status) { - int wait_times = 10; - while (true) { + static bool WaitForDiskFileStatus(DiskFile* file, DiskFileStatus status) { + DCHECK(file != nullptr); + int wait_times = 20; + while (wait_times-- > 0) { if (file->GetFileStatus() == status) { - break; + return true; } - // Suppose the upload should be finished in two seconds. - ASSERT_TRUE(wait_times-- > 0); - usleep(200 * 1000); + usleep(1000 * 1000); // sleep 1s each round. } + return false; } /// Helper to get the remote temporary file from the temporary file group. @@ -1300,13 +1300,8 @@ void TmpFileMgrTest::TestCompressBufferManagement( EXPECT_NE(compressed_handle->file_, uncompressed_handle->file_); auto wait_upload_func = [&](TmpFile* tmp_file) { // Wait until the file has been uploaded to remote dir. - // Should be finished in 2 seconds. - int wait_times = 10; - while (wait_times-- > 0) { - if (tmp_file->DiskFile()->GetFileStatus() == io::DiskFileStatus::PERSISTED) break; - usleep(200 * 1000); - } - EXPECT_EQ(tmp_file->DiskFile()->GetFileStatus(), io::DiskFileStatus::PERSISTED); + EXPECT_TRUE(WaitForDiskFileStatus(tmp_file->DiskFile(), + io::DiskFileStatus::PERSISTED)); // Remove the local buffer to enforce reading from the remote file. unique_lock<shared_mutex> buffer_file_lock( *(tmp_file->GetWriteFile()->GetFileLock())); @@ -2177,10 +2172,10 @@ TEST_F(TmpFileMgrTest, TestBatchReadingFromRemote) { ASSERT_EQ(GetRemoteTmpFileNum(file_group), 2); auto file1 = GetRemoteTmpFileByFileGroup(file_group, 0); auto file2 = GetRemoteTmpFileByFileGroup(file_group, 1); - WaitForDiskFileStatus(file1->DiskFile(), DiskFileStatus::PERSISTED); - WaitForDiskFileStatus(file1->DiskBufferFile(), DiskFileStatus::PERSISTED); - WaitForDiskFileStatus(file2->DiskFile(), DiskFileStatus::PERSISTED); - WaitForDiskFileStatus(file2->DiskBufferFile(), DiskFileStatus::PERSISTED); + EXPECT_TRUE(WaitForDiskFileStatus(file1->DiskFile(), DiskFileStatus::PERSISTED)); + EXPECT_TRUE(WaitForDiskFileStatus(file1->DiskBufferFile(), DiskFileStatus::PERSISTED)); + EXPECT_TRUE(WaitForDiskFileStatus(file2->DiskFile(), DiskFileStatus::PERSISTED)); + EXPECT_TRUE(WaitForDiskFileStatus(file2->DiskBufferFile(), DiskFileStatus::PERSISTED)); // Check Actual Size is as expected. ASSERT_EQ(file1->DiskBufferFile()->actual_file_size(), file_size_1);
