[fix](chmod) change chmod to filesystem::permission to avoid race con…

…dition #31032 (#31033) bp #31032
apache · Feb 17, 2024 · df6a7a9 · df6a7a9
1 parent 07d30d4
commit df6a7a9
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 7 deletions.
diff --git a/be/src/io/fs/local_file_system.cpp b/be/src/io/fs/local_file_system.cpp
@@ -24,6 +24,9 @@
 namespace doris {
 namespace io {
 
+std::filesystem::perms LocalFileSystem::PERMS_OWNER_RW =
+        std::filesystem::perms::owner_read | std::filesystem::perms::owner_write;
+
 LocalFileSystem::LocalFileSystem(Path root_path, ResourceId resource_id)
         : FileSystem(std::move(root_path), std::move(resource_id), FileSystemType::LOCAL) {}
 
@@ -142,9 +145,20 @@ Status LocalFileSystem::list(const Path& path, std::vector<Path>* files) {
     return Status::OK();
 }
 
-static FileSystemSPtr local_fs = std::make_shared<io::LocalFileSystem>("");
+Status LocalFileSystem::permission(const Path& file, std::filesystem::perms prms) {
+    auto path = absolute_path(file);
+    std::error_code ec;
+    std::filesystem::permissions(file, prms, ec);
+    if (ec) {
+        return Status::IOError("failed to change file permission {}: {}", file.native(),
+                               std::strerror(ec.value()));
+    }
+    return Status::OK();
+}
+
+static std::shared_ptr<LocalFileSystem> local_fs = std::make_shared<io::LocalFileSystem>("");
 
-FileSystemSPtr global_local_filesystem() {
+const std::shared_ptr<LocalFileSystem>& global_local_filesystem() {
     return local_fs;
 }
 

diff --git a/be/src/io/fs/local_file_system.h b/be/src/io/fs/local_file_system.h
@@ -46,11 +46,16 @@ class LocalFileSystem final : public FileSystem {
 
     Status list(const Path& path, std::vector<Path>* files) override;
 
+    // change the file permission of the given path
+    Status permission(const Path& file, std::filesystem::perms prms);
+
+    static std::filesystem::perms PERMS_OWNER_RW;
+
 private:
     Path absolute_path(const Path& path) const;
 };
 
-FileSystemSPtr global_local_filesystem();
+const std::shared_ptr<LocalFileSystem>& global_local_filesystem();
 
 } // namespace io
 } // namespace doris
diff --git a/be/src/olap/task/engine_clone_task.cpp b/be/src/olap/task/engine_clone_task.cpp
@@ -430,8 +430,8 @@ Status EngineCloneTask::_download_files(DataDir* data_dir, const std::string& re
                              << ", local_file_size=" << local_file_size;
                 return Status::InternalError("downloaded file size is not equal");
             }
-            chmod(local_file_path.c_str(), S_IRUSR | S_IWUSR);
-            return Status::OK();
+            return io::global_local_filesystem()->permission(local_file_path,
+                                                             io::LocalFileSystem::PERMS_OWNER_RW);
         };
         RETURN_IF_ERROR(HttpClient::execute_with_retry(DOWNLOAD_FILE_MAX_RETRY, 1, download_cb));
     } // Clone files from remote backend

diff --git a/be/src/service/internal_service.cpp b/be/src/service/internal_service.cpp
@@ -1164,8 +1164,8 @@ void PInternalServiceImpl::request_slave_tablet_pull_rowset(
                             << ", local_file_size=" << local_file_size;
                     return Status::InternalError("downloaded file size is not equal");
                 }
-                chmod(local_file_path.c_str(), S_IRUSR | S_IWUSR);
-                return Status::OK();
+                return io::global_local_filesystem()->permission(
+                        local_file_path, io::LocalFileSystem::PERMS_OWNER_RW);
             };
             auto st = HttpClient::execute_with_retry(DOWNLOAD_FILE_MAX_RETRY, 1, download_cb);
             if (!st.ok()) {