diff --git a/src/Common/ZooKeeper/IKeeper.h b/src/Common/ZooKeeper/IKeeper.h index 369aacf16c7..2703c1079c0 100644 --- a/src/Common/ZooKeeper/IKeeper.h +++ b/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; }; diff --git a/src/Common/ZooKeeper/ZooKeeper.h b/src/Common/ZooKeeper/ZooKeeper.h index 03200771e4a..1fcb048add2 100644 --- a/src/Common/ZooKeeper/ZooKeeper.h +++ b/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_); diff --git a/src/Common/ZooKeeper/ZooKeeperImpl.h b/src/Common/ZooKeeper/ZooKeeperImpl.h index ae6bef067e3..44ea993947e 100644 --- a/src/Common/ZooKeeper/ZooKeeperImpl.h +++ b/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; diff --git a/src/Coordination/FourLetterCommand.cpp b/src/Coordination/FourLetterCommand.cpp index 10d13657fb0..34540902d47 100644 --- a/src/Coordination/FourLetterCommand.cpp +++ b/src/Coordination/FourLetterCommand.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -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) - append(SettingFieldKeeperFeatureFlagTraits::toString(feature), feature_flags.isEnabled(feature)); + for (const auto & [feature_flag, name] : magic_enum::enum_entries()) + { + std::string feature_flag_string(name); + boost::to_lower(feature_flag_string); + append(feature_flag_string, feature_flags.isEnabled(feature_flag)); + } return ret.str(); diff --git a/src/Coordination/KeeperContext.cpp b/src/Coordination/KeeperContext.cpp index e6f30c81310..e1c3a138646 100644 --- a/src/Coordination/KeeperContext.cpp +++ b/src/Coordination/KeeperContext.cpp @@ -1,10 +1,19 @@ #include #include #include +#include +#include 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(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(); diff --git a/src/Coordination/KeeperFeatureFlags.cpp b/src/Coordination/KeeperFeatureFlags.cpp index 216dca014d4..3aff87bcea9 100644 --- a/src/Coordination/KeeperFeatureFlags.cpp +++ b/src/Coordination/KeeperFeatureFlags.cpp @@ -23,15 +23,10 @@ std::pair 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() - 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()) { 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"); } } diff --git a/src/Coordination/KeeperFeatureFlags.h b/src/Coordination/KeeperFeatureFlags.h index cdd4704a7ca..6c48915f60c 100644 --- a/src/Coordination/KeeperFeatureFlags.h +++ b/src/Coordination/KeeperFeatureFlags.h @@ -1,28 +1,18 @@ #pragma once -#include -#include #include 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: diff --git a/src/Storages/System/StorageSystemZooKeeperConnection.cpp b/src/Storages/System/StorageSystemZooKeeperConnection.cpp index cd78ae01457..33268d58358 100644 --- a/src/Storages/System/StorageSystemZooKeeperConnection.cpp +++ b/src/Storages/System/StorageSystemZooKeeperConnection.cpp @@ -1,8 +1,11 @@ #include +#include +#include #include #include #include #include +#include #include 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()); + for (const auto & [feature_flag, feature_flag_string] : magic_enum::enum_entries()) + feature_flags_enum_values.push_back(std::pair{std::string{feature_flag_string}, static_cast(feature_flag)}); + + auto feature_flags_enum = std::make_shared(std::move(feature_flags_enum_values)); + return { {"name", std::make_shared()}, {"host", std::make_shared()}, @@ -19,7 +29,8 @@ NamesAndTypesList StorageSystemZooKeeperConnection::getNamesAndTypes() {"session_uptime_elapsed_seconds", std::make_shared()}, {"is_expired", std::make_shared()}, {"keeper_api_version", std::make_shared()}, - {"client_id", std::make_shared()} + {"client_id", std::make_shared()}, + {"enabled_feature_flags", std::make_shared(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()) + { + 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[5]->insert(elem.second->expired()); - res_columns[6]->insert(0); - res_columns[7]->insert(elem.second->getClientID()); + res_columns[4]->insert(elem.second->getConnectedTime()); + res_columns[5]->insert(elem.second->getSessionUptime()); + res_columns[6]->insert(elem.second->expired()); + res_columns[7]->insert(0); res_columns[8]->insert(elem.second->getClientID()); + add_enabled_feature_flags(elem.second); } } diff --git a/tests/integration/test_keeper_feature_flags_config/test.py b/tests/integration/test_keeper_feature_flags_config/test.py index bb7252e9ec8..93ac6cbd3bd 100644 --- a/tests/integration/test_keeper_feature_flags_config/test.py +++ b/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, ) diff --git a/tests/queries/0_stateless/02735_system_zookeeper_connection.reference b/tests/queries/0_stateless/02735_system_zookeeper_connection.reference index eddd4829829..380da27cde6 100644 --- a/tests/queries/0_stateless/02735_system_zookeeper_connection.reference +++ b/tests/queries/0_stateless/02735_system_zookeeper_connection.reference @@ -1,2 +1,2 @@ -default ::1 9181 0 0 0 1 1 -zookeeper2 ::1 9181 0 0 0 1 \ No newline at end of file +default ::1 9181 0 0 0 1 1 ['FILTERED_LIST','MULTI_READ','CHECK_NOT_EXISTS'] +zookeeper2 ::1 9181 0 0 0 1 diff --git a/tests/queries/0_stateless/02735_system_zookeeper_connection.sql b/tests/queries/0_stateless/02735_system_zookeeper_connection.sql index 863d90e1654..f999da51225 100644 --- a/tests/queries/0_stateless/02735_system_zookeeper_connection.sql +++ b/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