Merge #35113

programs/copier/ClusterCopierApp.cpp

@@ -1,4 +1,5 @@
 #include "ClusterCopierApp.h"
+#include <Common/ZooKeeper/ZooKeeper.h>
 #include <Common/StatusFile.h>
 #include <Common/TerminalSize.h>
 #include <IO/ConnectionTimeouts.h>
@@ -192,6 +193,8 @@ void ClusterCopierApp::mainImpl()
     if (!task_file.empty())
         copier->uploadTaskDescription(task_path, task_file, config().getBool("task-upload-force", false));
 
+    zkutil::validateZooKeeperConfig(config());
+
     copier->init();
     copier->process(ConnectionTimeouts::getTCPTimeoutsWithoutFailover(context->getSettingsRef()));
 

programs/extract-from-config/ExtractFromConfig.cpp

@@ -89,8 +89,12 @@ static std::vector<std::string> extractFromConfig(
     if (has_zk_includes && process_zk_includes)
     {
         DB::ConfigurationPtr bootstrap_configuration(new Poco::Util::XMLConfiguration(config_xml));
+
+        zkutil::validateZooKeeperConfig(*bootstrap_configuration);
+
         zkutil::ZooKeeperPtr zookeeper = std::make_shared<zkutil::ZooKeeper>(
-                *bootstrap_configuration, "zookeeper", nullptr);
+            *bootstrap_configuration, bootstrap_configuration->has("zookeeper") ? "zookeeper" : "keeper", nullptr);
+
         zkutil::ZooKeeperNodeCache zk_node_cache([&] { return zookeeper; });
         config_xml = processor.processConfig(&has_zk_includes, &zk_node_cache);
     }

programs/server/Server.cpp

@@ -815,7 +815,8 @@ try
         }
     );
 
-    bool has_zookeeper = config().has("zookeeper");
+    zkutil::validateZooKeeperConfig(config());
+    bool has_zookeeper = zkutil::hasZooKeeperConfig(config());
 
     zkutil::ZooKeeperNodeCache main_config_zk_node_cache([&] { return global_context->getZooKeeper(); });
     zkutil::EventPtr main_config_zk_changed_event = std::make_shared<Poco::Event>();
@@ -1307,7 +1308,7 @@ try
             {
                 /// We do not load ZooKeeper configuration on the first config loading
                 /// because TestKeeper server is not started yet.
-                if (config->has("zookeeper"))
+                if (zkutil::hasZooKeeperConfig(*config))
                     global_context->reloadZooKeeperIfChanged(config);
 
                 global_context->reloadAuxiliaryZooKeepersConfigIfChanged(config);

src/Common/ZooKeeper/ZooKeeper.cpp

@@ -29,6 +29,8 @@ namespace ErrorCodes
     extern const int LOGICAL_ERROR;
     extern const int NOT_IMPLEMENTED;
     extern const int BAD_ARGUMENTS;
+    extern const int NO_ELEMENTS_IN_CONFIG;
+    extern const int EXCESSIVE_ELEMENT_IN_CONFIG;
 }
 }
 
@@ -1340,4 +1342,29 @@ String getSequentialNodeName(const String & prefix, UInt64 number)
     return name;
 }
 
+void validateZooKeeperConfig(const Poco::Util::AbstractConfiguration & config)
+{
+    if (config.has("zookeeper") && config.has("keeper"))
+        throw DB::Exception(DB::ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG, "Both ZooKeeper and Keeper are specified");
+}
+
+bool hasZooKeeperConfig(const Poco::Util::AbstractConfiguration & config)
+{
+    return config.has("zookeeper") || config.has("keeper") || (config.has("keeper_server") && config.getBool("keeper_server.use_cluster", true));
+}
+
+String getZooKeeperConfigName(const Poco::Util::AbstractConfiguration & config)
+{
+    if (config.has("zookeeper"))
+        return "zookeeper";
+
+    if (config.has("keeper"))
+        return "keeper";
+
+    if (config.has("keeper_server") && config.getBool("keeper_server.use_cluster", true))
+        return "keeper_server";
+
+    throw DB::Exception(DB::ErrorCodes::NO_ELEMENTS_IN_CONFIG, "There is no Zookeeper configuration in server config");
+}
+
 }

src/Common/ZooKeeper/ZooKeeper.h

