Refactor directory access check and add config system.lock.enable (#3354)

* refactor directory access and add config system.lock.enable
Signed-off-by: NWang Xiangyu <xy.wang@zilliz.com>

* update namespace
Signed-off-by: NWang Xiangyu <xy.wang@zilliz.com>

core/src/config/ConfigMgr.cpp

@@ -179,6 +179,9 @@ ConfigMgr::ConfigMgr() {
                              &config.engine.omp_thread_num.value, 0, nullptr, nullptr)},
         {"engine.simd_type", CreateEnumConfig("engine.simd_type", false, &SimdMap, &config.engine.simd_type.value,
                                               SimdType::AUTO, nullptr, nullptr)},
+
+        {"system.lock.enable",
+         CreateBoolConfig("system.lock.enable", false, &config.system.lock.enable.value, true, nullptr, nullptr)},
     };
 }
 

core/src/config/ConfigMgr.h

@@ -24,8 +24,7 @@ namespace milvus {
 
 class ConfigObserver {
  public:
-    virtual ~ConfigObserver() {
-    }
+    virtual ~ConfigObserver() = default;
 
     virtual void
     ConfigUpdate(const std::string& name) = 0;

core/src/config/ServerConfig.h

@@ -148,6 +148,12 @@ struct ServerConfig {
         Integer max_log_file_size{0};
         Integer log_rotate_num{0};
     } logs;
+
+    struct System {
+        struct Lock {
+            Bool enable{false};
+        } lock;
+    } system;
 };
 
 extern ServerConfig config;

core/src/server/Server.cpp

@@ -27,7 +27,6 @@
 #include "server/init/CpuChecker.h"
 #include "server/init/Directory.h"
 #include "server/init/GpuChecker.h"
-#include "server/init/StorageChecker.h"
 #include "server/init/Timezone.h"
 #include "server/web_impl/WebServer.h"
 #include "src/version.h"
@@ -38,8 +37,7 @@
 #include "utils/SignalHandler.h"
 #include "utils/TimeRecorder.h"
 
-namespace milvus {
-namespace server {
+namespace milvus::server {
 
 Server&
 Server::GetInstance() {
@@ -164,17 +162,20 @@ Server::Start() {
                                      config.logs.max_log_file_size(), config.logs.log_rotate_num()));
 
         auto wal_path = config.wal.enable() ? config.wal.path() : "";
-        STATUS_CHECK(Directory::Initialize(config.storage.path(), wal_path));
+        STATUS_CHECK(Directory::Initialize(config.storage.path(), wal_path, config.logs.path()));
 
-        /* TODO: add a invisible config */
-        if (true) {
-            bool cluster_enable = config.cluster.enable();
-            auto cluster_role = config.cluster.role();
+        std::cout << "Running on "
+                  << RunningMode(config.cluster.enable(), static_cast<ClusterRole>(config.cluster.role())) << " mode."
+                  << std::endl;
+        auto is_read_only = config.cluster.enable() && config.cluster.role() == ClusterRole::RO;
 
-            std::cout << "Running on " + RunningMode(cluster_enable, (ClusterRole)cluster_role) + " mode." << std::endl;
+        /* Only read-only mode do NOT lock directories */
+        if (is_read_only) {
+            STATUS_CHECK(Directory::Access("", "", config.logs.path()));
+        } else {
+            STATUS_CHECK(Directory::Access(config.storage.path(), config.wal.path(), config.logs.path()));
 
-            /* Only read-only mode do not lock directories */
-            if ((not cluster_enable) || cluster_role == ClusterRole::RW) {
+            if (config.system.lock.enable()) {
                 STATUS_CHECK(Directory::Lock(config.storage.path(), wal_path));
             }
         }
@@ -186,7 +187,6 @@ Server::Start() {
 #else
         LOG_SERVER_INFO_ << "CPU edition";
 #endif
-        STATUS_CHECK(StorageChecker::CheckStoragePermission());
         STATUS_CHECK(CpuChecker::CheckCpuInstructionSet());
 #ifdef MILVUS_GPU_VERSION
         STATUS_CHECK(GpuChecker::CheckGpuEnvironment());
@@ -332,5 +332,4 @@ Server::LogCpuInfo() {
     LOG_SERVER_INFO_ << "\n\n" << std::string(15, '*') << "CPU" << std::string(15, '*') << "\n\n" << sub_str;
 }
 
-}  // namespace server
-}  // namespace milvus
+}  // namespace milvus::server

core/src/server/Server.h

@@ -15,8 +15,7 @@
 #include "config/ConfigMgr.h"
 #include "utils/Status.h"
 
-namespace milvus {
-namespace server {
+namespace milvus::server {
 
 class Server {
  public:
@@ -61,5 +60,4 @@ class Server {
     // ConfigMgrPtr config_mgr_;
 };  // Server
 
-}  // namespace server
-}  // namespace milvus
+}  // namespace milvus::server

core/src/server/init/Directory.cpp

@@ -21,12 +21,11 @@
 
 namespace milvus::server {
 Status
-Directory::Initialize(const std::string& storage_path, const std::string& wal_path) {
+Directory::Initialize(const std::string& storage_path, const std::string& wal_path, const std::string& log_path) {
     try {
         init(storage_path);
-        if (not wal_path.empty()) {
-            init(wal_path);
-        }
+        init(wal_path);
+        init(log_path);
     } catch (std::exception& ex) {
         return Status(SERVER_UNEXPECTED_ERROR, ex.what());
     }
@@ -37,9 +36,19 @@ Status
 Directory::Lock(const std::string& storage_path, const std::string& wal_path) {
     try {
         lock(storage_path);
-        if (not wal_path.empty()) {
-            lock(wal_path);
-        }
+        lock(wal_path);
+    } catch (std::exception& ex) {
+        return Status(SERVER_UNEXPECTED_ERROR, ex.what());
+    }
+    return Status::OK();
+}
+
+Status
+Directory::Access(const std::string& storage_path, const std::string& wal_path, const std::string& log_path) {
+    try {
+        access_check(storage_path);
+        access_check(log_path);
+        access_check(wal_path);
     } catch (std::exception& ex) {
         return Status(SERVER_UNEXPECTED_ERROR, ex.what());
     }
@@ -48,6 +57,9 @@ Directory::Lock(const std::string& storage_path, const std::string& wal_path) {
 
 void
 Directory::init(const std::string& path) {
+    if (path.empty()) {
+        return;
+    }
     try {
         // Returns True if a new directory was created, otherwise false.
         boost::filesystem::create_directories(path);
@@ -62,11 +74,14 @@ Directory::init(const std::string& path) {
 
 void
 Directory::lock(const std::string& path) {
+    if (path.empty()) {
+        return;
+    }
     std::string lock_path = path + "/lock";
     auto fd = open(lock_path.c_str(), O_RDWR | O_CREAT | O_NOFOLLOW, 0640);
     fiu_do_on("Directory.lock.fd", fd = -1);
     if (fd < 0) {
-        std::string msg = "Cannot lock file: " + lock_path + ", reason: ";
+        std::string msg = "Cannot lock file: " + lock_path + ", error(" + std::to_string(errno) + "): ";
         if (errno == EROFS) {
             // Not using locking for read-only lock file
             msg += "Lock file is read-only.";
@@ -86,7 +101,7 @@ Directory::lock(const std::string& path) {
     auto fcl = fcntl(fd, F_SETLK, &fl);
     fiu_do_on("Directory.lock.fcntl", fcl = -1);
     if (fcl == -1) {
-        std::string msg = "Cannot lock file: " + lock_path + ", reason: ";
+        std::string msg = "Cannot lock file: " + lock_path + ", error(" + std::to_string(errno) + "): ";
         if (errno == EACCES || errno == EAGAIN) {
             msg += "Permission denied.";
         } else if (errno == ENOLCK) {
@@ -100,4 +115,18 @@ Directory::lock(const std::string& path) {
     }
 }
 
+void
+Directory::access_check(const std::string& path) {
+    if (path.empty()) {
+        return;
+    }
+    int ret = access(path.c_str(), F_OK | R_OK | W_OK);
+    fiu_do_on("Directory.access_check.path_access_fail", ret = -1);
+    if (0 != ret) {
+        std::string msg = "Cannot access path: " + path + ", error(" + std::to_string(errno) +
+                          "): " + std::string(strerror(errno)) + ".";
+        throw std::runtime_error(msg);
+    }
+}
+
 }  // namespace milvus::server

core/src/server/init/Directory.h

@@ -20,17 +20,23 @@ namespace milvus::server {
 class Directory {
  public:
     static Status
-    Initialize(const std::string& storage_path, const std::string& wal_path);
+    Initialize(const std::string& storage_path, const std::string& wal_path, const std::string& log_path);
 
     static Status
     Lock(const std::string& storage_path, const std::string& wal_path);
 
+    static Status
+    Access(const std::string& storage_path, const std::string& wal_path, const std::string& log_path);
+
  private:
     static void
     init(const std::string& path);
 
     static void
     lock(const std::string& path);
+
+    static void
+    access_check(const std::string& path);
 };
 
 }  // namespace milvus::server

core/src/server/init/StorageChecker.cpp

-// Copyright (C) 2019-2020 Zilliz. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software distributed under the License
-// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
-// or implied. See the License for the specific language governing permissions and limitations under the License.
-
-#include "server/init/StorageChecker.h"
-
-#include <unistd.h>
-
-#include <string>
-#include <vector>
-
-#include <fiu/fiu-local.h>
-
-#include "config/ServerConfig.h"
-#include "utils/Log.h"
-#include "utils/StringHelpFunctions.h"
-
-namespace milvus {
-namespace server {
-
-Status
-StorageChecker::CheckStoragePermission() {
-    /* Check log file write permission */
-    const std::string& logs_path = config.logs.path();
-    int ret = access(logs_path.c_str(), F_OK | R_OK | W_OK);
-    fiu_do_on("StorageChecker.CheckStoragePermission.logs_path_access_fail", ret = -1);
-    if (0 != ret) {
-        std::string err_msg =
-            " Access log path " + logs_path + " fail. " + strerror(errno) + "(code: " + std::to_string(errno) + ")";
-        LOG_SERVER_FATAL_ << err_msg;
-        std::cerr << err_msg << std::endl;
-        return Status(SERVER_UNEXPECTED_ERROR, err_msg);
-    }
-
-    if (config.cluster.enable() && config.cluster.role() == ClusterRole::RO) {
-        return Status::OK();
-    }
-
-    /* Check db directory write permission */
-    const std::string& primary_path = config.storage.path();
-
-    ret = access(primary_path.c_str(), F_OK | R_OK | W_OK);
-    fiu_do_on("StorageChecker.CheckStoragePermission.db_primary_path_access_fail", ret = -1);
-    if (0 != ret) {
-        std::string err_msg = " Access DB storage path " + primary_path + " fail. " + strerror(errno) +
-                              "(code: " + std::to_string(errno) + ")";
-        LOG_SERVER_FATAL_ << err_msg;
-        std::cerr << err_msg << std::endl;
-        return Status(SERVER_UNEXPECTED_ERROR, err_msg);
-    }
-
-    /* Check wal directory write permission */
-    if (config.wal.enable()) {
-        const std::string& wal_path = config.wal.path();
-
-        ret = access(wal_path.c_str(), F_OK | R_OK | W_OK);
-        fiu_do_on("StorageChecker.CheckStoragePermission.wal_path_access_fail", ret = -1);
-        if (0 != ret) {
-            std::string err_msg = " Access WAL storage path " + wal_path + " fail. " + strerror(errno) +
-                                  "(code: " + std::to_string(errno) + ")";
-            LOG_SERVER_FATAL_ << err_msg;
-            std::cerr << err_msg << std::endl;
-            return Status(SERVER_UNEXPECTED_ERROR, err_msg);
-        }
-    }
-
-    return Status::OK();
-}
-
-}  // namespace server
-}  // namespace milvus

core/src/server/init/StorageChecker.h

-// Copyright (C) 2019-2020 Zilliz. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software distributed under the License
-// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
-// or implied. See the License for the specific language governing permissions and limitations under the License.
-
-#pragma once
-
-#include "utils/Status.h"
-
-namespace milvus {
-namespace server {
-
-class StorageChecker {
- public:
-    static Status
-    CheckStoragePermission();
-};
-
-}  // namespace server
-}  // namespace milvus