From dfd1e78ed7be86fd673a0e1f1f35ba3db8d1632c Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 16 Dec 2025 11:50:03 +0800 Subject: [PATCH 1/7] fix: resolve multi thread mkdir error --- src/paimon/common/fs/file_system_test.cpp | 44 +++++++++++++++++++---- src/paimon/fs/local/local_file.cpp | 11 +++--- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/paimon/common/fs/file_system_test.cpp b/src/paimon/common/fs/file_system_test.cpp index e87f3c2d..b6b0d1f6 100644 --- a/src/paimon/common/fs/file_system_test.cpp +++ b/src/paimon/common/fs/file_system_test.cpp @@ -979,16 +979,21 @@ TEST_P(FileSystemTest, TestMkdirsFailsWithExistingParentFile) { } TEST_P(FileSystemTest, TestMkdir) { - std::string path = PathUtil::JoinPath(test_root_, "/tmp.txt/tmpB"); - ASSERT_OK(fs_->Mkdirs(path)); + { + std::string path = PathUtil::JoinPath(test_root_, "/tmp.txt/tmpB"); + ASSERT_OK(fs_->Mkdirs(path)); + } + { + std::string path = PathUtil::JoinPath(test_root_, "/tmpA/tmpB/"); + ASSERT_OK(fs_->Mkdirs(path)); + } + { + std::string path = "/"; + ASSERT_OK(fs_->Mkdirs(path)); + } } TEST_P(FileSystemTest, TestMkdir2) { - std::string path = PathUtil::JoinPath(test_root_, "/tmpA/tmpB/"); - ASSERT_OK(fs_->Mkdirs(path)); -} - -TEST_P(FileSystemTest, TestMkdir3) { { std::string dir_path = test_root_ + "/file_dir/"; ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); @@ -1012,6 +1017,7 @@ TEST_P(FileSystemTest, TestMkdir3) { } } +// test for create multi dir such as "partition1/bucket1" and "partition1/bucket2" TEST_P(FileSystemTest, TestMkdirMultiThread) { uint32_t runs_count = 10; uint32_t thread_count = 10; @@ -1036,6 +1042,30 @@ TEST_P(FileSystemTest, TestMkdirMultiThread) { } } +// test for create multi dir such as "partition1" and "partition1" +TEST_P(FileSystemTest, TestMkdirMultiThread2) { + uint32_t runs_count = 10; + uint32_t thread_count = 10; + auto executor = CreateDefaultExecutor(thread_count); + + for (uint32_t i = 0; i < runs_count; i++) { + std::string uuid; + ASSERT_TRUE(UUID::Generate(&uuid)); + std::vector> futures; + for (uint32_t thread_idx = 0; thread_idx < thread_count; thread_idx++) { + futures.push_back(Via(executor.get(), [this, &uuid]() -> void { + std::string dir_path = PathUtil::JoinPath(test_root_, uuid); + // ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); + // ASSERT_FALSE(is_exist); + ASSERT_OK(fs_->Mkdirs(dir_path)); + ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); + ASSERT_TRUE(is_exist); + })); + } + Wait(futures); + } +} + TEST_P(FileSystemTest, TestInvalidMkdir) { { // test mkdir with one exist dir diff --git a/src/paimon/fs/local/local_file.cpp b/src/paimon/fs/local/local_file.cpp index 947858a6..9ca70f97 100644 --- a/src/paimon/fs/local/local_file.cpp +++ b/src/paimon/fs/local/local_file.cpp @@ -163,9 +163,6 @@ Status LocalFile::Mkdir() const { dir.resize(len - 1); } } - if (access(dir.c_str(), F_OK) == 0) { - return Status::Exist(fmt::format("directory '{}' already exist", dir)); - } size_t pos = dir.rfind('/'); if (pos == std::string::npos) { if (mkdir(dir.c_str(), 0755) < 0) { @@ -180,9 +177,11 @@ Status LocalFile::Mkdir() const { PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir)); } if (mkdir(dir.c_str(), 0755) < 0) { - int32_t cur_errno = errno; - return Status::IOError( - fmt::format("create directory '{}' failed, ec: {}", dir, std::strerror(cur_errno))); + if (errno != EEXIST) { + int32_t cur_errno = errno; + return Status::IOError( + fmt::format("create directory '{}' failed, ec: {}", dir, std::strerror(cur_errno))); + } } return Status::OK(); } From c538452527a99acff362b85a6ca6bd54cee28117 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 16 Dec 2025 13:16:32 +0800 Subject: [PATCH 2/7] fix --- src/paimon/common/fs/file_system_test.cpp | 28 +++++++++++++++++++---- src/paimon/fs/local/local_file.cpp | 10 ++++---- src/paimon/fs/local/local_file_test.cpp | 4 ++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/paimon/common/fs/file_system_test.cpp b/src/paimon/common/fs/file_system_test.cpp index b6b0d1f6..56a6464b 100644 --- a/src/paimon/common/fs/file_system_test.cpp +++ b/src/paimon/common/fs/file_system_test.cpp @@ -1017,7 +1017,7 @@ TEST_P(FileSystemTest, TestMkdir2) { } } -// test for create multi dir such as "partition1/bucket1" and "partition1/bucket2" +// test for create multi dir such as "/table/partition1/bucket1" and "/table/partition1/bucket2" TEST_P(FileSystemTest, TestMkdirMultiThread) { uint32_t runs_count = 10; uint32_t thread_count = 10; @@ -1042,7 +1042,7 @@ TEST_P(FileSystemTest, TestMkdirMultiThread) { } } -// test for create multi dir such as "partition1" and "partition1" +// test for create multi dir such as "/table/partition1" and "/table/partition1" TEST_P(FileSystemTest, TestMkdirMultiThread2) { uint32_t runs_count = 10; uint32_t thread_count = 10; @@ -1055,8 +1055,28 @@ TEST_P(FileSystemTest, TestMkdirMultiThread2) { for (uint32_t thread_idx = 0; thread_idx < thread_count; thread_idx++) { futures.push_back(Via(executor.get(), [this, &uuid]() -> void { std::string dir_path = PathUtil::JoinPath(test_root_, uuid); - // ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); - // ASSERT_FALSE(is_exist); + ASSERT_OK(fs_->Mkdirs(dir_path)); + ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); + ASSERT_TRUE(is_exist); + })); + } + Wait(futures); + } +} + +// test for create multi dir such as "partition1" and "partition1" (relative path) +TEST_P(FileSystemTest, TestMkdirMultiThread3) { + uint32_t runs_count = 10; + uint32_t thread_count = 10; + auto executor = CreateDefaultExecutor(thread_count); + + for (uint32_t i = 0; i < runs_count; i++) { + std::string uuid; + ASSERT_TRUE(UUID::Generate(&uuid)); + std::vector> futures; + for (uint32_t thread_idx = 0; thread_idx < thread_count; thread_idx++) { + futures.push_back(Via(executor.get(), [this, &uuid]() -> void { + std::string dir_path = uuid; ASSERT_OK(fs_->Mkdirs(dir_path)); ASSERT_OK_AND_ASSIGN(bool is_exist, fs_->Exists(dir_path)); ASSERT_TRUE(is_exist); diff --git a/src/paimon/fs/local/local_file.cpp b/src/paimon/fs/local/local_file.cpp index 9ca70f97..0e187ce8 100644 --- a/src/paimon/fs/local/local_file.cpp +++ b/src/paimon/fs/local/local_file.cpp @@ -158,7 +158,7 @@ Status LocalFile::Mkdir() const { size_t len = dir.size(); if (dir[len - 1] == '/') { if (len == 1) { - return Status::Exist(fmt::format("directory '{}' already exist", dir)); + return Status::OK(); } else { dir.resize(len - 1); } @@ -166,9 +166,11 @@ Status LocalFile::Mkdir() const { size_t pos = dir.rfind('/'); if (pos == std::string::npos) { if (mkdir(dir.c_str(), 0755) < 0) { - int32_t cur_errno = errno; - return Status::IOError( - fmt::format("Mkdir path '{}' fail, ec: {}", dir, std::strerror(cur_errno))); + if (errno != EEXIST) { + int32_t cur_errno = errno; + return Status::IOError( + fmt::format("Mkdir path '{}' fail, ec: {}", dir, std::strerror(cur_errno))); + } } return Status::OK(); } diff --git a/src/paimon/fs/local/local_file_test.cpp b/src/paimon/fs/local/local_file_test.cpp index 254c9eb2..60cc022a 100644 --- a/src/paimon/fs/local/local_file_test.cpp +++ b/src/paimon/fs/local/local_file_test.cpp @@ -178,9 +178,9 @@ TEST(LocalFileTest, TestOpenFile) { ASSERT_GE(modify_time, -1); LocalFile dir2 = LocalFile("/"); - ASSERT_NOK_WITH_MSG(dir2.Mkdir(), "directory '/' already exist"); + ASSERT_OK(dir2.Mkdir()); LocalFile dir3 = LocalFile(test_root + "/"); - ASSERT_NOK_WITH_MSG(dir3.Mkdir(), "already exist"); + ASSERT_OK(dir3.Mkdir()); } } // namespace paimon::test From b91ee271316921cb0bacb1f424968e70d46d1f54 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 16 Dec 2025 16:53:16 +0800 Subject: [PATCH 3/7] fix --- src/paimon/fs/local/local_file.cpp | 53 +++++------------------ src/paimon/fs/local/local_file.h | 4 +- src/paimon/fs/local/local_file_system.cpp | 23 +++++----- src/paimon/fs/local/local_file_test.cpp | 28 +++++++----- 4 files changed, 40 insertions(+), 68 deletions(-) diff --git a/src/paimon/fs/local/local_file.cpp b/src/paimon/fs/local/local_file.cpp index 0e187ce8..2986c5b5 100644 --- a/src/paimon/fs/local/local_file.cpp +++ b/src/paimon/fs/local/local_file.cpp @@ -123,69 +123,43 @@ Status LocalFile::Delete() const { return Status::OK(); } -Status LocalFile::MkNestDir(const std::string& dir_name) const { +Result LocalFile::MkNestDir(const std::string& dir_name) const { CHECK_HOOK(); size_t pos = dir_name.rfind('/'); if (pos == std::string::npos) { - if (mkdir(dir_name.c_str(), 0755) < 0) { - if (errno != EEXIST) { - int32_t cur_errno = errno; - return Status::IOError(fmt::format("MkNestDir path '{}' fail, ec: {}", path_, - std::strerror(cur_errno))); - } - } - return Status::OK(); + return mkdir(dir_name.c_str(), 0755) == 0; } std::string parent_dir = dir_name.substr(0, pos); if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) { - PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir)); - } - - if (mkdir(dir_name.c_str(), 0755) < 0) { - if (errno != EEXIST) { - int32_t cur_errno = errno; - return Status::IOError( - fmt::format("MkNestDir path '{}' fail, ec: {}", path_, std::strerror(cur_errno))); + PAIMON_ASSIGN_OR_RAISE(bool success, MkNestDir(parent_dir)); + if (!success) { + return false; } } - return Status::OK(); + return mkdir(dir_name.c_str(), 0755) == 0; } -Status LocalFile::Mkdir() const { +Result LocalFile::Mkdir() const { CHECK_HOOK(); std::string dir = path_; size_t len = dir.size(); if (dir[len - 1] == '/') { if (len == 1) { - return Status::OK(); + return false; } else { dir.resize(len - 1); } } size_t pos = dir.rfind('/'); if (pos == std::string::npos) { - if (mkdir(dir.c_str(), 0755) < 0) { - if (errno != EEXIST) { - int32_t cur_errno = errno; - return Status::IOError( - fmt::format("Mkdir path '{}' fail, ec: {}", dir, std::strerror(cur_errno))); - } - } - return Status::OK(); + return mkdir(dir.c_str(), 0755) == 0; } std::string parent_dir = dir.substr(0, pos); if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) { PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir)); } - if (mkdir(dir.c_str(), 0755) < 0) { - if (errno != EEXIST) { - int32_t cur_errno = errno; - return Status::IOError( - fmt::format("create directory '{}' failed, ec: {}", dir, std::strerror(cur_errno))); - } - } - return Status::OK(); + return mkdir(dir.c_str(), 0755) == 0; } Result> LocalFile::GetFileStatus() const { @@ -350,13 +324,6 @@ Status LocalFile::OpenFile(bool is_read_file) { CHECK_HOOK(); file_ = fopen(path_.c_str(), "r"); } else { - LocalFile parent_dir = GetParentFile(); - if (!parent_dir.GetAbsolutePath().empty()) { - PAIMON_ASSIGN_OR_RAISE(bool is_exist, parent_dir.Exists()); - if (!is_exist) { - PAIMON_RETURN_NOT_OK(parent_dir.Mkdir()); - } - } CHECK_HOOK(); file_ = fopen(path_.c_str(), "w"); } diff --git a/src/paimon/fs/local/local_file.h b/src/paimon/fs/local/local_file.h index 73ce9ce5..804ee2c4 100644 --- a/src/paimon/fs/local/local_file.h +++ b/src/paimon/fs/local/local_file.h @@ -47,7 +47,7 @@ class LocalFile { Status Delete() const; const std::string& GetAbsolutePath() const; LocalFile GetParentFile() const; - Status Mkdir() const; + Result Mkdir() const; Result> GetFileStatus() const; Result Length() const; Result LastModifiedTimeMs() const; @@ -68,7 +68,7 @@ class LocalFile { } private: - Status MkNestDir(const std::string& dir_name) const; + Result MkNestDir(const std::string& dir_name) const; const std::string path_; FILE* file_ = nullptr; diff --git a/src/paimon/fs/local/local_file_system.cpp b/src/paimon/fs/local/local_file_system.cpp index 6dab2999..a220dc8a 100644 --- a/src/paimon/fs/local/local_file_system.cpp +++ b/src/paimon/fs/local/local_file_system.cpp @@ -90,7 +90,16 @@ Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const { } } - PAIMON_RETURN_NOT_OK(file.Mkdir()); + PAIMON_ASSIGN_OR_RAISE(bool success, file.Mkdir()); + if (!success) { + PAIMON_ASSIGN_OR_RAISE(bool is_dir, file.IsDir()); + if (is_dir) { + return Status::OK(); + } else { + return Status::IOError( + fmt::format("create directory '{}' failed", file.GetAbsolutePath())); + } + } return Status::OK(); } @@ -210,17 +219,7 @@ Status LocalFileSystem::Rename(const std::string& src, const std::string& dst) c } PAIMON_ASSIGN_OR_RAISE(LocalFile dst_file, ToFile(dst)); auto parent = dst_file.GetParentFile(); - if (!parent.GetAbsolutePath().empty()) { - PAIMON_ASSIGN_OR_RAISE(bool is_exist, parent.Exists()); - if (is_exist) { - // pass - } else { - Status status = parent.Mkdir(); - if (!status.ok() && !status.IsExist()) { - return status; - } - } - } + PAIMON_RETURN_NOT_OK(Mkdirs(parent.GetAbsolutePath())); if (::rename(src.c_str(), dst.c_str()) != 0) { int32_t cur_errno = errno; return Status::IOError(err_msg, std::strerror(cur_errno)); diff --git a/src/paimon/fs/local/local_file_test.cpp b/src/paimon/fs/local/local_file_test.cpp index 60cc022a..1e592b65 100644 --- a/src/paimon/fs/local/local_file_test.cpp +++ b/src/paimon/fs/local/local_file_test.cpp @@ -33,7 +33,8 @@ TEST(LocalFileTest, TestReadWriteEmptyContent) { if (dir.Exists().ok()) { ASSERT_TRUE(dir.Delete().ok()); } - ASSERT_TRUE(dir.Mkdir().ok()); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); std::string path = test_root + "/test.txt"; LocalFile file = LocalFile(path); if (file.Exists().ok()) { @@ -69,7 +70,8 @@ TEST(LocalFileTest, TestSimple) { if (dir.Exists().ok()) { ASSERT_OK(dir.Delete()); } - ASSERT_OK(dir.Mkdir()); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); std::string path = test_root + "/test.txt"; LocalFile file = LocalFile(path); if (file.Exists().ok()) { @@ -127,6 +129,9 @@ TEST(LocalFileTest, TestSimple) { ASSERT_EQ(strcmp(str_read, "test_data"), 0); } + ASSERT_OK_AND_ASSIGN(success, dir.Mkdir()); + ASSERT_FALSE(success); + ASSERT_OK(file2.Delete()); ASSERT_FALSE(file2.Exists().value()); } @@ -134,12 +139,14 @@ TEST(LocalFileTest, TestSimple) { TEST(LocalFileTest, TestUsage) { std::string test_root = "tmp/local_file_test_usage"; LocalFile dir = LocalFile(test_root); - ASSERT_OK(dir.Mkdir()); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); std::vector file_list; ASSERT_OK(dir.List(&file_list)); std::string path_deep_dir = test_root + "/tmp2/tmp3"; LocalFile deep_dir = LocalFile(path_deep_dir); - ASSERT_OK(deep_dir.Mkdir()); + ASSERT_OK_AND_ASSIGN(success, deep_dir.Mkdir()); + ASSERT_TRUE(success); LocalFile parent_deep_dir = deep_dir.GetParentFile(); ASSERT_EQ(parent_deep_dir.GetAbsolutePath(), test_root + "/tmp2"); ASSERT_OK(deep_dir.Delete()); @@ -155,7 +162,8 @@ TEST(LocalFileTest, TestOpenFile) { if (dir.Exists().ok()) { ASSERT_OK(dir.Delete()); } - ASSERT_OK(dir.Mkdir()); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); std::string path = test_root + "/test.txt"; LocalFile file = LocalFile(path); if (file.Exists().ok()) { @@ -167,10 +175,6 @@ TEST(LocalFileTest, TestOpenFile) { ASSERT_NOK_WITH_MSG(file.OpenFile(/*is_read_file=*/true), "file not exist"); ASSERT_NOK_WITH_MSG(dir.OpenFile(/*is_read_file=*/true), "cannot open a directory"); - std::string path2 = test_root + "/foo/test.txt"; - LocalFile file2 = LocalFile(path2); - ASSERT_OK(file2.OpenFile(/*is_read_file=*/false)); - std::string path3 = "test.txt"; LocalFile file3 = LocalFile(path3); ASSERT_OK(file3.OpenFile(/*is_read_file=*/false)); @@ -178,9 +182,11 @@ TEST(LocalFileTest, TestOpenFile) { ASSERT_GE(modify_time, -1); LocalFile dir2 = LocalFile("/"); - ASSERT_OK(dir2.Mkdir()); + ASSERT_OK_AND_ASSIGN(success, dir2.Mkdir()); + ASSERT_FALSE(success); LocalFile dir3 = LocalFile(test_root + "/"); - ASSERT_OK(dir3.Mkdir()); + ASSERT_OK_AND_ASSIGN(success, dir3.Mkdir()); + ASSERT_FALSE(success); } } // namespace paimon::test From 29acd084e8693c5c2919b854c9c2b2f95cd06e26 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 16 Dec 2025 17:46:22 +0800 Subject: [PATCH 4/7] fix --- src/paimon/fs/local/local_file.cpp | 11 ++--------- src/paimon/fs/local/local_file_system.cpp | 6 +++++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/paimon/fs/local/local_file.cpp b/src/paimon/fs/local/local_file.cpp index 2986c5b5..3e313ee8 100644 --- a/src/paimon/fs/local/local_file.cpp +++ b/src/paimon/fs/local/local_file.cpp @@ -151,15 +151,8 @@ Result LocalFile::Mkdir() const { dir.resize(len - 1); } } - size_t pos = dir.rfind('/'); - if (pos == std::string::npos) { - return mkdir(dir.c_str(), 0755) == 0; - } - std::string parent_dir = dir.substr(0, pos); - if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) { - PAIMON_RETURN_NOT_OK(MkNestDir(parent_dir)); - } - return mkdir(dir.c_str(), 0755) == 0; + PAIMON_ASSIGN_OR_RAISE(bool success, MkNestDir(dir)); + return success; } Result> LocalFile::GetFileStatus() const { diff --git a/src/paimon/fs/local/local_file_system.cpp b/src/paimon/fs/local/local_file_system.cpp index a220dc8a..73061eb1 100644 --- a/src/paimon/fs/local/local_file_system.cpp +++ b/src/paimon/fs/local/local_file_system.cpp @@ -76,7 +76,7 @@ Status LocalFileSystem::Mkdirs(const std::string& path) const { } Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const { - // Important: The 'Exists()' check above must come before the 'IsDirectory()' + // Important: The 'Exists()' check above must come before the 'IsDir()' // check to be safe when multiple parallel instances try to create the directory PAIMON_ASSIGN_OR_RAISE(bool is_exist, file.Exists()); if (is_exist) { @@ -90,6 +90,10 @@ Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const { } } + auto parent = file.GetParentFile(); + if (!parent.IsEmpty()) { + PAIMON_RETURN_NOT_OK(MkdirsInternal(parent)); + } PAIMON_ASSIGN_OR_RAISE(bool success, file.Mkdir()); if (!success) { PAIMON_ASSIGN_OR_RAISE(bool is_dir, file.IsDir()); From f4c376bd9c8ca515bc07cf65e96c453d06ca54ca Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 16 Dec 2025 18:48:41 +0800 Subject: [PATCH 5/7] fix --- src/paimon/common/utils/path_util.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/paimon/common/utils/path_util.h b/src/paimon/common/utils/path_util.h index 4605aa29..1d3c921c 100644 --- a/src/paimon/common/utils/path_util.h +++ b/src/paimon/common/utils/path_util.h @@ -39,6 +39,8 @@ class PAIMON_EXPORT PathUtil { ~PathUtil() = delete; static std::string JoinPath(const std::string& path, const std::string& name) noexcept; + // TODO(jinli.zjw): should pass `Path.path` and normalize; otherwise if path is + // "oss://bucket1/", GetParentDirPath will return "oss:" static std::string GetParentDirPath(const std::string& path) noexcept; static std::string GetName(const std::string& path) noexcept; static void TrimLastDelim(std::string* dir_path) noexcept; From 58215ef83375740bb12986153dd0ac9c78333847 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Wed, 17 Dec 2025 11:37:58 +0800 Subject: [PATCH 6/7] fix --- src/paimon/common/fs/file_system_test.cpp | 20 +++++------- src/paimon/fs/local/local_file.cpp | 29 +---------------- src/paimon/fs/local/local_file.h | 2 -- src/paimon/fs/local/local_file_test.cpp | 38 +++++++++++++++++++++-- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/paimon/common/fs/file_system_test.cpp b/src/paimon/common/fs/file_system_test.cpp index 56a6464b..097a2c83 100644 --- a/src/paimon/common/fs/file_system_test.cpp +++ b/src/paimon/common/fs/file_system_test.cpp @@ -979,18 +979,14 @@ TEST_P(FileSystemTest, TestMkdirsFailsWithExistingParentFile) { } TEST_P(FileSystemTest, TestMkdir) { - { - std::string path = PathUtil::JoinPath(test_root_, "/tmp.txt/tmpB"); - ASSERT_OK(fs_->Mkdirs(path)); - } - { - std::string path = PathUtil::JoinPath(test_root_, "/tmpA/tmpB/"); - ASSERT_OK(fs_->Mkdirs(path)); - } - { - std::string path = "/"; - ASSERT_OK(fs_->Mkdirs(path)); - } + ASSERT_OK(fs_->Mkdirs(test_root_ + "/tmp.txt/tmpB")); + ASSERT_OK(fs_->Mkdirs(test_root_ + "/tmpA/tmpB/")); + + ASSERT_OK(fs_->Mkdirs(test_root_ + "/tmp/local/f/1")); + ASSERT_OK(fs_->Mkdirs(test_root_ + "/tmp1")); + ASSERT_OK(fs_->Mkdirs(test_root_ + "/tmp1/f2/")); + ASSERT_OK(fs_->Mkdirs("/")); + ASSERT_NOK_WITH_MSG(fs_->Mkdirs(""), "path is an empty string."); } TEST_P(FileSystemTest, TestMkdir2) { diff --git a/src/paimon/fs/local/local_file.cpp b/src/paimon/fs/local/local_file.cpp index 3e313ee8..c9a13818 100644 --- a/src/paimon/fs/local/local_file.cpp +++ b/src/paimon/fs/local/local_file.cpp @@ -123,36 +123,9 @@ Status LocalFile::Delete() const { return Status::OK(); } -Result LocalFile::MkNestDir(const std::string& dir_name) const { - CHECK_HOOK(); - size_t pos = dir_name.rfind('/'); - if (pos == std::string::npos) { - return mkdir(dir_name.c_str(), 0755) == 0; - } - - std::string parent_dir = dir_name.substr(0, pos); - if (!parent_dir.empty() && access(parent_dir.c_str(), F_OK) != 0) { - PAIMON_ASSIGN_OR_RAISE(bool success, MkNestDir(parent_dir)); - if (!success) { - return false; - } - } - return mkdir(dir_name.c_str(), 0755) == 0; -} - Result LocalFile::Mkdir() const { CHECK_HOOK(); - std::string dir = path_; - size_t len = dir.size(); - if (dir[len - 1] == '/') { - if (len == 1) { - return false; - } else { - dir.resize(len - 1); - } - } - PAIMON_ASSIGN_OR_RAISE(bool success, MkNestDir(dir)); - return success; + return mkdir(path_.c_str(), 0755) == 0; } Result> LocalFile::GetFileStatus() const { diff --git a/src/paimon/fs/local/local_file.h b/src/paimon/fs/local/local_file.h index 804ee2c4..cb4b3532 100644 --- a/src/paimon/fs/local/local_file.h +++ b/src/paimon/fs/local/local_file.h @@ -68,8 +68,6 @@ class LocalFile { } private: - Result MkNestDir(const std::string& dir_name) const; - const std::string path_; FILE* file_ = nullptr; IOHook* hook_; diff --git a/src/paimon/fs/local/local_file_test.cpp b/src/paimon/fs/local/local_file_test.cpp index 1e592b65..66489456 100644 --- a/src/paimon/fs/local/local_file_test.cpp +++ b/src/paimon/fs/local/local_file_test.cpp @@ -129,6 +129,7 @@ TEST(LocalFileTest, TestSimple) { ASSERT_EQ(strcmp(str_read, "test_data"), 0); } + // dir already exists ASSERT_OK_AND_ASSIGN(success, dir.Mkdir()); ASSERT_FALSE(success); @@ -137,18 +138,18 @@ TEST(LocalFileTest, TestSimple) { } TEST(LocalFileTest, TestUsage) { - std::string test_root = "tmp/local_file_test_usage"; + std::string test_root = "local_file_test_usage"; LocalFile dir = LocalFile(test_root); ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); ASSERT_TRUE(success); std::vector file_list; ASSERT_OK(dir.List(&file_list)); - std::string path_deep_dir = test_root + "/tmp2/tmp3"; + std::string path_deep_dir = test_root + "/tmp2"; LocalFile deep_dir = LocalFile(path_deep_dir); ASSERT_OK_AND_ASSIGN(success, deep_dir.Mkdir()); ASSERT_TRUE(success); LocalFile parent_deep_dir = deep_dir.GetParentFile(); - ASSERT_EQ(parent_deep_dir.GetAbsolutePath(), test_root + "/tmp2"); + ASSERT_EQ(parent_deep_dir.GetAbsolutePath(), test_root); ASSERT_OK(deep_dir.Delete()); ASSERT_OK(parent_deep_dir.Delete()); ASSERT_OK(dir.Delete()); @@ -189,4 +190,35 @@ TEST(LocalFileTest, TestOpenFile) { ASSERT_FALSE(success); } +TEST(LocalFileTest, TestMkdir) { + auto test_root_dir = UniqueTestDirectory::Create(); + ASSERT_TRUE(test_root_dir); + std::string test_root = test_root_dir->Str(); + { + LocalFile dir = LocalFile(test_root + "tmp/local/f/1"); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_FALSE(success); + } + { + LocalFile dir = LocalFile(test_root + "tmp1"); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); + } + { + LocalFile dir = LocalFile(test_root + "tmp1/f2/"); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_TRUE(success); + } + { + LocalFile dir = LocalFile("/"); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_FALSE(success); + } + { + LocalFile dir = LocalFile(""); + ASSERT_OK_AND_ASSIGN(bool success, dir.Mkdir()); + ASSERT_FALSE(success); + } +} + } // namespace paimon::test From e811b17de46ea44c9b2e01f338ce87f0d9930945 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Wed, 17 Dec 2025 14:32:16 +0800 Subject: [PATCH 7/7] fix --- src/paimon/common/fs/file_system_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/paimon/common/fs/file_system_test.cpp b/src/paimon/common/fs/file_system_test.cpp index 097a2c83..ad9450c3 100644 --- a/src/paimon/common/fs/file_system_test.cpp +++ b/src/paimon/common/fs/file_system_test.cpp @@ -1014,7 +1014,7 @@ TEST_P(FileSystemTest, TestMkdir2) { } // test for create multi dir such as "/table/partition1/bucket1" and "/table/partition1/bucket2" -TEST_P(FileSystemTest, TestMkdirMultiThread) { +TEST_P(FileSystemTest, TestMkdirMultiThreadWithSameNonExistParentDir) { uint32_t runs_count = 10; uint32_t thread_count = 10; auto executor = CreateDefaultExecutor(thread_count); @@ -1039,7 +1039,7 @@ TEST_P(FileSystemTest, TestMkdirMultiThread) { } // test for create multi dir such as "/table/partition1" and "/table/partition1" -TEST_P(FileSystemTest, TestMkdirMultiThread2) { +TEST_P(FileSystemTest, TestMkdirMultiThreadWithSameName) { uint32_t runs_count = 10; uint32_t thread_count = 10; auto executor = CreateDefaultExecutor(thread_count); @@ -1061,7 +1061,7 @@ TEST_P(FileSystemTest, TestMkdirMultiThread2) { } // test for create multi dir such as "partition1" and "partition1" (relative path) -TEST_P(FileSystemTest, TestMkdirMultiThread3) { +TEST_P(FileSystemTest, TestMkdirMultiThreadWithSameNameWithRelativePath) { uint32_t runs_count = 10; uint32_t thread_count = 10; auto executor = CreateDefaultExecutor(thread_count);