Use magic_enum for values

src/Common/ZooKeeper/IKeeper.h

@@ -532,6 +532,8 @@ public:
 
     virtual bool isFeatureEnabled(DB::KeeperFeatureFlag feature_flag) const = 0;
 
+    virtual const DB::KeeperFeatureFlags * getKeeperFeatureFlags() const { return nullptr; }
+
     /// Expire session and finish all pending requests
     virtual void finalize(const String & reason) = 0;
 };

src/Common/ZooKeeper/ZooKeeper.h

@@ -529,6 +529,8 @@ public:
     size_t getConnectedZooKeeperIndex() const { return connected_zk_index; }
     UInt64 getConnectedTime() const { return connected_time; }
 
+    const DB::KeeperFeatureFlags * getKeeperFeatureFlags() const { return impl->getKeeperFeatureFlags(); }
+
 private:
     void init(ZooKeeperArgs args_);
 

src/Common/ZooKeeper/ZooKeeperImpl.h

@@ -202,6 +202,8 @@ public:
 
     void setServerCompletelyStarted();
 
+    const KeeperFeatureFlags * getKeeperFeatureFlags() const override { return &keeper_feature_flags; }
+
 private:
     ACLs default_acls;
     Poco::Net::SocketAddress connected_zk_address;

src/Coordination/FourLetterCommand.cpp

@@ -13,6 +13,7 @@
 #include <Coordination/Keeper4LWInfo.h>
 #include <IO/WriteHelpers.h>
 #include <IO/Operators.h>
+#include <boost/algorithm/string.hpp>
 
 #include <unistd.h>
 #include <bit>
@@ -545,7 +546,7 @@ String FeatureFlagsCommand::run()
 
     StringBuffer ret;
 
-    auto append = [&ret] (String key, uint8_t value) -> void
+    auto append = [&ret] (const String & key, uint8_t value) -> void
     {
         writeText(key, ret);
         writeText('\t', ret);
@@ -553,8 +554,12 @@ String FeatureFlagsCommand::run()
         writeText('\n', ret);
     };
 
-    for (const auto feature : all_keeper_feature_flags)
+    for (const auto & [feature_flag, name] : magic_enum::enum_entries<KeeperFeatureFlag>())
-        append(SettingFieldKeeperFeatureFlagTraits::toString(feature), feature_flags.isEnabled(feature));
+    {
+        std::string feature_flag_string(name);
+        boost::to_lower(feature_flag_string);
+        append(feature_flag_string, feature_flags.isEnabled(feature_flag));
+    }
 
     return ret.str();
 

src/Coordination/KeeperContext.cpp

@@ -1,10 +1,19 @@
 #include <Coordination/KeeperContext.h>
 #include <Coordination/KeeperConstants.h>
 #include <Common/logger_useful.h>
