Merge pull request #64986 from ClickHouse/fix_init_order

Fix initialization order (ServerUUID/ZooKeeper)

programs/server/Server.cpp

@@ -983,6 +983,18 @@ try
         }
     }
 
+    std::string path_str = getCanonicalPath(config().getString("path", DBMS_DEFAULT_PATH));
+    fs::path path = path_str;
+
+    /// Check that the process user id matches the owner of the data.
+    assertProcessUserMatchesDataOwner(path_str, [&](const std::string & message){ global_context->addWarningMessage(message); });
+
+    global_context->setPath(path_str);
+
+    StatusFile status{path / "status", StatusFile::write_full_info};
+
+    ServerUUID::load(path / "uuid", log);
+
     zkutil::validateZooKeeperConfig(config());
     bool has_zookeeper = zkutil::hasZooKeeperConfig(config());
 
@@ -994,7 +1006,7 @@ try
         ConfigProcessor config_processor(config_path);
         loaded_config = config_processor.loadConfigWithZooKeeperIncludes(
             main_config_zk_node_cache, main_config_zk_changed_event, /* fallback_to_preprocessed = */ true);
-        config_processor.savePreprocessedConfig(loaded_config, config().getString("path", DBMS_DEFAULT_PATH));
+        config_processor.savePreprocessedConfig(loaded_config, path_str);
         config().removeConfiguration(old_configuration.get());
         config().add(loaded_config.configuration.duplicate(), PRIO_DEFAULT, false);
         global_context->setConfig(loaded_config.configuration);
@@ -1128,19 +1140,6 @@ try
     global_context->setRemoteHostFilter(config());
     global_context->setHTTPHeaderFilter(config());
 
-    std::string path_str = getCanonicalPath(config().getString("path", DBMS_DEFAULT_PATH));
-    fs::path path = path_str;
-    std::string default_database = server_settings.default_database.toString();
-
-    /// Check that the process user id matches the owner of the data.
-    assertProcessUserMatchesDataOwner(path_str, [&](const std::string & message){ global_context->addWarningMessage(message); });
-
-    global_context->setPath(path_str);
-
-    StatusFile status{path / "status", StatusFile::write_full_info};
-
-    ServerUUID::load(path / "uuid", log);
-
     /// Try to increase limit on number of open files.
     {
         rlimit rlim;
@@ -1932,6 +1931,7 @@ try
 
     /// Set current database name before loading tables and databases because
     /// system logs may copy global context.
+    std::string default_database = server_settings.default_database.toString();
     global_context->setCurrentDatabaseNameInGlobalContext(default_database);
 
     LOG_INFO(log, "Loading metadata from {}", path_str);

src/Coordination/Standalone/Context.h

@@ -130,7 +130,8 @@ public:
 
     enum class ApplicationType : uint8_t
     {
-        KEEPER
+        KEEPER,
+        SERVER,
     };
 
     void setApplicationType(ApplicationType) {}

src/Core/ServerUUID.cpp

@@ -1,4 +1,5 @@
 #include <Core/ServerUUID.h>
+#include <Interpreters/Context.h>
 #include <IO/ReadBufferFromFile.h>
 #include <IO/WriteBufferFromFile.h>
 #include <IO/ReadHelpers.h>
@@ -11,6 +12,16 @@ namespace DB
 namespace ErrorCodes
 {
     extern const int CANNOT_CREATE_FILE;
+    extern const int LOGICAL_ERROR;
+}
+
+UUID ServerUUID::get()
+{
+    if (server_uuid == UUIDHelpers::Nil &&
+        (Context::getGlobalContextInstance()->getApplicationType() == Context::ApplicationType::SERVER ||
+         Context::getGlobalContextInstance()->getApplicationType() == Context::ApplicationType::KEEPER))
+        throw Exception(ErrorCodes::LOGICAL_ERROR, "ServerUUID is not initialized yet");
+    return server_uuid;
 }
 
 void ServerUUID::load(const fs::path & server_uuid_file, Poco::Logger * log)
@@ -57,4 +68,9 @@ UUID loadServerUUID(const fs::path & server_uuid_file, Poco::Logger * log)
     }
 }
 
