From 1050df7076b01b62204576dd06c704dbfe4e5557 Mon Sep 17 00:00:00 2001 From: Mingyu Chen Date: Thu, 30 Mar 2023 21:36:07 +0800 Subject: [PATCH] [fix](fs) fix local file system copy bug (#18243) `copy_dirs` has a bug that will cause infinity iteration --- be/src/io/fs/local_file_system.cpp | 16 +-- be/src/io/fs/local_file_system.h | 3 - be/test/io/fs/local_file_system_test.cpp | 156 +++++++++++++++++++++++ 3 files changed, 157 insertions(+), 18 deletions(-) diff --git a/be/src/io/fs/local_file_system.cpp b/be/src/io/fs/local_file_system.cpp index c82295ae27..5ffcabca5e 100644 --- a/be/src/io/fs/local_file_system.cpp +++ b/be/src/io/fs/local_file_system.cpp @@ -327,24 +327,10 @@ Status LocalFileSystem::get_space_info_impl(const Path& path, size_t* capacity, return Status::OK(); } -Status LocalFileSystem::resize_file(const Path& file, size_t new_size) { - auto path = absolute_path(file); - FILESYSTEM_M(resize_file_impl(path, new_size)); -} - -Status LocalFileSystem::resize_file_impl(const Path& file, size_t new_size) { - std::error_code ec; - std::filesystem::resize_file(file, new_size, ec); - if (ec) { - return Status::IOError("failed to resize file {}: {}", file.native(), errcode_to_str(ec)); - } - return Status::OK(); -} - Status LocalFileSystem::copy_dirs(const Path& src, const Path& dest) { auto src_path = absolute_path(src); auto dest_path = absolute_path(dest); - FILESYSTEM_M(copy_dirs(src_path, dest_path)); + FILESYSTEM_M(copy_dirs_impl(src_path, dest_path)); } Status LocalFileSystem::copy_dirs_impl(const Path& src, const Path& dest) { diff --git a/be/src/io/fs/local_file_system.h b/be/src/io/fs/local_file_system.h index 308cd5e1fc..da1704f381 100644 --- a/be/src/io/fs/local_file_system.h +++ b/be/src/io/fs/local_file_system.h @@ -50,8 +50,6 @@ public: Status delete_and_create_directory(const Path& dir); // return disk available space where the given path is. Status get_space_info(const Path& path, size_t* capacity, size_t* available); - // changes the size of the regular file - Status resize_file(const Path& file, size_t new_size); // copy src dir to dest dir, recursivly Status copy_dirs(const Path& src, const Path& dest); // return true if parent path contain sub path @@ -78,7 +76,6 @@ protected: Status mtime_impl(const Path& file, time_t* m_time); Status delete_and_create_directory_impl(const Path& dir); Status get_space_info_impl(const Path& path, size_t* capacity, size_t* available); - Status resize_file_impl(const Path& file, size_t new_size); Status copy_dirs_impl(const Path& src, const Path& dest); private: diff --git a/be/test/io/fs/local_file_system_test.cpp b/be/test/io/fs/local_file_system_test.cpp index 90cd5d7391..2080feac2b 100644 --- a/be/test/io/fs/local_file_system_test.cpp +++ b/be/test/io/fs/local_file_system_test.cpp @@ -308,6 +308,162 @@ TEST_F(LocalFileSystemTest, TestContainPath) { } } +TEST_F(LocalFileSystemTest, TestRename) { + std::string path = "./file_rename/"; + std::string new_path = "./file_rename2/"; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->rename_dir(path, new_path).ok()); + + save_string_file("./file_rename2/f1", "just test1"); + EXPECT_TRUE( + io::global_local_filesystem()->rename("./file_rename2/f1", "./file_rename2/f2").ok()); + + std::vector dirs; + std::vector files; + EXPECT_TRUE(list_dirs_files(new_path, &dirs, &files).ok()); + EXPECT_EQ(0, dirs.size()); + EXPECT_EQ(1, files.size()); + EXPECT_EQ("f2", files[0]); + + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(new_path).ok()); +} + +TEST_F(LocalFileSystemTest, TestLink) { + std::string path = "./file_link/"; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory(path).ok()); + + // link file + save_string_file("./file_link/f2", "just test2"); + EXPECT_TRUE( + io::global_local_filesystem()->link_file("./file_link/f2", "./file_link/f2-1").ok()); + std::vector dirs; + std::vector files; + EXPECT_TRUE(list_dirs_files("./file_link/", &dirs, &files).ok()); + EXPECT_EQ(0, dirs.size()); + EXPECT_EQ(2, files.size()); + + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->delete_directory("file_link2").ok()); +} + +TEST_F(LocalFileSystemTest, TestMD5) { + std::string path = "./file_md5/"; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory(path).ok()); + + // link fir + save_string_file("./file_md5/f1", "just test1"); + std::string md5; + EXPECT_TRUE(io::global_local_filesystem()->md5sum("./file_md5/f1", &md5).ok()); + EXPECT_EQ("56947c63232fef1c65e4c3f4d1c69a9c", md5); + + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); +} + +TEST_F(LocalFileSystemTest, TestCopyAndBatchDelete) { + std::string path = "./file_copy/"; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_copy/1").ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_copy/2").ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_copy/3").ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_copy/4").ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_copy/5").ok()); + + save_string_file("./file_copy/f1", "just test1"); + save_string_file("./file_copy/f2", "just test2"); + save_string_file("./file_copy/f3", "just test3"); + save_string_file("./file_copy/1/f4", "just test3"); + save_string_file("./file_copy/2/f5", "just test3"); + + // copy + std::string dest_path = "./file_copy_dest/"; + EXPECT_TRUE(io::global_local_filesystem()->copy_dirs(path, dest_path).ok()); + + std::vector dirs; + std::vector files; + EXPECT_TRUE(list_dirs_files("./file_copy_dest", &dirs, &files).ok()); + EXPECT_EQ(5, dirs.size()); + EXPECT_EQ(3, files.size()); + + dirs.clear(); + files.clear(); + EXPECT_TRUE(list_dirs_files("./file_copy_dest/1/", &dirs, &files).ok()); + EXPECT_EQ(0, dirs.size()); + EXPECT_EQ(1, files.size()); + + dirs.clear(); + files.clear(); + EXPECT_TRUE(list_dirs_files("./file_copy_dest/2/", &dirs, &files).ok()); + EXPECT_EQ(0, dirs.size()); + EXPECT_EQ(1, files.size()); + + // batch delete + std::vector delete_files; + delete_files.emplace_back("./file_copy/f3"); + delete_files.emplace_back("./file_copy/1/f4"); + delete_files.emplace_back("./file_copy/2/f5"); + EXPECT_TRUE(io::global_local_filesystem()->batch_delete(delete_files).ok()); + + dirs.clear(); + files.clear(); + EXPECT_TRUE(list_dirs_files("./file_copy/1/", &dirs, &files).ok()); + EXPECT_EQ(0, dirs.size()); + EXPECT_EQ(0, files.size()); + + dirs.clear(); + files.clear(); + EXPECT_TRUE(list_dirs_files("./file_copy/", &dirs, &files).ok()); + EXPECT_EQ(5, dirs.size()); + EXPECT_EQ(2, files.size()); + + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(dest_path).ok()); +} + +TEST_F(LocalFileSystemTest, TestIterate) { + std::string path = "./file_iterate/"; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory(path).ok()); + + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_iterate/d1").ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory("./file_iterate/d2").ok()); + save_string_file("./file_iterate/f1", "just test1"); + save_string_file("./file_iterate/f2", "just test2"); + save_string_file("./file_iterate/f3", "just test3"); + save_string_file("./file_iterate/d1/f4", "just test4"); + save_string_file("./file_iterate/d2/f5", "just test5"); + + int64_t file_count = 0; + int64_t dir_count = 0; + auto cb = [&](const io::FileInfo& file) -> bool { + if (file.is_file) { + if (file.file_name != "f1") { + file_count++; + } + } else { + dir_count++; + } + return true; + }; + + auto cb2 = [&](const io::FileInfo& file) -> bool { return false; }; + + EXPECT_TRUE(io::global_local_filesystem()->iterate_directory("./file_iterate/", cb).ok()); + EXPECT_EQ(2, file_count); + EXPECT_EQ(2, dir_count); + + file_count = 0; + dir_count = 0; + EXPECT_TRUE(io::global_local_filesystem()->iterate_directory("./file_iterate/", cb2).ok()); + EXPECT_EQ(0, file_count); + EXPECT_EQ(0, dir_count); + + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok()); +} + TEST_F(LocalFileSystemTest, TestListDirsFiles) { std::string path = "./file_test/"; EXPECT_TRUE(io::global_local_filesystem()->delete_directory(path).ok());