@@ -669,4 +669,10 @@ String extractZooKeeperPath(const String & path, bool check_starts_with_slash, P
 
 String getSequentialNodeName(const String & prefix, UInt64 number);
 
+void validateZooKeeperConfig(const Poco::Util::AbstractConfiguration & config);
+
+bool hasZooKeeperConfig(const Poco::Util::AbstractConfiguration & config);
+
+String getZooKeeperConfigName(const Poco::Util::AbstractConfiguration & config);
+
 }

src/Common/ZooKeeper/ZooKeeperArgs.cpp

@@ -18,6 +18,116 @@ namespace zkutil
 {
 
 ZooKeeperArgs::ZooKeeperArgs(const Poco::Util::AbstractConfiguration & config, const String & config_name)
+{
+    if (config_name == "keeper_server")
+        initFromKeeperServerSection(config);
+    else
+        initFromKeeperSection(config, config_name);
+
+    if (!chroot.empty())
+    {
+        if (chroot.front() != '/')
+            throw KeeperException(
+                Coordination::Error::ZBADARGUMENTS,
+                "Root path in config file should start with '/', but got {}", chroot);
+        if (chroot.back() == '/')
+            chroot.pop_back();
+    }
+
+    if (session_timeout_ms < 0 || operation_timeout_ms < 0 || connection_timeout_ms < 0)
+        throw KeeperException("Timeout cannot be negative", Coordination::Error::ZBADARGUMENTS);
+
+    /// init get_priority_load_balancing
+    get_priority_load_balancing.hostname_differences.resize(hosts.size());
+    const String & local_hostname = getFQDNOrHostName();
+    for (size_t i = 0; i < hosts.size(); ++i)
+    {
+        const String & node_host = hosts[i].substr(0, hosts[i].find_last_of(':'));
+        get_priority_load_balancing.hostname_differences[i] = DB::getHostNameDifference(local_hostname, node_host);
+    }
+}
+
+ZooKeeperArgs::ZooKeeperArgs(const String & hosts_string)
+{
+    splitInto<','>(hosts, hosts_string);
+}
+
+void ZooKeeperArgs::initFromKeeperServerSection(const Poco::Util::AbstractConfiguration & config)
+{
+    static constexpr std::string_view config_name = "keeper_server";
+
+    if (auto key = std::string{config_name} + ".tcp_port_secure";
+        config.has(key))
+    {
+        auto tcp_port_secure = config.getString(key);
+
+        if (tcp_port_secure.empty())
+            throw KeeperException("Empty tcp_port_secure in config file", Coordination::Error::ZBADARGUMENTS);
+    }
+
+    bool secure{false};
+    std::string tcp_port;
+    if (auto tcp_port_secure_key = std::string{config_name} + ".tcp_port_secure";
+        config.has(tcp_port_secure_key))
+    {
+        secure = true;
+        tcp_port = config.getString(tcp_port_secure_key);
+    }
+    else if (auto tcp_port_key = std::string{config_name} + ".tcp_port";
+        config.has(tcp_port_key))
+    {
+        tcp_port = config.getString(tcp_port_key);
+    }
+
+    if (tcp_port.empty())
+        throw KeeperException("No tcp_port or tcp_port_secure in config file", Coordination::Error::ZBADARGUMENTS);
+
+    if (auto coordination_key = std::string{config_name} + ".coordination_settings";
+        config.has(coordination_key))
+    {
+        if (auto operation_timeout_key = coordination_key + ".operation_timeout_ms";
+            config.has(operation_timeout_key))
+            operation_timeout_ms = config.getInt(operation_timeout_key);
+
+        if (auto session_timeout_key = coordination_key + ".session_timeout_ms";
+            config.has(session_timeout_key))
+            session_timeout_ms = config.getInt(session_timeout_key);
+    }
+
+    Poco::Util::AbstractConfiguration::Keys keys;
+    std::string raft_configuration_key = std::string{config_name} + ".raft_configuration";
+    config.keys(raft_configuration_key, keys);
+    for (const auto & key : keys)
+    {
+        if (startsWith(key, "server"))
+            hosts.push_back(
+                (secure ? "secure://" : "") + config.getString(raft_configuration_key + "." + key + ".hostname") + ":" + tcp_port);
+    }
+
+    static constexpr std::array load_balancing_keys
+    {
+        ".zookeeper_load_balancing",
+        ".keeper_load_balancing"
+    };
+
+    for (const auto * load_balancing_key : load_balancing_keys)
+    {
+        if (auto load_balancing_config = std::string{config_name} + load_balancing_key;
+            config.has(load_balancing_config))
+        {
+            String load_balancing_str = config.getString(load_balancing_config);
+            /// Use magic_enum to avoid dependency from dbms (`SettingFieldLoadBalancingTraits::fromString(...)`)
+            auto load_balancing = magic_enum::enum_cast<DB::LoadBalancing>(Poco::toUpper(load_balancing_str));
+            if (!load_balancing)
+                throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Unknown load balancing: {}", load_balancing_str);
+            get_priority_load_balancing.load_balancing = *load_balancing;
+            break;
+        }
+    }
+
+}
+
+void ZooKeeperArgs::initFromKeeperSection(const Poco::Util::AbstractConfiguration & config, const std::string & config_name)
 {
     Poco::Util::AbstractConfiguration::Keys keys;
     config.keys(config_name, keys);
@@ -84,7 +194,7 @@ ZooKeeperArgs::ZooKeeperArgs(const Poco::Util::AbstractConfiguration & config, c
         {
             implementation = config.getString(config_name + "." + key);
         }
-        else if (key == "zookeeper_load_balancing")
+        else if (key == "zookeeper_load_balancing" || key == "keeper_load_balancing")
         {
             String load_balancing_str = config.getString(config_name + "." + key);
             /// Use magic_enum to avoid dependency from dbms (`SettingFieldLoadBalancingTraits::fromString(...)`)
@@ -96,33 +206,6 @@ ZooKeeperArgs::ZooKeeperArgs(const Poco::Util::AbstractConfiguration & config, c
         else
             throw KeeperException(std::string("Unknown key ") + key + " in config file", Coordination::Error::ZBADARGUMENTS);
     }
-
-    if (!chroot.empty())
-    {
-        if (chroot.front() != '/')
-            throw KeeperException(
-                Coordination::Error::ZBADARGUMENTS,
-                "Root path in config file should start with '/', but got {}", chroot);
-        if (chroot.back() == '/')
-            chroot.pop_back();
-    }
-
-    if (session_timeout_ms < 0 || operation_timeout_ms < 0 || connection_timeout_ms < 0)
-        throw KeeperException("Timeout cannot be negative", Coordination::Error::ZBADARGUMENTS);
-
-    /// init get_priority_load_balancing
-    get_priority_load_balancing.hostname_differences.resize(hosts.size());
-    const String & local_hostname = getFQDNOrHostName();
-    for (size_t i = 0; i < hosts.size(); ++i)
-    {
-        const String & node_host = hosts[i].substr(0, hosts[i].find_last_of(':'));
-        get_priority_load_balancing.hostname_differences[i] = DB::getHostNameDifference(local_hostname, node_host);
-    }
-}
-
-ZooKeeperArgs::ZooKeeperArgs(const String & hosts_string)
-{
-    splitInto<','>(hosts, hosts_string);
 }
 
 }

src/Common/ZooKeeper/ZooKeeperArgs.h

@@ -37,6 +37,10 @@ struct ZooKeeperArgs
     UInt64 recv_sleep_ms = 0;
 
     DB::GetPriorityForLoadBalancing get_priority_load_balancing;
+
+private:
+    void initFromKeeperServerSection(const Poco::Util::AbstractConfiguration & config);
+    void initFromKeeperSection(const Poco::Util::AbstractConfiguration & config, const std::string & config_name);
 };
 
 }