+#include <Coordination/KeeperFeatureFlags.h>
+#include <boost/algorithm/string.hpp>
 
 namespace DB
 {
 
+namespace ErrorCodes
+{
+
+extern const int BAD_ARGUMENTS;
+
+}
+
 KeeperContext::KeeperContext()
 {
     /// enable by default some feature flags
@@ -29,12 +38,17 @@ void KeeperContext::initialize(const Poco::Util::AbstractConfiguration & config)
         config.keys(feature_flags_key, keys);
         for (const auto & key : keys)
         {
-            auto feature_flag = SettingFieldKeeperFeatureFlagTraits::fromString(key);
+            auto feature_flag_string = boost::to_upper_copy(key);
+            auto feature_flag = magic_enum::enum_cast<KeeperFeatureFlag>(feature_flag_string);
+
+            if (!feature_flag.has_value())
+                throw Exception(ErrorCodes::BAD_ARGUMENTS, "Invalid feature flag defined in config for Keeper: {}", key);
+
             auto is_enabled = config.getBool(feature_flags_key + "." + key);
             if (is_enabled)
-                feature_flags.enableFeatureFlag(feature_flag);
+                feature_flags.enableFeatureFlag(feature_flag.value());
             else
-                feature_flags.disableFeatureFlag(feature_flag);
+                feature_flags.disableFeatureFlag(feature_flag.value());
         }
 
         system_nodes_with_data[keeper_api_feature_flags_path] = feature_flags.getFeatureFlags();

src/Coordination/KeeperFeatureFlags.cpp

@@ -23,15 +23,10 @@ std::pair<size_t, size_t> getByteAndBitIndex(size_t num)
 
 }
 
-IMPLEMENT_SETTING_ENUM(KeeperFeatureFlag, ErrorCodes::BAD_ARGUMENTS,
-     {{"filtered_list", KeeperFeatureFlag::FILTERED_LIST},
-     {"multi_read", KeeperFeatureFlag::MULTI_READ},
-     {"check_not_exists", KeeperFeatureFlag::CHECK_NOT_EXISTS}});
-
 KeeperFeatureFlags::KeeperFeatureFlags()
 {
     /// get byte idx of largest value
-    auto [byte_idx, _] = getByteAndBitIndex(all_keeper_feature_flags.size() - 1);
+    auto [byte_idx, _] = getByteAndBitIndex(magic_enum::enum_count<KeeperFeatureFlag>() - 1);
     feature_flags = std::string(byte_idx + 1, 0);
 }
 
@@ -56,7 +51,7 @@ void KeeperFeatureFlags::fromApiVersion(KeeperApiVersion keeper_api_version)
 
 bool KeeperFeatureFlags::isEnabled(KeeperFeatureFlag feature_flag) const
 {
-    auto [byte_idx, bit_idx] = getByteAndBitIndex(feature_flag);
+    auto [byte_idx, bit_idx] = getByteAndBitIndex(magic_enum::enum_integer(feature_flag));
 
     if (byte_idx > feature_flags.size())
         return false;
@@ -71,7 +66,7 @@ void KeeperFeatureFlags::setFeatureFlags(std::string feature_flags_)
 
 void KeeperFeatureFlags::enableFeatureFlag(KeeperFeatureFlag feature_flag)
 {
-    auto [byte_idx, bit_idx] = getByteAndBitIndex(feature_flag);
+    auto [byte_idx, bit_idx] = getByteAndBitIndex(magic_enum::enum_integer(feature_flag));
     chassert(byte_idx < feature_flags.size());
 
     feature_flags[byte_idx] |= (1 << bit_idx);
@@ -79,7 +74,7 @@ void KeeperFeatureFlags::enableFeatureFlag(KeeperFeatureFlag feature_flag)
 
 void KeeperFeatureFlags::disableFeatureFlag(KeeperFeatureFlag feature_flag)
 {
-    auto [byte_idx, bit_idx] = getByteAndBitIndex(feature_flag);
+    auto [byte_idx, bit_idx] = getByteAndBitIndex(magic_enum::enum_integer(feature_flag));
     chassert(byte_idx < feature_flags.size());
 
     feature_flags[byte_idx] &= ~(1 << bit_idx);
@@ -92,10 +87,10 @@ const std::string & KeeperFeatureFlags::getFeatureFlags() const
 
 void KeeperFeatureFlags::logFlags(Poco::Logger * log) const
 {
-    for (const auto & feature_flag : all_keeper_feature_flags)
+    for (const auto & [feature_flag, feature_flag_name] : magic_enum::enum_entries<KeeperFeatureFlag>())
     {
         auto is_enabled = isEnabled(feature_flag);
-        LOG_INFO(log, "Keeper feature flag {}: {}", SettingFieldKeeperFeatureFlagTraits::toString(feature_flag), is_enabled ? "enabled" : "disabled");
+        LOG_INFO(log, "Keeper feature flag {}: {}", feature_flag_name, is_enabled ? "enabled" : "disabled");
     }
 }
 

src/Coordination/KeeperFeatureFlags.h

@@ -1,28 +1,18 @@
 #pragma once
 
-#include <Core/SettingsEnums.h>
-#include <Core/SettingsFields.h>
 #include <Coordination/KeeperConstants.h>
 
 namespace DB
 {
 
-enum KeeperFeatureFlag
+/// these values cannot be reordered or removed, only new values can be added
+enum class KeeperFeatureFlag : size_t
 {
     FILTERED_LIST = 0,
     MULTI_READ,
     CHECK_NOT_EXISTS,
 };
 
-static inline constexpr std::array all_keeper_feature_flags
-{
-    KeeperFeatureFlag::FILTERED_LIST,
-    KeeperFeatureFlag::MULTI_READ,
-    KeeperFeatureFlag::CHECK_NOT_EXISTS,
-};
-
-DECLARE_SETTING_ENUM(KeeperFeatureFlag);
-
 class KeeperFeatureFlags
 {
 public:

src/Storages/System/StorageSystemZooKeeperConnection.cpp

@@ -1,8 +1,11 @@
 #include <Interpreters/Context.h>
+#include <DataTypes/DataTypeArray.h>
+#include <DataTypes/DataTypeEnum.h>
 #include <DataTypes/DataTypeString.h>
 #include <DataTypes/DataTypesNumber.h>
 #include <DataTypes/DataTypeDateTime.h>
 #include <Common/ZooKeeper/ZooKeeper.h>
+#include <Coordination/KeeperFeatureFlags.h>
 #include <Storages/System/StorageSystemZooKeeperConnection.h>
 
 namespace DB
@@ -10,6 +13,13 @@ namespace DB
 
 NamesAndTypesList StorageSystemZooKeeperConnection::getNamesAndTypes()
 {
+    DataTypeEnum16::Values feature_flags_enum_values;
+    feature_flags_enum_values.reserve(magic_enum::enum_count<KeeperFeatureFlag>());
+    for (const auto & [feature_flag, feature_flag_string] : magic_enum::enum_entries<KeeperFeatureFlag>())
+        feature_flags_enum_values.push_back(std::pair{std::string{feature_flag_string}, static_cast<Int16>(feature_flag)});
+
+    auto feature_flags_enum = std::make_shared<DataTypeEnum16>(std::move(feature_flags_enum_values));
+
     return {
         {"name", std::make_shared<DataTypeString>()},
         {"host", std::make_shared<DataTypeString>()},
@@ -19,7 +29,8 @@ NamesAndTypesList StorageSystemZooKeeperConnection::getNamesAndTypes()
         {"session_uptime_elapsed_seconds", std::make_shared<DataTypeUInt64>()},
         {"is_expired", std::make_shared<DataTypeUInt8>()},
         {"keeper_api_version", std::make_shared<DataTypeUInt8>()},
-        {"client_id", std::make_shared<DataTypeInt64>()}
+        {"client_id", std::make_shared<DataTypeInt64>()},
+        {"enabled_feature_flags", std::make_shared<DataTypeArray>(std::move(feature_flags_enum))}
     };
 }
 
@@ -36,17 +47,37 @@ void StorageSystemZooKeeperConnection::fillData(MutableColumns & res_columns, Co
     res_columns[7]->insert(0);
     res_columns[8]->insert(context->getZooKeeper()->getClientID());
 
+    const auto add_enabled_feature_flags = [&](const auto & zookeeper)
+    {
+        Array enabled_feature_flags;
+        const auto * feature_flags = zookeeper->getKeeperFeatureFlags();
+        if (feature_flags)
+        {
+            for (const auto & feature_flag : magic_enum::enum_values<KeeperFeatureFlag>())
+            {
+                if (feature_flags->isEnabled(feature_flag))
+                {
+                    enabled_feature_flags.push_back(feature_flag);
+                }
+            }
+        }
+        res_columns[9]->insert(std::move(enabled_feature_flags));
+    };
+
+    add_enabled_feature_flags(context->getZooKeeper());
+
     for (const auto & elem : context->getAuxiliaryZooKeepers())
     {
         res_columns[0]->insert(elem.first);
         res_columns[1]->insert(elem.second->getConnectedZooKeeperHost());
         res_columns[2]->insert(elem.second->getConnectedZooKeeperPort());
         res_columns[3]->insert(elem.second->getConnectedZooKeeperIndex());
-        res_columns[4]->insert(elem.second->getSessionUptime());
+        res_columns[4]->insert(elem.second->getConnectedTime());
-        res_columns[5]->insert(elem.second->expired());
+        res_columns[5]->insert(elem.second->getSessionUptime());
-        res_columns[6]->insert(0);
+        res_columns[6]->insert(elem.second->expired());
-        res_columns[7]->insert(elem.second->getClientID());
+        res_columns[7]->insert(0);
         res_columns[8]->insert(elem.second->getClientID());
+        add_enabled_feature_flags(elem.second);
     }
 
 }

tests/integration/test_keeper_feature_flags_config/test.py

@@ -69,12 +69,12 @@ def test_keeper_feature_flags(started_cluster):
 
         for feature, is_enabled in feature_flags:
             node.wait_for_log_line(
-                f"ZooKeeperClient: Keeper feature flag {feature}: {'enabled' if is_enabled else 'disabled'}",
+                f"ZooKeeperClient: Keeper feature flag {feature.upper()}: {'enabled' if is_enabled else 'disabled'}",
                 look_behind_lines=1000,
             )
 
             node.wait_for_log_line(
-                f"KeeperContext: Keeper feature flag {feature}: {'enabled' if is_enabled else 'disabled'}",
+                f"KeeperContext: Keeper feature flag {feature.upper()}: {'enabled' if is_enabled else 'disabled'}",
                 look_behind_lines=1000,
             )
 

tests/queries/0_stateless/02735_system_zookeeper_connection.reference

@@ -1,2 +1,2 @@
-default	::1	9181	0	0	0	1	1
+default	::1	9181	0	0	0	1	1	['FILTERED_LIST','MULTI_READ','CHECK_NOT_EXISTS']
 zookeeper2	::1	9181	0	0	0	1

tests/queries/0_stateless/02735_system_zookeeper_connection.sql

@@ -9,7 +9,7 @@ ENGINE ReplicatedMergeTree('zookeeper2:/clickhouse/{database}/02731_zk_connectio
 ORDER BY tuple();
 
 select name, host, port, index, is_expired, keeper_api_version, (connected_time between yesterday() and now()),
-       (abs(session_uptime_elapsed_seconds  - zookeeperSessionUptime()) < 10)
+       (abs(session_uptime_elapsed_seconds  - zookeeperSessionUptime()) < 10), enabled_feature_flags
 from system.zookeeper_connection where name='default';
 
 -- keeper_api_version will by 0 for auxiliary_zookeeper2, because we fail to get /api_version due to chroot