+void ServerUUID::setRandomForUnitTests()
+{
+    server_uuid = UUIDHelpers::generateV4();
+}
+
 }

src/Core/ServerUUID.h

@@ -15,10 +15,12 @@ class ServerUUID
 
 public:
     /// Returns persistent UUID of current clickhouse-server or clickhouse-keeper instance.
-    static UUID get() { return server_uuid; }
+    static UUID get();
 
     /// Loads server UUID from file or creates new one. Should be called on daemon startup.
     static void load(const fs::path & server_uuid_file, Poco::Logger * log);
+
+    static void setRandomForUnitTests();
 };
 
 UUID loadServerUUID(const fs::path & server_uuid_file, Poco::Logger * log);

src/Interpreters/tests/gtest_filecache.cpp

@@ -8,6 +8,7 @@
 #include <numeric>
 #include <thread>
 
+#include <Core/ServerUUID.h>
 #include <Common/iota.h>
 #include <Common/randomSeed.h>
 #include <DataTypes/DataTypesNumber.h>
@@ -333,6 +334,7 @@ public:
 
 TEST_F(FileCacheTest, LRUPolicy)
 {
+    ServerUUID::setRandomForUnitTests();
     DB::ThreadStatus thread_status;
 
     /// To work with cache need query_id and query context.
@@ -807,6 +809,7 @@ TEST_F(FileCacheTest, LRUPolicy)
 
 TEST_F(FileCacheTest, writeBuffer)
 {
+    ServerUUID::setRandomForUnitTests();
     FileCacheSettings settings;
     settings.max_size = 100;
     settings.max_elements = 5;
@@ -938,6 +941,7 @@ static size_t readAllTemporaryData(TemporaryFileStream & stream)
 
 TEST_F(FileCacheTest, temporaryData)
 {
+    ServerUUID::setRandomForUnitTests();
     DB::FileCacheSettings settings;
     settings.max_size = 10_KiB;
     settings.max_file_segment_size = 1_KiB;
@@ -1044,6 +1048,7 @@ TEST_F(FileCacheTest, temporaryData)
 
 TEST_F(FileCacheTest, CachedReadBuffer)
 {
+    ServerUUID::setRandomForUnitTests();
     DB::ThreadStatus thread_status;
 
     /// To work with cache need query_id and query context.
@@ -1120,6 +1125,7 @@ TEST_F(FileCacheTest, CachedReadBuffer)
 
 TEST_F(FileCacheTest, TemporaryDataReadBufferSize)
 {
+    ServerUUID::setRandomForUnitTests();
     /// Temporary data stored in cache
     {
         DB::FileCacheSettings settings;
@@ -1167,6 +1173,7 @@ TEST_F(FileCacheTest, TemporaryDataReadBufferSize)
 
 TEST_F(FileCacheTest, SLRUPolicy)
 {
+    ServerUUID::setRandomForUnitTests();
     DB::ThreadStatus thread_status;
     std::string query_id = "query_id"; /// To work with cache need query_id and query context.
 

tests/queries/0_stateless/02922_server_exit_code.sh

@@ -7,6 +7,6 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 
 # We will check that the server's exit code corresponds to the exception code if it was terminated after exception.
 # In this example, we provide an invalid path to the server's config, ignore its logs and check the exit code.
-# The exception code is 400 = CANNOT_STAT, so the exit code will be 400 % 256.
+# The exception code is 76 = CANNOT_OPEN_FILE, so the exit code will be 76 % 256.
 
-${CLICKHOUSE_SERVER_BINARY} -- --path /dev/null 2>/dev/null; [[ "$?" == "$((400 % 256))" ]] && echo 'Ok' || echo 'Fail'
+${CLICKHOUSE_SERVER_BINARY} -- --path /dev/null 2>/dev/null; [[ "$?" == "$((76 % 256))" ]] && echo 'Ok' || echo 'Fail'