src/Interpreters/Context.cpp

@@ -2362,7 +2362,7 @@ zkutil::ZooKeeperPtr Context::getZooKeeper() const
 
     const auto & config = shared->zookeeper_config ? *shared->zookeeper_config : getConfigRef();
     if (!shared->zookeeper)
-        shared->zookeeper = std::make_shared<zkutil::ZooKeeper>(config, "zookeeper", getZooKeeperLog());
+        shared->zookeeper = std::make_shared<zkutil::ZooKeeper>(config, zkutil::getZooKeeperConfigName(config), getZooKeeperLog());
     else if (shared->zookeeper->expired())
     {
         Stopwatch watch;
@@ -2401,8 +2401,9 @@ bool Context::tryCheckClientConnectionToMyKeeperCluster() const
 {
     try
     {
+        const auto config_name = zkutil::getZooKeeperConfigName(getConfigRef());
         /// If our server is part of main Keeper cluster
-        if (checkZooKeeperConfigIsLocal(getConfigRef(), "zookeeper"))
+        if (config_name == "keeper_server" || checkZooKeeperConfigIsLocal(getConfigRef(), config_name))
         {
             LOG_DEBUG(shared->log, "Keeper server is participant of the main zookeeper cluster, will try to connect to it");
             getZooKeeper();
@@ -2608,7 +2609,7 @@ void Context::reloadZooKeeperIfChanged(const ConfigurationPtr & config) const
     bool server_started = isServerCompletelyStarted();
     std::lock_guard lock(shared->zookeeper_mutex);
     shared->zookeeper_config = config;
-    reloadZooKeeperIfChangedImpl(config, "zookeeper", shared->zookeeper, getZooKeeperLog(), server_started);
+    reloadZooKeeperIfChangedImpl(config, zkutil::getZooKeeperConfigName(*config), shared->zookeeper, getZooKeeperLog(), server_started);
 }
 
 void Context::reloadAuxiliaryZooKeepersConfigIfChanged(const ConfigurationPtr & config)
@@ -2633,7 +2634,7 @@ void Context::reloadAuxiliaryZooKeepersConfigIfChanged(const ConfigurationPtr &
 
 bool Context::hasZooKeeper() const
 {
-    return getConfigRef().has("zookeeper");
+    return zkutil::hasZooKeeperConfig(getConfigRef());
 }
 
 bool Context::hasAuxiliaryZooKeeper(const String & name) const

src/Storages/examples/get_abandonable_lock_in_all_partitions.cpp

@@ -26,7 +26,7 @@ try
     auto config = processor.loadConfig().configuration;
     String root_path = argv[2];
 
-    zkutil::ZooKeeper zk(*config, "zookeeper", nullptr);
+    zkutil::ZooKeeper zk(*config, zkutil::getZooKeeperConfigName(*config), nullptr);
 
     String temp_path = root_path + "/temp";
     String blocks_path = root_path + "/block_numbers";

src/Storages/examples/get_current_inserts_in_replicated.cpp

@@ -29,7 +29,7 @@ try
     auto config = processor.loadConfig().configuration;
     String zookeeper_path = argv[2];
 
-    auto zookeeper = std::make_shared<zkutil::ZooKeeper>(*config, "zookeeper", nullptr);
+    auto zookeeper = std::make_shared<zkutil::ZooKeeper>(*config, zkutil::getZooKeeperConfigName(*config), nullptr);
 
     std::unordered_map<String, std::set<Int64>> current_inserts;
 

tests/integration/test_alternative_keeper_config/configs/enable_keeper1.xml

@@ -0,0 +1,41 @@
+<clickhouse>
+    <keeper_server>
+        <tcp_port>9181</tcp_port>
+        <server_id>1</server_id>
+        <log_storage_path>/var/lib/clickhouse/coordination/log</log_storage_path>
+        <snapshot_storage_path>/var/lib/clickhouse/coordination/snapshots</snapshot_storage_path>
+
+        <coordination_settings>
+            <operation_timeout_ms>5000</operation_timeout_ms>
+            <session_timeout_ms>10000</session_timeout_ms>
+            <snapshot_distance>75</snapshot_distance>
+            <raft_logs_level>trace</raft_logs_level>
+        </coordination_settings>
+
+        <raft_configuration>
+            <server>
+                <id>1</id>
+                <hostname>node1</hostname>
+                <port>9234</port>
+                <can_become_leader>true</can_become_leader>
+                <priority>3</priority>
+            </server>
+            <server>
+                <id>2</id>
+                <hostname>node2</hostname>
+                <port>9234</port>
+                <can_become_leader>true</can_become_leader>
+                <start_as_follower>true</start_as_follower>
+                <priority>2</priority>
+            </server>
+            <server>
+                <id>3</id>
+                <hostname>node3</hostname>
+                <port>9234</port>
+                <can_become_leader>true</can_become_leader>
+                <start_as_follower>true</start_as_follower>
+                <priority>3</priority>
+            </server>
+        </raft_configuration>
+    </keeper_server>
+</clickhouse>