From 8b432b7f16f4c42adae538f47c01677d1c49aaf8 Mon Sep 17 00:00:00 2001 From: antonyzhilin Date: Tue, 28 Apr 2026 13:31:50 +0300 Subject: [PATCH 1/8] cc engine: add tests for critical task cancellation commit_hash:3ff5e3b9ee94fc59a4c232b6d26a392288f02c9e --- core/src/engine/task/cancel_test.cpp | 61 ++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/core/src/engine/task/cancel_test.cpp b/core/src/engine/task/cancel_test.cpp index 7375af463d5a..2e0714b1db0c 100644 --- a/core/src/engine/task/cancel_test.cpp +++ b/core/src/engine/task/cancel_test.cpp @@ -9,10 +9,12 @@ #include #include #include +#include #include #include #include #include +#include using namespace std::chrono_literals; @@ -435,4 +437,63 @@ UTEST(Cancel, ParentCancelledSample) { EXPECT_FALSE(deadline.IsReached()); } +UTEST(Async, CancellationBeforeStartNormal) { + // The test relies on there only being a single TaskProcessor thread, otherwise there could be a race + // between `RequestCancel()` and the concurrent execution of the child task. + ASSERT_EQ(GetThreadCount(), 1); + + bool captures_destroyed = false; + utils::FastScopeGuard guard([&captures_destroyed]() noexcept { + EXPECT_TRUE(engine::current_task::IsTaskProcessorThread()); + captures_destroyed = true; + }); + + auto task = engine::AsyncNoSpan([guard = std::move(guard)] { ADD_FAILURE() << "The task body should not start"; }); + + EXPECT_FALSE(task.IsFinished()); + EXPECT_FALSE(captures_destroyed); + + // The task has not started by this point. This means that the task is cancelled before starting. + // This should cause the task body to be skipped, because this is a non-Critical task. + // See details on engine::TaskBase::State::kCancelled. + task.RequestCancel(); + + task.WaitFor(utest::kMaxTestWaitTime); + EXPECT_EQ(task.GetState(), engine::TaskBase::State::kCancelled); + UEXPECT_THROW(task.Get(), engine::TaskCancelledException); + // The task destructor should run even if the execution of the task body is skipped due to cancellation. + EXPECT_TRUE(captures_destroyed); +} + +UTEST(Async, CancellationBeforeStartCritical) { + // The test relies on there only being a single TaskProcessor thread, otherwise there could be a race + // between `RequestCancel()` and the concurrent execution of the child task. + ASSERT_EQ(GetThreadCount(), 1); + + engine::SingleUseEvent event; + auto task = engine::CriticalAsyncNoSpan([&event] { + // The task should start (proven by `return true;` below and `Get() -> true`.) + + EXPECT_TRUE(engine::current_task::impl::IsCritical()); + + // The task is still cancelled, and any waiting operation will be interrupted. + EXPECT_TRUE(engine::current_task::IsCancelRequested()); + EXPECT_TRUE(engine::current_task::ShouldCancel()); + const auto deadline = engine::Deadline::FromDuration(utest::kMaxTestWaitTime); + EXPECT_EQ(event.WaitUntil(deadline), engine::FutureStatus::kCancelled); + + return true; + }); + + EXPECT_FALSE(task.IsFinished()); + + // The task has not started by this point. This means that the task is cancelled before starting. + // But because the task is started as Critical, the task body should be executed anyway. + task.RequestCancel(); + + task.WaitFor(utest::kMaxTestWaitTime / 2); + EXPECT_EQ(task.GetState(), engine::TaskBase::State::kCompleted); + EXPECT_TRUE(task.Get()); +} + USERVER_NAMESPACE_END From 9a0dc8eeaa7909c03db6add3514502f6a601de3c Mon Sep 17 00:00:00 2001 From: Fedor Shatokhin Date: Tue, 28 Apr 2026 14:55:44 +0300 Subject: [PATCH 2/8] fix chaotic: fixup chrono milliseconds convert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests: протестировано CI --- Pull Request resolved: https://github.com/userver-framework/userver/pull/1195 Co-authored-by: antoshkka commit_hash:9a73d342ab276357c57b27707df398165dbff73c --- .../userver/chaotic/io/std/chrono/milliseconds.hpp | 3 +++ .../integration_tests/schemas/custom_cpp_type.yaml | 3 +++ chaotic/integration_tests/tests/render/custom.cpp | 12 ++++++++++++ chaotic/src/chaotic/io/std/chrono/milliseconds.cpp | 6 ++++++ 4 files changed, 24 insertions(+) diff --git a/chaotic/include/userver/chaotic/io/std/chrono/milliseconds.hpp b/chaotic/include/userver/chaotic/io/std/chrono/milliseconds.hpp index e78ed2fdd31a..de573ee59426 100644 --- a/chaotic/include/userver/chaotic/io/std/chrono/milliseconds.hpp +++ b/chaotic/include/userver/chaotic/io/std/chrono/milliseconds.hpp @@ -14,10 +14,13 @@ USERVER_NAMESPACE_BEGIN namespace chaotic::convert { template +requires std::is_integral_v || std::is_floating_point_v T Convert(std::chrono::milliseconds value, chaotic::convert::To) { return utils::numeric_cast(value.count()); } +std::string Convert(const std::chrono::milliseconds& value, chaotic::convert::To); + std::chrono::milliseconds Convert(const std::string& str, chaotic::convert::To); std::chrono::milliseconds Convert(std::string_view str, chaotic::convert::To); diff --git a/chaotic/integration_tests/schemas/custom_cpp_type.yaml b/chaotic/integration_tests/schemas/custom_cpp_type.yaml index e8545b6498e4..5cf6f5fe34f5 100644 --- a/chaotic/integration_tests/schemas/custom_cpp_type.yaml +++ b/chaotic/integration_tests/schemas/custom_cpp_type.yaml @@ -28,6 +28,9 @@ definitions: integer: type: integer x-usrv-cpp-type: std::chrono::milliseconds + ms: + type: string + x-usrv-cpp-type: std::chrono::milliseconds sizet: type: integer minimum: 0 diff --git a/chaotic/integration_tests/tests/render/custom.cpp b/chaotic/integration_tests/tests/render/custom.cpp index bb7b8fd71dad..f55c8021c187 100644 --- a/chaotic/integration_tests/tests/render/custom.cpp +++ b/chaotic/integration_tests/tests/render/custom.cpp @@ -22,6 +22,18 @@ TEST(Custom, Int) { EXPECT_EQ(custom2, custom); } +TEST(Custom, Ms) { + auto json = formats::json::MakeObject("ms", "12ms"); + auto custom = json.As(); + EXPECT_EQ(custom.ms, std::chrono::milliseconds(12)); + + auto json_back = formats::json::ValueBuilder{custom}.ExtractValue(); + EXPECT_EQ(json_back, json); + + const auto custom2 = FromJsonString(ToString(json), formats::parse::To{}); + EXPECT_EQ(custom2, custom); +} + TEST(Custom, String) { auto json = formats::json::MakeObject("string", "make love"); auto custom = json.As(); diff --git a/chaotic/src/chaotic/io/std/chrono/milliseconds.cpp b/chaotic/src/chaotic/io/std/chrono/milliseconds.cpp index c632f546619b..1b05f5618fe7 100644 --- a/chaotic/src/chaotic/io/std/chrono/milliseconds.cpp +++ b/chaotic/src/chaotic/io/std/chrono/milliseconds.cpp @@ -2,10 +2,16 @@ #include +#include + USERVER_NAMESPACE_BEGIN namespace chaotic::convert { +std::string Convert(const std::chrono::milliseconds& value, chaotic::convert::To) { + return fmt::format("{}ms", value.count()); +} + std::chrono::milliseconds Convert(const std::string& str, chaotic::convert::To) { return std::chrono::milliseconds{utils::StringToDuration(str)}; } From d9bb339e5b242fe7990b936a6242318b14373651 Mon Sep 17 00:00:00 2001 From: antonyzhilin Date: Tue, 28 Apr 2026 20:36:48 +0300 Subject: [PATCH 3/8] cc rabbitmq: fix ubuntu tests commit_hash:cf262ceb5de32a8336ad10cd0d94dcc79e18da4f --- scripts/rabbitmq/ubuntu_install_rabbitmq_server.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/rabbitmq/ubuntu_install_rabbitmq_server.sh b/scripts/rabbitmq/ubuntu_install_rabbitmq_server.sh index 35e5895ca136..25a7a627b43c 100755 --- a/scripts/rabbitmq/ubuntu_install_rabbitmq_server.sh +++ b/scripts/rabbitmq/ubuntu_install_rabbitmq_server.sh @@ -31,3 +31,10 @@ mkdir /tmp/fake && ln -s /bin/true/ /tmp/fake/initctl && \ ln -s /bin/true /tmp/fake/service && \ ln -s /bin/true /tmp/fake/deb-systemd-helper sudo PATH=/tmp/fake:$PATH apt install -y rabbitmq-server + +# rabbitmq gives false positive errors even if this feature is not in use: +# https://github.com/rabbitmq/rabbitmq-server/issues/12802 +sudo mkdir -p /etc/rabbitmq/conf.d +sudo tee /etc/rabbitmq/conf.d/10-userver-ci.conf <<'EOF' +deprecated_features.permit.transient_nonexcl_queues = true +EOF From 10da6511bb5e1a585529a289f19addada3e513db Mon Sep 17 00:00:00 2001 From: antonyzhilin Date: Tue, 28 Apr 2026 21:09:58 +0300 Subject: [PATCH 4/8] cc rabbitmq: fix macos tests commit_hash:de8ad0052ef76890a95460adbba75eea8e9c466d --- .github/workflows/ci-conan.yml | 2 ++ .github/workflows/macos.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/ci-conan.yml b/.github/workflows/ci-conan.yml index 2f872f391e6f..b36df76d0dd8 100644 --- a/.github/workflows/ci-conan.yml +++ b/.github/workflows/ci-conan.yml @@ -53,6 +53,8 @@ jobs: brew update brew tap mongodb/brew brew install clang-format postgresql redis kafka rabbitmq mongodb-community + mkdir -p "$(brew --prefix)/etc/rabbitmq/conf.d" + echo 'deprecated_features.permit.transient_nonexcl_queues = true' > "$(brew --prefix)/etc/rabbitmq/conf.d/10-userver-ci.conf" brew install python@3.11 - name: Checkout diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index a714b89113ab..f03532c7806d 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -76,6 +76,8 @@ jobs: run: | brew tap mongodb/brew brew install clickhouse redis kafka mongodb-community rabbitmq + mkdir -p "$(brew --prefix)/etc/rabbitmq/conf.d" + echo 'deprecated_features.permit.transient_nonexcl_queues = true' > "$(brew --prefix)/etc/rabbitmq/conf.d/10-userver-ci.conf" - name: Setup ccache run: | From 22dbc4b49617b9fdbdf4580466c5fa5902fde22d Mon Sep 17 00:00:00 2001 From: Artyom Kolpakov Date: Wed, 29 Apr 2026 00:06:25 +0300 Subject: [PATCH 5/8] fix universal: use std::addressof in FindOrNullptr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/userver-framework/userver/issues/1184 Tests: протестировано CI --- Pull Request resolved: https://github.com/userver-framework/userver/pull/1196 Co-authored-by: antoshkka Co-authored-by: antoshkka Co-authored-by: antoshkka commit_hash:94c5fac1d860840ea0d5a6f3df5e81070c1ad450 --- .mapping.json | 1 + universal/include/userver/utils/algo.hpp | 5 ++-- universal/src/utils/algo_test.cpp | 14 ++++++++++ .../overloaded_address_operator_test.hpp | 27 +++++++++++++++++++ 4 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 universal/src/utils/overloaded_address_operator_test.hpp diff --git a/.mapping.json b/.mapping.json index 1fd496bffc77..e1ce84f75f15 100644 --- a/.mapping.json +++ b/.mapping.json @@ -6006,6 +6006,7 @@ "universal/src/utils/not_null_test.cpp":"taxi/uservices/userver/universal/src/utils/not_null_test.cpp", "universal/src/utils/numeric_cast_test.cpp":"taxi/uservices/userver/universal/src/utils/numeric_cast_test.cpp", "universal/src/utils/optional_ref_test.cpp":"taxi/uservices/userver/universal/src/utils/optional_ref_test.cpp", + "universal/src/utils/overloaded_address_operator_test.hpp":"taxi/uservices/userver/universal/src/utils/overloaded_address_operator_test.hpp", "universal/src/utils/overloaded_test.cpp":"taxi/uservices/userver/universal/src/utils/overloaded_test.cpp", "universal/src/utils/projected_set_test.cpp":"taxi/uservices/userver/universal/src/utils/projected_set_test.cpp", "universal/src/utils/rand.cpp":"taxi/uservices/userver/universal/src/utils/rand.cpp", diff --git a/universal/include/userver/utils/algo.hpp b/universal/include/userver/utils/algo.hpp index 032fa3a54ec0..56126c441b2f 100644 --- a/universal/include/userver/utils/algo.hpp +++ b/universal/include/userver/utils/algo.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -46,9 +47,9 @@ template auto* FindOrNullptr(Container& container, const Key& key) { const auto it = container.find(key); if constexpr (impl::HasMappedType) { - return (it != container.end() ? &(it->second) : nullptr); + return (it != container.end() ? std::addressof(it->second) : nullptr); } else { - return (it != container.end() ? &(*it) : nullptr); + return (it != container.end() ? std::addressof(*it) : nullptr); } } diff --git a/universal/src/utils/algo_test.cpp b/universal/src/utils/algo_test.cpp index 28d1005e9d33..149cc0779f84 100644 --- a/universal/src/utils/algo_test.cpp +++ b/universal/src/utils/algo_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "overloaded_address_operator_test.hpp" + #include USERVER_NAMESPACE_BEGIN @@ -46,6 +48,18 @@ TEST(UtilsAlgo, FindOrNullptrSets) { EXPECT_EQ(*utils::FindOrNullptr(us, "1"), "1"); } +TEST(UtilsAlgo, FindOrNullptrMapAddressof) { + const std::map m{{"1", {.payload = 1}}}; + const auto ptr = utils::FindOrNullptr(m, "1"); + ASSERT_TRUE(ptr); +} + +TEST(UtilsAlgo, FindOrNullptrSetAddressof) { + const std::set s{{.payload = 1}}; + const auto ptr = utils::FindOrNullptr(s, utils::OverloadedAddressOperator{.payload = 1}); + ASSERT_TRUE(ptr); +} + TEST(UtilsAlgo, FindOrDefaultMaps) { constexpr int kFallback = 42; std::map m = {{"1", 2}}; diff --git a/universal/src/utils/overloaded_address_operator_test.hpp b/universal/src/utils/overloaded_address_operator_test.hpp new file mode 100644 index 000000000000..8e56430da18c --- /dev/null +++ b/universal/src/utils/overloaded_address_operator_test.hpp @@ -0,0 +1,27 @@ +#pragma once + +#include + +USERVER_NAMESPACE_BEGIN + +namespace utils { + +struct OverloadedAddressOperator { + int payload{0}; + + auto operator<=>(const OverloadedAddressOperator& other) const { return payload <=> other.payload; }; + + auto operator&() { // NOLINT(google-runtime-operator) + UASSERT(false); + return this; + } + + auto operator&() const { // NOLINT(google-runtime-operator) + UASSERT(false); + return this; + } +}; + +} // namespace utils + +USERVER_NAMESPACE_END From 4661929e725c2ff06af2ea376c984ad55c58c10a Mon Sep 17 00:00:00 2001 From: vaninvv Date: Wed, 29 Apr 2026 11:06:51 +0300 Subject: [PATCH 6/8] feat redis: add CLUSTER SHARDS option Adding support for CLUSTER SHARDS to determine topology instead of CLUSTER SLOTS. Unlike cluster slots, it includes unavailable hosts (marked with "health": "failed") in the list of shards. This is important for determining the source of the problem: whether we cannot connect to the host, or the host is unavailable and the cluster itself is aware of it. commit_hash:2fccb9fbebd76ec9093937b77b4f89c1635ead20 --- .mapping.json | 13 + .../CMakeLists.txt | 6 + .../redis_service.cpp | 140 +++ .../static_config.yaml | 79 ++ .../tests/conftest.py | 36 + .../tests/test_redis_cluster.py | 91 ++ .../configs/cluster.conf | 2 +- .../storages/redis/topology_update_method.hpp | 15 + redis/src/storages/redis/component.cpp | 14 +- redis/src/storages/redis/component.yaml | 20 + .../redis/impl/cluster_shards_query.cpp | 426 ++++++++ .../redis/impl/cluster_shards_query.hpp | 91 ++ .../redis/impl/cluster_shards_query_test.cpp | 953 ++++++++++++++++++ .../redis/impl/cluster_slots_query.cpp | 384 +++++++ .../redis/impl/cluster_slots_query.hpp | 134 +++ .../redis/impl/cluster_slots_query_test.cpp | 197 ++++ .../storages/redis/impl/cluster_topology.hpp | 2 +- .../redis/impl/cluster_topology_holder.cpp | 33 +- .../redis/impl/cluster_topology_holder.hpp | 5 +- .../storages/redis/impl/mock_server_test.cpp | 34 + redis/src/storages/redis/impl/sentinel.cpp | 13 +- redis/src/storages/redis/impl/sentinel.hpp | 7 +- .../src/storages/redis/impl/sentinel_impl.cpp | 6 +- .../src/storages/redis/impl/sentinel_impl.hpp | 4 +- .../storages/redis/impl/sentinel_query.cpp | 335 ------ .../storages/redis/impl/sentinel_query.hpp | 110 -- .../redis/impl/sentinel_query_test.cpp | 190 ---- .../redis/impl/subscribe_sentinel.cpp | 12 +- .../redis/impl/subscribe_sentinel.hpp | 7 +- .../storages/redis/topology_update_method.cpp | 39 + 30 files changed, 2734 insertions(+), 664 deletions(-) create mode 100644 redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt create mode 100644 redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp create mode 100644 redis/functional_tests/cluster_shards_auto_topology/static_config.yaml create mode 100644 redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py create mode 100644 redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py create mode 100644 redis/include/userver/storages/redis/topology_update_method.hpp create mode 100644 redis/src/storages/redis/impl/cluster_shards_query.cpp create mode 100644 redis/src/storages/redis/impl/cluster_shards_query.hpp create mode 100644 redis/src/storages/redis/impl/cluster_shards_query_test.cpp create mode 100644 redis/src/storages/redis/impl/cluster_slots_query.cpp create mode 100644 redis/src/storages/redis/impl/cluster_slots_query.hpp create mode 100644 redis/src/storages/redis/impl/cluster_slots_query_test.cpp create mode 100644 redis/src/storages/redis/topology_update_method.cpp diff --git a/.mapping.json b/.mapping.json index e1ce84f75f15..0262d2a78c31 100644 --- a/.mapping.json +++ b/.mapping.json @@ -4016,6 +4016,11 @@ "redis/functional_tests/cluster_auto_topology_pubsub/static_config.yaml":"taxi/uservices/userver/redis/functional_tests/cluster_auto_topology_pubsub/static_config.yaml", "redis/functional_tests/cluster_auto_topology_pubsub/tests/conftest.py":"taxi/uservices/userver/redis/functional_tests/cluster_auto_topology_pubsub/tests/conftest.py", "redis/functional_tests/cluster_auto_topology_pubsub/tests/test_redis_pubsub.py":"taxi/uservices/userver/redis/functional_tests/cluster_auto_topology_pubsub/tests/test_redis_pubsub.py", + "redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt":"taxi/uservices/userver/redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt", + "redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp":"taxi/uservices/userver/redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp", + "redis/functional_tests/cluster_shards_auto_topology/static_config.yaml":"taxi/uservices/userver/redis/functional_tests/cluster_shards_auto_topology/static_config.yaml", + "redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py":"taxi/uservices/userver/redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py", + "redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py":"taxi/uservices/userver/redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py", "redis/functional_tests/database_index/CMakeLists.txt":"taxi/uservices/userver/redis/functional_tests/database_index/CMakeLists.txt", "redis/functional_tests/database_index/redis_service.cpp":"taxi/uservices/userver/redis/functional_tests/database_index/redis_service.cpp", "redis/functional_tests/database_index/static_config.yaml":"taxi/uservices/userver/redis/functional_tests/database_index/static_config.yaml", @@ -4078,6 +4083,7 @@ "redis/include/userver/storages/redis/sharding_strategies.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/sharding_strategies.hpp", "redis/include/userver/storages/redis/subscribe_client.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/subscribe_client.hpp", "redis/include/userver/storages/redis/subscription_token.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/subscription_token.hpp", + "redis/include/userver/storages/redis/topology_update_method.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/topology_update_method.hpp", "redis/include/userver/storages/redis/transaction.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/transaction.hpp", "redis/include/userver/storages/redis/ttl_reply.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/ttl_reply.hpp", "redis/include/userver/storages/redis/utest/redis_fixture.hpp":"taxi/uservices/userver/redis/include/userver/storages/redis/utest/redis_fixture.hpp", @@ -4107,6 +4113,12 @@ "redis/src/storages/redis/impl/cluster_shard.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shard.cpp", "redis/src/storages/redis/impl/cluster_shard.hpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shard.hpp", "redis/src/storages/redis/impl/cluster_shard_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shard_test.cpp", + "redis/src/storages/redis/impl/cluster_shards_query.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shards_query.cpp", + "redis/src/storages/redis/impl/cluster_shards_query.hpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shards_query.hpp", + "redis/src/storages/redis/impl/cluster_shards_query_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_shards_query_test.cpp", + "redis/src/storages/redis/impl/cluster_slots_query.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_slots_query.cpp", + "redis/src/storages/redis/impl/cluster_slots_query.hpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_slots_query.hpp", + "redis/src/storages/redis/impl/cluster_slots_query_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_slots_query_test.cpp", "redis/src/storages/redis/impl/cluster_subscription_storage.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_subscription_storage.cpp", "redis/src/storages/redis/impl/cluster_subscription_storage.hpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_subscription_storage.hpp", "redis/src/storages/redis/impl/cluster_subscription_storage_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/impl/cluster_subscription_storage_test.cpp", @@ -4212,6 +4224,7 @@ "redis/src/storages/redis/test/mock_client_google_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/test/mock_client_google_test.cpp", "redis/src/storages/redis/test/mock_publish_waiter_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/test/mock_publish_waiter_test.cpp", "redis/src/storages/redis/test/subscribe_client_mock_test.cpp":"taxi/uservices/userver/redis/src/storages/redis/test/subscribe_client_mock_test.cpp", + "redis/src/storages/redis/topology_update_method.cpp":"taxi/uservices/userver/redis/src/storages/redis/topology_update_method.cpp", "redis/src/storages/redis/transaction_impl.cpp":"taxi/uservices/userver/redis/src/storages/redis/transaction_impl.cpp", "redis/src/storages/redis/transaction_impl.hpp":"taxi/uservices/userver/redis/src/storages/redis/transaction_impl.hpp", "redis/src/storages/redis/transaction_redistest.cpp":"taxi/uservices/userver/redis/src/storages/redis/transaction_redistest.cpp", diff --git a/redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt b/redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt new file mode 100644 index 000000000000..801a9adf4bea --- /dev/null +++ b/redis/functional_tests/cluster_shards_auto_topology/CMakeLists.txt @@ -0,0 +1,6 @@ +project(userver-redis-tests-cluster-auto-topology CXX) + +add_executable(${PROJECT_NAME} "redis_service.cpp") +target_link_libraries(${PROJECT_NAME} userver::redis) + +userver_chaos_testsuite_add_redis() diff --git a/redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp b/redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp new file mode 100644 index 000000000000..40ec8b745f6e --- /dev/null +++ b/redis/functional_tests/cluster_shards_auto_topology/redis_service.cpp @@ -0,0 +1,140 @@ +#include +#include // IWYU pragma: keep + +#include +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "userver/storages/redis/exception.hpp" + +namespace chaos { + +class KeyValue final : public server::handlers::HttpHandlerBase { +public: + static constexpr std::string_view kName = "handler-redis"; + + KeyValue(const components::ComponentConfig& config, const components::ComponentContext& context); + + std::string HandleRequestThrow(const server::http::HttpRequest& request, server::request::RequestContext&) + const override; + + static yaml_config::Schema GetStaticConfigSchema(); + +private: + std::string GetValue(std::string_view key, const server::http::HttpRequest& request) const; + std::string PostValue(std::string_view key, const server::http::HttpRequest& request) const; + std::string DeleteValue(std::string_view key) const; + + storages::redis::ClientPtr redis_client_; + storages::redis::CommandControl redis_cc_; +}; + +KeyValue::KeyValue(const components::ComponentConfig& config, const components::ComponentContext& context) + : server::handlers::HttpHandlerBase(config, context), + redis_client_{ + context.FindComponent("key-value-database").GetClient(config["db"].As()) + } +{} + +std::string KeyValue::HandleRequestThrow( + const server::http::HttpRequest& request, + server::request::RequestContext& /*context*/ +) const { + const auto& key = request.GetArg("key"); + if (key.empty()) { + throw server::handlers::ClientError(server::handlers::ExternalBody{"No 'key' query argument"}); + } + + switch (request.GetMethod()) { + case server::http::HttpMethod::kGet: + return GetValue(key, request); + case server::http::HttpMethod::kPost: + return PostValue(key, request); + case server::http::HttpMethod::kDelete: + return DeleteValue(key); + default: + throw server::handlers::ClientError(server::handlers::ExternalBody{ + fmt::format("Unsupported method {}", request.GetMethod()) + }); + } +} + +yaml_config::Schema KeyValue::GetStaticConfigSchema() { + return yaml_config::MergeSchemas(R"( +type: object +description: KeyValue handler schema +additionalProperties: false +properties: + db: + type: string + description: redis database name +)"); +} + +std::string KeyValue::GetValue(std::string_view key, const server::http::HttpRequest& request) const { + try { + const auto result = redis_client_->Get(std::string{key}, redis_cc_).Get(); + if (!result) { + request.SetResponseStatus(server::http::HttpStatus::kNotFound); + return {}; + } + return *result; + } catch (const storages::redis::RequestFailedException& e) { + LOG_WARNING() << "EXCEPTION GET: " << e.what() << ", " << e.GetStatusString() << "\n"; + throw; + } +} + +std::string KeyValue::PostValue(std::string_view key, const server::http::HttpRequest& request) const { + try { + const auto& value = request.GetArg("value"); + const auto result = redis_client_->SetIfNotExist(std::string{key}, value, redis_cc_).Get(); + if (!result) { + request.SetResponseStatus(server::http::HttpStatus::kConflict); + return {}; + } + + request.SetResponseStatus(server::http::HttpStatus::kCreated); + return std::string{value}; + } catch (const storages::redis::RequestFailedException& e) { + LOG_WARNING() << "EXCEPTION POST: " << e.what() << ", " << e.GetStatusString() << "\n"; + throw; + } +} + +std::string KeyValue::DeleteValue(std::string_view key) const { + const auto result = redis_client_->Del(std::string{key}, redis_cc_).Get(); + return std::to_string(result); +} + +} // namespace chaos + +int main(int argc, char* argv[]) { + const auto component_list = + components::MinimalServerComponentList() + .AppendComponentList(USERVER_NAMESPACE::dynamic_config::updater::ComponentList()) + .Append("handler-cluster") + .AppendComponentList(clients::http::ComponentList()) + .Append() + .Append() + .Append("key-value-database") + .Append() + .Append() + .Append(); + return utils::DaemonMain(argc, argv, component_list); +} diff --git a/redis/functional_tests/cluster_shards_auto_topology/static_config.yaml b/redis/functional_tests/cluster_shards_auto_topology/static_config.yaml new file mode 100644 index 000000000000..11b269c25cc5 --- /dev/null +++ b/redis/functional_tests/cluster_shards_auto_topology/static_config.yaml @@ -0,0 +1,79 @@ +# yaml +components_manager: + components: + handler-cluster: + db: redis-cluster + path: /redis-cluster + task_processor: main-task-processor + method: GET,DELETE,POST + + key-value-database: + groups: + - config_name: redis-cluster + db: redis-cluster + sharding_strategy: RedisCluster + topology_update_method: cluster_shards + - config_name: redis-cluster2 + db: redis-cluster2 + sharding_strategy: RedisCluster + topology_update_method: cluster_shards + subscribe_groups: [] + thread_pools: + redis_thread_pool_size: 2 + sentinel_thread_pool_size: 1 + + testsuite-support: + + http-client: + http-client-core: + fs-task-processor: main-task-processor + + tests-control: + method: POST + path: /tests/{action} + skip-unregistered-testpoints: true + task_processor: main-task-processor + testpoint-timeout: 10s + testpoint-url: $mockserver/testpoint + throttling_enabled: false + + secdist: {} + default-secdist-provider: + config: /etc/redis_service/secure_data.json + missing-ok: true + environment-secrets-key: SECDIST_CONFIG + + server: + listener: + port: 8187 + task_processor: main-task-processor + logging: + fs-task-processor: fs-task-processor + loggers: + default: + file_path: '@stderr' + level: debug + overflow_behavior: discard + + dynamic-config: + updates-enabled: true + dynamic-config-client: + config-url: $config-server-url + http-retries: 5 + http-timeout: 20s + service-name: testsuite-support + dynamic-config-client-updater: + config-settings: false + first-update-fail-ok: true + full-update-interval: 1m + update-interval: 5s + dns-client: + fs-task-processor: fs-task-processor + + task_processors: + main-task-processor: + worker_threads: 4 + fs-task-processor: + worker_threads: 4 + + default_task_processor: main-task-processor diff --git a/redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py b/redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py new file mode 100644 index 000000000000..0a26f2cb43dd --- /dev/null +++ b/redis/functional_tests/cluster_shards_auto_topology/tests/conftest.py @@ -0,0 +1,36 @@ +import json + +import pytest + +pytest_plugins = [ + 'pytest_userver.plugins.redis', + 'pytest_userver.plugins.dynamic_config', + 'taxi.uservices.userver.redis.functional_tests.pytest_redis_cluster_topology_plugin.pytest_plugin', +] + + +@pytest.fixture(scope='session') +def service_env(redis_cluster_ports, redis_cluster_topology_session): + cluster_hosts = [] + cluster_shards = [] + for index, port in enumerate(redis_cluster_ports): + cluster_hosts.append({'host': '127.0.0.1', 'port': port}) + for index in range(3): + cluster_shards.append({'name': f'shard{index}'}) + + secdist_config = { + 'redis_settings': { + 'redis-cluster': { + 'password': '', + 'sentinels': cluster_hosts, + 'shards': cluster_shards, + }, + 'redis-cluster2': { + 'password': '', + 'sentinels': cluster_hosts, + 'shards': cluster_shards, + }, + }, + } + + return {'SECDIST_CONFIG': json.dumps(secdist_config)} diff --git a/redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py b/redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py new file mode 100644 index 000000000000..67ea5de2b326 --- /dev/null +++ b/redis/functional_tests/cluster_shards_auto_topology/tests/test_redis_cluster.py @@ -0,0 +1,91 @@ +import asyncio + +import pytest +import pytest_userver.utils.sync as sync + +KEYS_SEQ_LEN = 20 # enough sequential keys to test all slots +REDIS_PORT = 6379 +FAILOVER_DEADLINE_SEC = 30 # maximum time allowed to finish failover + + +async def test_happy_path(service_client, redis_cluster_topology): + post_reqs = [ + service_client.post( + '/redis-cluster', + params={'key': f'key{i}', 'value': 'abc'}, + ) + for i in range(KEYS_SEQ_LEN) + ] + assert all(res.status == 201 for res in await asyncio.gather(*post_reqs)) + + get_reqs = [service_client.get('/redis-cluster', params={'key': f'key{i}'}) for i in range(KEYS_SEQ_LEN)] + assert all(res.status == 200 and res.text == 'abc' for res in await asyncio.gather(*get_reqs)) + + +async def _check_write_all_slots(service_client, key_prefix, value): + post_reqs = [ + service_client.post( + '/redis-cluster', + params={'key': f'{key_prefix}{i}', 'value': value}, + ) + for i in range(KEYS_SEQ_LEN) + ] + return all(res.status != 500 for res in await asyncio.gather(*post_reqs)) + + +async def _assert_read_all_slots(service_client, key_prefix, value): + get_reqs = [ + service_client.get( + '/redis-cluster', + params={'key': f'{key_prefix}{i}'}, + ) + for i in range(KEYS_SEQ_LEN) + ] + assert all(res.status == 200 and res.text == value for res in await asyncio.gather(*get_reqs)) + + +async def test_hard_failover(service_client, redis_cluster_topology): + # Write enough different keys to have something in every slot + assert await _check_write_all_slots(service_client, 'hf_key1', 'abc') + + # Start the failover + redis_cluster_topology.get_masters()[0].stop() + + # wait until service detect that shard 0 is broken + # Failover starts in ~10 seconds + async def is_ready(): + return await _check_write_all_slots( + service_client, + 'hf_key2', + 'cde', + ) + + await sync.wait(is_ready) + + # Now that one of the replicas has become the master, + # check reading from the remaining replica + await _assert_read_all_slots(service_client, 'hf_key1', 'abc') + await _assert_read_all_slots(service_client, 'hf_key2', 'cde') + + +@pytest.mark.skip(reason='Flaky TAXICOMMON-11677') +async def test_add_shard(service_client, redis_cluster_topology): + # Write enough different keys to have something in every slot + assert await _check_write_all_slots(service_client, 'hf_key1', 'abc') + + await redis_cluster_topology.add_shard() + + # Failover starts in ~10 seconds + async def is_ready(): + return await _check_write_all_slots( + service_client, + 'hf_key2', + 'cde', + ) + + await sync.wait(is_ready) + + # Now that one of the replicas has become the master, + # check reading from the remaining replica + await _assert_read_all_slots(service_client, 'hf_key1', 'abc') + await _assert_read_all_slots(service_client, 'hf_key2', 'cde') diff --git a/redis/functional_tests/pytest_redis_cluster_topology_plugin/configs/cluster.conf b/redis/functional_tests/pytest_redis_cluster_topology_plugin/configs/cluster.conf index 9df1b57426d7..b49bd71cfae9 100644 --- a/redis/functional_tests/pytest_redis_cluster_topology_plugin/configs/cluster.conf +++ b/redis/functional_tests/pytest_redis_cluster_topology_plugin/configs/cluster.conf @@ -4,7 +4,7 @@ daemonize yes tcp-backlog 511 timeout 0 tcp-keepalive 0 -loglevel notice +loglevel debug databases 16 cluster-enabled yes diff --git a/redis/include/userver/storages/redis/topology_update_method.hpp b/redis/include/userver/storages/redis/topology_update_method.hpp new file mode 100644 index 000000000000..acefb3965d90 --- /dev/null +++ b/redis/include/userver/storages/redis/topology_update_method.hpp @@ -0,0 +1,15 @@ +#pragma once +#include + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis { + +enum class TopologyUpdateMethod { kClusterSlots, kClusterShards }; + +TopologyUpdateMethod ToTopologyUpdateMethod(std::string_view view); +std::string_view ToStringView(TopologyUpdateMethod); + +} // namespace storages::redis + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/component.cpp b/redis/src/storages/redis/component.cpp index 4261236dd806..a6ba0a5e0cd3 100644 --- a/redis/src/storages/redis/component.cpp +++ b/redis/src/storages/redis/component.cpp @@ -24,7 +24,9 @@ #include #include +#include #include +#include #include #include @@ -73,6 +75,7 @@ struct RedisGroup { std::string config_name; storages::redis::ShardingStrategy sharding_strategy{storages::redis::ShardingStrategy::kKeyShardTaximeterCrc32}; bool allow_reads_from_master{false}; + storages::redis::TopologyUpdateMethod topology_update_method{storages::redis::TopologyUpdateMethod::kClusterSlots}; }; RedisGroup Parse(const yaml_config::YamlConfig& value, formats::parse::To) { @@ -82,6 +85,8 @@ RedisGroup Parse(const yaml_config::YamlConfig& value, formats::parse::To("KeyShardTaximeterCrc32")); config.allow_reads_from_master = value["allow_reads_from_master"].As(false); + config.topology_update_method = + storages::redis::ToTopologyUpdateMethod(value["topology_update_method"].As("cluster_slots")); return config; } @@ -90,6 +95,7 @@ struct SubscribeRedisGroup { std::string config_name; storages::redis::ShardingStrategy sharding_strategy{storages::redis::ShardingStrategy::kKeyShardTaximeterCrc32}; bool allow_reads_from_master{false}; + storages::redis::TopologyUpdateMethod topology_update_method{storages::redis::TopologyUpdateMethod::kClusterSlots}; }; SubscribeRedisGroup Parse(const yaml_config::YamlConfig& value, formats::parse::To) { @@ -99,6 +105,8 @@ SubscribeRedisGroup Parse(const yaml_config::YamlConfig& value, formats::parse:: config.sharding_strategy = storages::redis::ToShardingStrategy(value["sharding_strategy"].As("KeyShardTaximeterCrc32")); config.allow_reads_from_master = value["allow_reads_from_master"].As(false); + config.topology_update_method = + storages::redis::ToTopologyUpdateMethod(value["topology_update_method"].As("cluster_slots")); return config; } @@ -214,7 +222,8 @@ void Redis::Connect( redis_group.db, redis_group.sharding_strategy, cc, - testsuite_redis_control + testsuite_redis_control, + redis_group.topology_update_method ); if (sentinel) { sentinels_.emplace(redis_group.db, sentinel); @@ -247,7 +256,8 @@ void Redis::Connect( redis_group.db, redis_group.sharding_strategy, cc, - testsuite_redis_control + testsuite_redis_control, + redis_group.topology_update_method ); if (sentinel) { subscribe_clients_ diff --git a/redis/src/storages/redis/component.yaml b/redis/src/storages/redis/component.yaml index d64dd8aab6ae..2361932058aa 100644 --- a/redis/src/storages/redis/component.yaml +++ b/redis/src/storages/redis/component.yaml @@ -41,6 +41,16 @@ properties: type: boolean description: allows read requests from master instance default: false + topology_update_method: + type: string + description: | + Topology update method for cluster databases. + cluster_slots - use CLUSTER SLOTS command (default). + cluster_shards - use CLUSTER SHARDS command. + default: cluster_slots + enum: + - cluster_slots + - cluster_shards metrics_level: type: string description: set metrics detail level @@ -75,3 +85,13 @@ properties: type: boolean description: allows subscriptions to master instance to distribute load default: false + topology_update_method: + type: string + description: | + Topology update method for cluster databases. + cluster_slots - use CLUSTER SLOTS command (default). + cluster_shards - use CLUSTER SHARDS command. + default: cluster_slots + enum: + - cluster_slots + - cluster_shards diff --git a/redis/src/storages/redis/impl/cluster_shards_query.cpp b/redis/src/storages/redis/impl/cluster_shards_query.cpp new file mode 100644 index 000000000000..8591a78aabdd --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_shards_query.cpp @@ -0,0 +1,426 @@ +#include "cluster_shards_query.hpp" + +#include +#include + +#include +#include + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis::impl { + +namespace { + +bool ParseClusterShardsHost(const ReplyData& data, ClusterShardsHost& shard_host, const std::string& shard_group_name) { + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + if (!data.IsArray()) { + LOG_ERROR() << log_extra() << "Expected array, got: " << data.GetTypeString(); + return false; + } + const auto& arr = data.GetArray(); + + // The response must be a flat array of key/value pairs. + // If the number of elements is odd, the last key has no value – treat as error. + if (arr.size() % 2 != 0) { + LOG_ERROR() << log_extra() << "Odd number of elements in host map"; + return false; + } + + bool has_ip = false; + bool has_port = false; + bool has_role = false; + + for (size_t i = 0; i + 1 < arr.size(); i += 2) { + const auto& key_elem = arr[i]; + const auto& val_elem = arr[i + 1]; + + // Keys must be strings; otherwise the format is invalid. + if (!key_elem.IsString()) { + LOG_ERROR() << log_extra() << "Host map key is not a string"; + return false; + } + const auto& key = key_elem.GetString(); + + if (key == "ip") { + if (!val_elem.IsString()) { + LOG_ERROR() << log_extra() << "Host 'ip' value is not a string"; + return false; + } + shard_host.ip = val_elem.GetString(); + has_ip = true; + } else if (key == "hostname") { + if (!val_elem.IsString()) { + LOG_ERROR() << log_extra() << "Host 'hostname' value is not a string"; + return false; + } + shard_host.hostname = val_elem.GetString(); + } else if (key == "port") { + if (!val_elem.IsInt()) { + LOG_ERROR() << log_extra() << "Host 'port' value is not an integer"; + return false; + } + shard_host.port = static_cast(val_elem.GetInt()); + has_port = true; + } else if (key == "role") { + if (!val_elem.IsString()) { + LOG_ERROR() << log_extra() << "Host 'role' value is not a string"; + return false; + } + shard_host.is_master = val_elem.GetString() == "master"; + has_role = true; + } else if (key == "replication-offset") { + if (!val_elem.IsInt()) { + LOG_ERROR() << log_extra() << "Host 'replication-offset' value is not an integer"; + return false; + } + shard_host.replication_offset = val_elem.GetInt(); + } else if (key == "health") { + if (!val_elem.IsString()) { + LOG_ERROR() << log_extra() << "Host 'health' value is not an string"; + return false; + } + shard_host.health_online = val_elem.GetString() == "online"; + } + // Unknown keys are ignored – they do not affect validation. + } + + if (!has_ip) { + LOG_ERROR() << log_extra() << "Missing required 'ip' field in host"; + } + if (!has_port) { + LOG_ERROR() << log_extra() << "Missing required 'port' field in host"; + } + if (!has_role) { + LOG_ERROR() << log_extra() << "Missing required 'role' field in host"; + } + + // A failed master is not considered a real master + shard_host.is_master = shard_host.is_master && shard_host.health_online; + + return has_ip && has_port && has_role; +} + +bool ParseShardNodes( + const ReplyData& data, + std::vector& nodes, + const std::string& shard_group_name +) { + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + if (!data.IsArray()) { + LOG_ERROR() << log_extra() << "Expected array for nodes list"; + return false; + } + + const auto& arr = data.GetArray(); + + nodes.clear(); + nodes.reserve(arr.size()); + for (const auto& node_elem : arr) { + ClusterShardsHost host; + if (!ParseClusterShardsHost(node_elem, host, shard_group_name)) { + LOG_ERROR() << log_extra() << "Failed to parse a node entry"; + return false; + } + nodes.emplace_back(std::move(host)); + } + return true; +} + +bool ParseSlots(const ReplyData& data, std::vector& slot_intervals, const std::string& shard_group_name) { + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + if (!data.IsArray()) { + LOG_ERROR() << log_extra() << "Expected array for slots"; + return false; + } + const auto& arr = data.GetArray(); + + // The response must be a flat array of key/value pairs. + // If the number of elements is odd, the last key has no value – treat as error. + if (arr.size() % 2 != 0) { + LOG_ERROR() << log_extra() << "Odd number of elements in slots array"; + return false; + } + + slot_intervals.clear(); + slot_intervals.reserve(arr.size() / 2); + for (size_t i = 0; i + 1 < arr.size(); i += 2) { + const auto& min_reply = arr[i]; + const auto& max_reply = arr[i + 1]; + if (!min_reply.IsInt() || !max_reply.IsInt()) { + LOG_ERROR() << log_extra() << "Slot boundaries are not integers"; + return false; + } + + const auto min = min_reply.GetInt(); + const auto max = max_reply.GetInt(); + + if (min < 0 || min > 16384) { + LOG_ERROR() << log_extra() << "Slot min out of range: " << min; + return false; + } + + if (max < 0 || max > 16384) { + LOG_ERROR() << log_extra() << "Slot max out of range: " << max; + return false; + } + + if (max < min) { + LOG_ERROR() << log_extra() << "Slot max less than min: " << max << " < " << min; + return false; + } + + slot_intervals.emplace_back(min, max); + } + + return true; +} + +bool ParseClusterShardsShard(const ReplyData& data, ClusterShardsShard& shard, const std::string& shard_group_name) { + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + if (!data.IsArray()) { + LOG_ERROR() << log_extra() << "Expected array"; + return false; + } + const auto& arr = data.GetArray(); + + // The response must be a flat array of key/value pairs. + // expected keys are slots, nodes and id + if (arr.size() % 2 != 0) { + LOG_ERROR() << log_extra() << "Unexpected number of elements in shard map: " << arr.size(); + return false; + } + + bool has_slots = false; + bool has_nodes = false; + for (size_t i = 0; i + 1 < arr.size(); i += 2) { + const auto& key_elem = arr[i]; + const auto& val_elem = arr[i + 1]; + + if (!key_elem.IsString()) { + LOG_ERROR() << log_extra() << "Shard map key is not a string"; + return false; + } + + const auto& key = key_elem.GetString(); + if (key == "slots") { + if (!ParseSlots(val_elem, shard.slot_intervals, shard_group_name)) { + LOG_ERROR() << log_extra() << "Failed to parse slots for shard"; + return false; + } + has_slots = true; + } else if (key == "nodes") { + if (!ParseShardNodes(val_elem, shard.hosts, shard_group_name)) { + LOG_ERROR() << log_extra() << "Failed to parse nodes for shard"; + return false; + } + has_nodes = true; + } + // other keys (e.g., "id") are ignored + } + + if (!has_slots) { + LOG_ERROR() << log_extra() << "Shard missing required 'slots' field"; + } + if (!has_nodes) { + LOG_ERROR() << log_extra() << "Shard missing required 'nodes' field"; + } + + return has_slots && has_nodes; +} + +bool ParseClusterShards( + const ReplyData& data, + std::vector& shards, + const std::string& shard_group_name +) { + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + LOG_DEBUG() << "Received CLUSTER SHARDS response: " << data.ToDebugString(); + + if (!data.IsArray()) { + LOG_ERROR() << log_extra() << "Expected array for cluster shards"; + return false; + } + + const auto& arr = data.GetArray(); + shards.clear(); + shards.reserve(arr.size()); + for (const auto& shard_elem : arr) { + ClusterShardsShard shard; + if (!ParseClusterShardsShard(shard_elem, shard, shard_group_name)) { + LOG_ERROR() << log_extra() << "Failed to parse a shard element"; + return false; + } + shards.emplace_back(std::move(shard)); + } + return true; +} + +} // namespace + +ClusterShardsResponseStatus ParseClusterShardsResponse( + const ReplyPtr& reply, + ClusterShardsResponse& res, + const std::string& shard_group_name +) { + UASSERT(reply); + if (reply->IsUnknownCommandError()) { + return ClusterShardsResponseStatus::kNonCluster; + } + if (!reply->IsOk()) { + return ClusterShardsResponseStatus::kFail; + } + if (!reply->data.IsArray()) { + return ClusterShardsResponseStatus::kFail; + } + if (!ParseClusterShards(reply->data, res, shard_group_name)) { + return ClusterShardsResponseStatus::kFail; + } + + return ClusterShardsResponseStatus::kOk; +} + +GetClusterShardsRequest::GetClusterShardsRequest(Shard& sentinel_shard, Password password, std::string shard_group_name) + : sentinel_shard(sentinel_shard), + command({"CLUSTER", "SHARDS"}), + password(std::move(password)), + shard_group_name(std::move(shard_group_name)) +{} + +void GetClusterShardsContext::ProcessRequest( + std::shared_ptr> shard_names, + GetClusterShardsRequest request, + ProcessGetClusterHostsRequestCb callback +) { + auto ids = request.sentinel_shard.GetAllInstancesServerId(); + auto context = std::make_shared( + request.password, + std::move(shard_names), + request.shard_group_name, + std::move(callback), + ids.size() + ); + for (const auto& id : ids) { + auto cmd = PrepareCommand(request.command.Clone(), [context](const CommandPtr& command, const ReplyPtr& reply) { + context->OnResponse(command, reply); + }); + cmd->control.force_server_id = id; + if (!request.sentinel_shard.AsyncCommand(cmd)) { + context->OnAsyncCommandFailed(); + } + } +} + +GetClusterShardsContext::GetClusterShardsContext( + Password password, + std::shared_ptr> shard_names, + std::string shard_group_name, + ProcessGetClusterHostsRequestCb&& callback, + size_t expected_responses_cnt +) + : shard_group_name_(std::move(shard_group_name)), + password_(std::move(password)), + shard_names_(std::move(shard_names)), + callback_(std::move(callback)), + expected_responses_cnt_(expected_responses_cnt) +{} + +void GetClusterShardsContext::OnAsyncCommandFailed() { + --expected_responses_cnt_; + ProcessResponses(); +} + +void GetClusterShardsContext::OnResponse(const CommandPtr&, const ReplyPtr& reply) { + ClusterShardsResponse response; + switch (ParseClusterShardsResponse(reply, response, shard_group_name_)) { + case ClusterShardsResponseStatus::kOk: { + { + const std::lock_guard lock(mutex_); + responses_by_id_[reply->server_id] = std::move(response); + } + ++responses_parsed_; + break; + } + case ClusterShardsResponseStatus::kFail: + break; + case ClusterShardsResponseStatus::kNonCluster: + is_non_cluster_ = true; + break; + } + ++response_got_; + ProcessResponses(); +} + +void GetClusterShardsContext::ProcessResponses() { + if (response_got_ >= expected_responses_cnt_ || is_non_cluster_) { + if (!process_responses_started_.test_and_set()) { + ProcessResponsesOnce(); + } + } +} + +std::map ConvertToClusterSlotsResponse( + const std::unordered_map& responses_by_id +) { + std::map result; + for (const auto& [server_id, shards] : responses_by_id) { + ClusterSlotsResponse slots_response; + for (const auto& shard : shards) { + // Find master host + const ClusterShardsHost* master_host = nullptr; + for (const auto& host : shard.hosts) { + if (host.is_master) { + master_host = &host; + break; + } + } + if (!master_host) { + // No master in this shard – skip it + LOG_DEBUG() << "No master in shard"; + continue; + } + + // Build ConnectionInfoInt for master + MasterSlavesConnInfos msci; + msci.master = ConnectionInfoInt{{master_host->ip, static_cast(master_host->port), {}}}; + // Add slaves + for (const auto& host : shard.hosts) { + if (host.is_master) { + continue; + } + ConnectionInfoInt slave_conn{{host.ip, static_cast(host.port), {}}}; + msci.slaves.insert(std::move(slave_conn)); + } + // Associate each slot interval with the master/slaves info + for (const auto& interval : shard.slot_intervals) { + slots_response.emplace(interval, msci); + } + } + result.emplace(server_id, std::move(slots_response)); + } + return result; +} + +void GetClusterShardsContext::ProcessResponsesOnce() { + std::map + cluster_slots_response = ConvertToClusterSlotsResponse(this->responses_by_id_); + ProceResponseOnceImpl( + cluster_slots_response, + callback_, + shard_group_name_, + *shard_names_, + password_, + expected_responses_cnt_.load(), + responses_parsed_.load(), + is_non_cluster_.load() + ); +} + +} // namespace storages::redis::impl + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_shards_query.hpp b/redis/src/storages/redis/impl/cluster_shards_query.hpp new file mode 100644 index 000000000000..83375d6fbab7 --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_shards_query.hpp @@ -0,0 +1,91 @@ +#pragma once + +#include "cluster_slots_query.hpp" + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis::impl { + +struct GetClusterShardsRequest { + GetClusterShardsRequest(Shard& sentinel_shard, Password password, std::string shard_group_name); + + Shard& sentinel_shard; + CmdArgs command; + + Password password; + std::string shard_group_name; +}; + +enum class ClusterShardsHostHealth { kOnline, kFail, kLoading }; + +struct ClusterShardsHost { + std::string ip; + std::optional hostname; + int64_t replication_offset{0}; + uint16_t port{6379}; + bool is_master{false}; + bool health_online{false}; +}; + +struct ClusterShardsShard { + std::vector hosts; + std::vector slot_intervals; +}; + +using ClusterShardsResponse = std::vector; + +class GetClusterShardsContext { +public: + GetClusterShardsContext( + Password password, + std::shared_ptr> shard_names, + std::string shard_group_name, + ProcessGetClusterHostsRequestCb&& callback, + size_t expected_responses_cnt + ); + + static void ProcessRequest( + std::shared_ptr> shard_names, + GetClusterShardsRequest request, + ProcessGetClusterHostsRequestCb callback + ); + +private: + void OnAsyncCommandFailed(); + void OnResponse(const CommandPtr&, const ReplyPtr& reply); + void ProcessResponses(); + void ProcessResponsesOnce(); + + const std::string shard_group_name_; + const Password password_; + const std::shared_ptr> shard_names_; + const ProcessGetClusterHostsRequestCb callback_; + std::atomic response_got_{0}; + std::atomic responses_parsed_{0}; + std::atomic_flag process_responses_started_ ATOMIC_FLAG_INIT; + std::atomic expected_responses_cnt_{0}; + std::atomic is_non_cluster_{false}; + + std::mutex mutex_; + std::unordered_map responses_by_id_; +}; + +std::map ConvertToClusterSlotsResponse( + const std::unordered_map& responses_by_id +); + +enum class ClusterShardsResponseStatus { + kOk, + kFail, + kNonCluster, +}; + +ClusterShardsResponseStatus ParseClusterShardsResponse( + const ReplyPtr& reply, + ClusterShardsResponse& res, + const std::string& shard_group_name +); + +} // namespace storages::redis::impl + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_shards_query_test.cpp b/redis/src/storages/redis/impl/cluster_shards_query_test.cpp new file mode 100644 index 000000000000..de86dbc87621 --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_shards_query_test.cpp @@ -0,0 +1,953 @@ +#include +#include +#include +#include + +USERVER_NAMESPACE_BEGIN + +namespace { + +using ServerId = storages::redis::ServerId; +using ClusterShardsHost = storages::redis::impl::ClusterShardsHost; +using ClusterShardsShard = storages::redis::impl::ClusterShardsShard; +using ClusterShardsResponseStatus = storages::redis::impl::ClusterShardsResponseStatus; +using ClusterShardsResponse = storages::redis::impl::ClusterShardsResponse; +using SlotInterval = storages::redis::impl::SlotInterval; +using ClusterShardsResponsesByServerId = + std::unordered_map; +using ReplyData = storages::redis::ReplyData; +using Reply = storages::redis::Reply; +using ReplyPtr = storages::redis::ReplyPtr; +using ReplyStatus = storages::redis::ReplyStatus; + +ClusterShardsHost CreateHost(const std::string& ip, uint16_t port, bool is_master, int64_t replication_offset = 0) { + ClusterShardsHost host; + host.ip = ip; + host.port = port; + host.is_master = is_master; + host.replication_offset = replication_offset; + return host; +} + +ClusterShardsShard CreateShard( + const std::vector& hosts, + const std::vector& slot_intervals +) { + ClusterShardsShard shard; + shard.hosts = hosts; + shard.slot_intervals = slot_intervals; + return shard; +} + +ClusterShardsResponse CreateClusterShardsResponse(const std::vector& shards) { return shards; } + +} // namespace + +TEST(ConvertToClusterSlotsResponse, EmptyResponse) { + ClusterShardsResponsesByServerId responses_by_id; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + EXPECT_TRUE(result.empty()); +} + +TEST(ConvertToClusterSlotsResponse, SingleServerSingleShard) { + const std::string master_ip = "192.168.1.1"; + const std::string slave_ip = "192.168.1.2"; + const auto master_port = 6379; + const auto slave_port = 6380; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse({CreateShard( + {CreateHost(master_ip, master_port, true), CreateHost(slave_ip, slave_port, false)}, + {SlotInterval(0, 5460)} + )}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + EXPECT_EQ(result.count(server_id), 1); + + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 1); + + const auto& [interval, conn_infos] = *slots_response.begin(); + EXPECT_EQ(interval.slot_min, 0); + EXPECT_EQ(interval.slot_max, 5460); + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, static_cast(master_port))); + ASSERT_EQ(conn_infos.slaves.size(), 1); + EXPECT_EQ(conn_infos.slaves.begin()->HostPort(), std::make_pair(slave_ip, static_cast(slave_port))); +} + +TEST(ConvertToClusterSlotsResponse, SingleServerMultipleShards) { + const std::string master_ip1 = "192.168.1.1"; + const std::string slave_ip1 = "192.168.1.2"; + const std::string master_ip2 = "192.168.1.3"; + const std::string slave_ip2 = "192.168.1.4"; + const std::string master_ip3 = "192.168.1.5"; + const std::string slave_ip3 = "192.168.1.6"; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse( + {CreateShard({CreateHost(master_ip1, 6379, true), CreateHost(slave_ip1, 6380, false)}, {SlotInterval(0, 5460)}), + CreateShard( + {CreateHost(master_ip2, 6379, true), CreateHost(slave_ip2, 6380, false)}, + {SlotInterval(5461, 10922)} + ), + CreateShard( + {CreateHost(master_ip3, 6379, true), CreateHost(slave_ip3, 6380, false)}, + {SlotInterval(10923, 16383)} + )} + ); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 3); + + // Check first shard + auto it = slots_response.find(SlotInterval(0, 5460)); + ASSERT_NE(it, slots_response.end()); + EXPECT_EQ(it->second.master.HostPort(), std::make_pair(master_ip1, 6379)); + ASSERT_EQ(it->second.slaves.size(), 1); + EXPECT_EQ(it->second.slaves.begin()->HostPort(), std::make_pair(slave_ip1, 6380)); + + // Check second shard + it = slots_response.find(SlotInterval(5461, 10922)); + ASSERT_NE(it, slots_response.end()); + EXPECT_EQ(it->second.master.HostPort(), std::make_pair(master_ip2, 6379)); + ASSERT_EQ(it->second.slaves.size(), 1); + EXPECT_EQ(it->second.slaves.begin()->HostPort(), std::make_pair(slave_ip2, 6380)); + + // Check third shard + it = slots_response.find(SlotInterval(10923, 16383)); + ASSERT_NE(it, slots_response.end()); + EXPECT_EQ(it->second.master.HostPort(), std::make_pair(master_ip3, 6379)); + ASSERT_EQ(it->second.slaves.size(), 1); + EXPECT_EQ(it->second.slaves.begin()->HostPort(), std::make_pair(slave_ip3, 6380)); +} + +TEST(ConvertToClusterSlotsResponse, ShardWithoutMasterIsSkipped) { + const std::string slave_ip = "192.168.1.2"; + + ClusterShardsResponse shards_response = + CreateClusterShardsResponse({CreateShard({CreateHost(slave_ip, 6380, false)}, {SlotInterval(0, 5460)})}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + EXPECT_TRUE(slots_response.empty()); +} + +TEST(ConvertToClusterSlotsResponse, MultipleServers) { + const std::string master_ip1 = "192.168.1.1"; + const std::string master_ip2 = "192.168.2.1"; + + ClusterShardsResponse shards_response1 = + CreateClusterShardsResponse({CreateShard({CreateHost(master_ip1, 6379, true)}, {SlotInterval(0, 8191)})}); + + ClusterShardsResponse shards_response2 = + CreateClusterShardsResponse({CreateShard({CreateHost(master_ip2, 6379, true)}, {SlotInterval(8192, 16383)})}); + + auto server_id1 = ServerId::Generate(); + auto server_id2 = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id1] = shards_response1; + responses_by_id[server_id2] = shards_response2; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 2); + + // Check first server + ASSERT_EQ(result.count(server_id1), 1); + const auto& slots_response1 = result.at(server_id1); + ASSERT_EQ(slots_response1.size(), 1); + const auto& [interval1, conn_infos1] = *slots_response1.begin(); + EXPECT_EQ(interval1.slot_min, 0); + EXPECT_EQ(interval1.slot_max, 8191); + EXPECT_EQ(conn_infos1.master.HostPort(), std::make_pair(master_ip1, 6379)); + + // Check second server + ASSERT_EQ(result.count(server_id2), 1); + const auto& slots_response2 = result.at(server_id2); + ASSERT_EQ(slots_response2.size(), 1); + const auto& [interval2, conn_infos2] = *slots_response2.begin(); + EXPECT_EQ(interval2.slot_min, 8192); + EXPECT_EQ(interval2.slot_max, 16383); + EXPECT_EQ(conn_infos2.master.HostPort(), std::make_pair(master_ip2, 6379)); +} + +TEST(ConvertToClusterSlotsResponse, ShardWithMultipleSlaves) { + const std::string master_ip = "192.168.1.1"; + const std::string slave_ip1 = "192.168.1.2"; + const std::string slave_ip2 = "192.168.1.3"; + const std::string slave_ip3 = "192.168.1.4"; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse({CreateShard( + {CreateHost(master_ip, 6379, true), + CreateHost(slave_ip1, 6380, false), + CreateHost(slave_ip2, 6381, false), + CreateHost(slave_ip3, 6382, false)}, + {SlotInterval(0, 16383)} + )}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 1); + + const auto& [interval, conn_infos] = *slots_response.begin(); + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, 6379)); + ASSERT_EQ(conn_infos.slaves.size(), 3); + + std::set> expected_slaves = {{slave_ip1, 6380}, {slave_ip2, 6381}, {slave_ip3, 6382}}; + + for (const auto& slave : conn_infos.slaves) { + EXPECT_TRUE(expected_slaves.erase(slave.HostPort())); + } + EXPECT_TRUE(expected_slaves.empty()); +} + +TEST(ConvertToClusterSlotsResponse, ShardWithMultipleSlotIntervals) { + const std::string master_ip = "192.168.1.1"; + const std::string slave_ip = "192.168.1.2"; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse({CreateShard( + {CreateHost(master_ip, 6379, true), CreateHost(slave_ip, 6380, false)}, + {SlotInterval(0, 1000), SlotInterval(2000, 3000), SlotInterval(5000, 6000)} + )}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 3); + + // All intervals should map to the same master/slaves + for (const auto& [interval, conn_infos] : slots_response) { + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, 6379)); + ASSERT_EQ(conn_infos.slaves.size(), 1); + EXPECT_EQ(conn_infos.slaves.begin()->HostPort(), std::make_pair(slave_ip, 6380)); + } + + // Verify all intervals are present + EXPECT_NE(slots_response.find(SlotInterval(0, 1000)), slots_response.end()); + EXPECT_NE(slots_response.find(SlotInterval(2000, 3000)), slots_response.end()); + EXPECT_NE(slots_response.find(SlotInterval(5000, 6000)), slots_response.end()); +} + +TEST(ConvertToClusterSlotsResponse, MixedValidAndInvalidShards) { + const std::string master_ip1 = "192.168.1.1"; + const std::string slave_ip1 = "192.168.1.2"; + const std::string slave_ip2 = "192.168.1.3"; + const std::string master_ip2 = "192.168.1.4"; + const std::string slave_ip3 = "192.168.1.5"; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse( + {// Valid shard with master + CreateShard({CreateHost(master_ip1, 6379, true), CreateHost(slave_ip1, 6380, false)}, {SlotInterval(0, 5460)}), + // Invalid shard - no master + CreateShard({CreateHost(slave_ip2, 6380, false)}, {SlotInterval(5461, 10922)}), + // Valid shard with master + CreateShard( + {CreateHost(master_ip2, 6379, true), CreateHost(slave_ip3, 6380, false)}, + {SlotInterval(10923, 16383)} + ) + } + ); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + + // Only valid shards should be present + ASSERT_EQ(slots_response.size(), 2); + EXPECT_NE(slots_response.find(SlotInterval(0, 5460)), slots_response.end()); + EXPECT_EQ(slots_response.find(SlotInterval(5461, 10922)), slots_response.end()); + EXPECT_NE(slots_response.find(SlotInterval(10923, 16383)), slots_response.end()); +} + +TEST(ConvertToClusterSlotsResponse, ShardWithOnlyMaster) { + const std::string master_ip = "192.168.1.1"; + + ClusterShardsResponse shards_response = + CreateClusterShardsResponse({CreateShard({CreateHost(master_ip, 6379, true)}, {SlotInterval(0, 16383)})}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 1); + + const auto& [interval, conn_infos] = *slots_response.begin(); + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, 6379)); + EXPECT_TRUE(conn_infos.slaves.empty()); +} + +TEST(ConvertToClusterSlotsResponse, EmptyShardsList) { + ClusterShardsResponse shards_response; + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + EXPECT_TRUE(slots_response.empty()); +} + +TEST(ConvertToClusterSlotsResponse, IPv6Addresses) { + const std::string master_ip = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:aaaa"; + const std::string slave_ip = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:bbbb"; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse( + {CreateShard({CreateHost(master_ip, 6379, true), CreateHost(slave_ip, 6380, false)}, {SlotInterval(0, 16383)})} + ); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 1); + + const auto& [interval, conn_infos] = *slots_response.begin(); + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, 6379)); + ASSERT_EQ(conn_infos.slaves.size(), 1); + EXPECT_EQ(conn_infos.slaves.begin()->HostPort(), std::make_pair(slave_ip, 6380)); +} + +TEST(ConvertToClusterSlotsResponse, DifferentPorts) { + const std::string master_ip = "192.168.1.1"; + const std::string slave_ip = "192.168.1.2"; + constexpr auto master_port = 7000; + constexpr auto slave_port = 7001; + + ClusterShardsResponse shards_response = CreateClusterShardsResponse({CreateShard( + {CreateHost(master_ip, master_port, true), CreateHost(slave_ip, slave_port, false)}, + {SlotInterval(0, 16383)} + )}); + + auto server_id = ServerId::Generate(); + ClusterShardsResponsesByServerId responses_by_id; + responses_by_id[server_id] = shards_response; + + auto result = ConvertToClusterSlotsResponse(responses_by_id); + + ASSERT_EQ(result.size(), 1); + const auto& slots_response = result.at(server_id); + ASSERT_EQ(slots_response.size(), 1); + + const auto& [interval, conn_infos] = *slots_response.begin(); + EXPECT_EQ(conn_infos.master.HostPort(), std::make_pair(master_ip, master_port)); + ASSERT_EQ(conn_infos.slaves.size(), 1); + EXPECT_EQ(conn_infos.slaves.begin()->HostPort(), std::make_pair(slave_ip, slave_port)); +} + +// Helper function to create a ReplyPtr for testing +ReplyPtr CreateReply(ReplyData&& data, ReplyStatus status = ReplyStatus::kOk) { + return std::make_shared("CLUSTER SHARDS", std::move(data), status); +} + +TEST(ParseClusterShardsResponse, ValidSingleShard) { + // Create a valid CLUSTER SHARDS response with one shard + auto response_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ + ReplyData(0), // + ReplyData(5460) + }), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online"), + ReplyData("replication-offset"), + ReplyData(123456789) + }), + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.2"), + ReplyData("port"), + ReplyData(6380), + ReplyData("role"), + ReplyData("slave"), + ReplyData("health"), + ReplyData("online") + }) + }) + }) + }); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + + const auto& shard = result[0]; + ASSERT_EQ(shard.hosts.size(), 2); + EXPECT_EQ(shard.hosts[0].ip, "192.168.1.1"); + EXPECT_EQ(shard.hosts[0].port, 6379); + EXPECT_EQ(shard.hosts[0].replication_offset, 123456789); + EXPECT_TRUE(shard.hosts[0].is_master); + EXPECT_EQ(shard.hosts[1].ip, "192.168.1.2"); + EXPECT_EQ(shard.hosts[1].port, 6380); + EXPECT_FALSE(shard.hosts[1].is_master); + + ASSERT_EQ(shard.slot_intervals.size(), 1); + EXPECT_EQ(shard.slot_intervals[0].slot_min, 0); + EXPECT_EQ(shard.slot_intervals[0].slot_max, 5460); +} + +TEST(ParseClusterShardsResponse, ValidMultipleShards) { + // Create a valid CLUSTER SHARDS response with multiple shards + ReplyData response_data = ReplyData::Array( + {// Shards + ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("health"), + ReplyData("online"), + ReplyData("role"), + ReplyData("master") + })}) + }, + ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(5461), ReplyData(10922)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ + // Nodes + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.2"), + ReplyData("port"), + ReplyData(6379), + ReplyData("health"), + ReplyData("online"), + ReplyData("role"), + ReplyData("master") + }) + }) + } + } + ); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 2); + + EXPECT_EQ(result[0].slot_intervals[0].slot_min, 0); + EXPECT_EQ(result[0].slot_intervals[0].slot_max, 5460); + EXPECT_EQ(result[0].hosts[0].ip, "192.168.1.1"); + + EXPECT_EQ(result[1].slot_intervals[0].slot_min, 5461); + EXPECT_EQ(result[1].slot_intervals[0].slot_max, 10922); + EXPECT_EQ(result[1].hosts[0].ip, "192.168.1.2"); +} + +TEST(ParseClusterShardsResponse, SingleShardWithOneFailedMaster) { + // Create a valid CLUSTER SHARDS response with one shard + // one of the instance was master before and failed. + auto response_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ + ReplyData(0), // + ReplyData(5460) + }), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("failed"), + ReplyData("replication-offset"), + ReplyData(123456789) + }), + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.2"), + ReplyData("port"), + ReplyData(6380), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + }) + }) + }) + }); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + + const auto& shard = result[0]; + ASSERT_EQ(shard.hosts.size(), 2); + EXPECT_EQ(shard.hosts[0].ip, "192.168.1.1"); + EXPECT_EQ(shard.hosts[0].port, 6379); + EXPECT_EQ(shard.hosts[0].replication_offset, 123456789); + // Not a master because it has "failed" health + EXPECT_FALSE(shard.hosts[0].is_master); + EXPECT_EQ(shard.hosts[1].ip, "192.168.1.2"); + EXPECT_EQ(shard.hosts[1].port, 6380); + // Is a master because it has "online" health + EXPECT_TRUE(shard.hosts[1].is_master); + + ASSERT_EQ(shard.slot_intervals.size(), 1); + EXPECT_EQ(shard.slot_intervals[0].slot_min, 0); + EXPECT_EQ(shard.slot_intervals[0].slot_max, 5460); +} + +TEST(ParseClusterShardsResponse, UnknownCommandError) { + // Simulate unknown command error (non-cluster mode) + auto reply = CreateReply(ReplyData::CreateError("ERR unknown command `CLUSTER`"), ReplyStatus::kOtherError); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kNonCluster); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, NotOkReply) { + // Simulate a non-ok reply + auto reply = CreateReply(ReplyData::CreateError("Some error"), ReplyStatus::kOtherError); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, NonArrayResponse) { + // Response is not an array + auto reply = CreateReply(ReplyData("OK")); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, MissingRequiredHostFields) { + // Host missing required 'ip' field + auto reply_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online"), + // Missing 'ip' field + })}) + }) + }); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, MissingRequiredShardFields) { + // Shard missing required 'slots' field + auto reply_data = ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + })}) + // Missing 'slots' field + })}); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, InvalidSlotRange) { + // Invalid slot range (max < min) + auto reply_data = ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(5460), ReplyData(0)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + })}) + })}); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, SlotOutOfRange) { + // Slot value out of valid range (0-16384) + auto reply_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(20000)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + })}) + }) + }); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, OddNumberOfHostElements) { + // Host map with odd number of elements (missing value) + auto reply_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + })}) + }) + }); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, OddNumberOfSlotElements) { + // Slots array with odd number of elements (missing max value) + auto reply_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + })}) + }) + }); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, InvalidHostFieldType) { + // Host field with wrong type (port is string instead of int) + auto reply_data = ReplyData(ReplyData::Array{ + // Shard + ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData("6379"), // Should be int + ReplyData("role"), + ReplyData("master"), + })}) + }) + }); + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kFail); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, HostWithHostname) { + // Host with optional hostname field + ReplyData response_data(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("hostname"), + ReplyData("redis-master.example.com"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + })}) + })}); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(result[0].hosts.size(), 1); + EXPECT_EQ(result[0].hosts[0].ip, "192.168.1.1"); + ASSERT_TRUE(result[0].hosts[0].hostname.has_value()); + EXPECT_EQ(result[0].hosts[0].hostname.value(), "redis-master.example.com"); +} + +TEST(ParseClusterShardsResponse, MultipleSlotIntervals) { + // Shard with multiple slot intervals + auto response_data = ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ + ReplyData(0), + ReplyData(1000), + ReplyData(2000), + ReplyData(3000), + ReplyData(5000), + ReplyData(6000) + }), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + })}) + })}); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(result[0].slot_intervals.size(), 3); + EXPECT_EQ(result[0].slot_intervals[0].slot_min, 0); + EXPECT_EQ(result[0].slot_intervals[0].slot_max, 1000); + EXPECT_EQ(result[0].slot_intervals[1].slot_min, 2000); + EXPECT_EQ(result[0].slot_intervals[1].slot_max, 3000); + EXPECT_EQ(result[0].slot_intervals[2].slot_min, 5000); + EXPECT_EQ(result[0].slot_intervals[2].slot_max, 6000); +} + +TEST(ParseClusterShardsResponse, EmptyShardsArray) { + // Empty shards array + ReplyData response_data(ReplyData::Array{}); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + EXPECT_TRUE(result.empty()); +} + +TEST(ParseClusterShardsResponse, ShardWithMultipleNodes) { + // Shard with multiple nodes (master + multiple slaves) + auto reply_data = ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(16383)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("health"), + ReplyData("online") + }), + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.2"), + ReplyData("port"), + ReplyData(6380), + ReplyData("role"), + ReplyData("slave"), + ReplyData("health"), + ReplyData("online") + }), + ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.3"), + ReplyData("port"), + ReplyData(6381), + ReplyData("role"), + ReplyData("slave"), + ReplyData("health"), + ReplyData("online") + }) + }) + })}); + + auto reply = CreateReply(std::move(reply_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(result[0].hosts.size(), 3); + EXPECT_TRUE(result[0].hosts[0].is_master); + EXPECT_FALSE(result[0].hosts[1].is_master); + EXPECT_FALSE(result[0].hosts[2].is_master); +} + +TEST(ParseClusterShardsResponse, UnknownKeysIgnored) { + // Unknown keys in host and shard should be ignored + auto response_data = ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("slots"), + ReplyData(ReplyData::Array{ReplyData(0), ReplyData(5460)}), + ReplyData("nodes"), + ReplyData(ReplyData::Array{ReplyData(ReplyData::Array{ + ReplyData("ip"), + ReplyData("192.168.1.1"), + ReplyData("port"), + ReplyData(6379), + ReplyData("role"), + ReplyData("master"), + ReplyData("unknown-field"), + ReplyData("some-value") // Should be ignored + })}), + ReplyData("id"), + ReplyData("shard-123") // Unknown shard key, should be ignored + })}); + auto reply = CreateReply(std::move(response_data)); + + ClusterShardsResponse result; + auto status = ParseClusterShardsResponse(reply, result, "test_shard_group"); + + EXPECT_EQ(status, ClusterShardsResponseStatus::kOk); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(result[0].hosts.size(), 1); + EXPECT_EQ(result[0].hosts[0].ip, "192.168.1.1"); +} + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_slots_query.cpp b/redis/src/storages/redis/impl/cluster_slots_query.cpp new file mode 100644 index 000000000000..e86e5bbd46a5 --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_slots_query.cpp @@ -0,0 +1,384 @@ +#include + +#include + +#include +#include +#include + +#include +#include + +#include +#include +#include + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis::impl { + +namespace { + +std::optional GetIpFromMeta(const ReplyData::Array& host_info_array) { + if (host_info_array.size() < 4) { + return std::nullopt; + } + const auto& meta = host_info_array[3]; + if (!meta.IsArray() || meta.GetSize() < 2) { + return std::nullopt; + } + + const auto& array = meta.GetArray(); + if (!array[0].IsString() || !array[1].IsString() || array[0].GetString() != "ip") { + return std::nullopt; + } + return array[1].GetString(); +} + +std::string GetIpFromHostInfo(const ReplyData::Array& host_info_array) { + auto from_meta = GetIpFromMeta(host_info_array); + if (from_meta) { + return *from_meta; + } + return host_info_array[0].GetString(); +} + +} // namespace + +logging::LogHelper& operator<<(logging::LogHelper& log, SlotInterval interval) { + log << '[' << interval.slot_min << ',' << interval.slot_max << ']'; + return log; +} + +ClusterSlotsResponseStatus ParseClusterSlotsResponse( + const ReplyPtr& reply, + ClusterSlotsResponse& res, + const std::string& shard_group_name +) { + UASSERT(reply); + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + + LOG_DEBUG() << "Received CLUSTER SLOTS response: " << reply->data.ToDebugString(); + + LOG_TRACE() << log_extra() << "Got reply to CLUSTER SLOTS: " << reply->data.ToDebugString(); + if (reply->IsUnknownCommandError()) { + LOG_ERROR() + << log_extra() + << "Redis CLUSTER SLOTS reply contains unknown command error: " << reply->data.ToDebugString(); + return ClusterSlotsResponseStatus::kNonCluster; + } + if (reply->status != ReplyStatus::kOk) { + LOG_ERROR() << log_extra() << "Redis CLUSTER SLOTS reply contains error: " << reply->data.ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + if (!reply->data.IsArray()) { + LOG_ERROR() << log_extra() << "Redis CLUSTER SLOTS reply is not an array: " << reply->data.ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + std::size_t array_index = 0; + for (const auto& reply_interval : reply->data.GetArray()) { + if (!reply_interval.IsArray()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index + << "] is not an array: " << reply_interval.ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + const auto& array = reply_interval.GetArray(); + if (array.size() < 3) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index + << "] is an array of size less than 3: " << reply_interval.ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + if (!array[0].IsInt()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index + << "][0] is not an int: " << array[0].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + if (!array[1].IsInt()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index + << "][1] is not an int: " << array[1].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + for (std::size_t i = 2; i < array.size(); i++) { + if (!array[i].IsArray()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i + << "] is not an array: " << array[i].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + const auto& host_info_array = array[i].GetArray(); + if (host_info_array.size() < 2) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i + << "] is an array of size less than 2: " << array[i].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + if (!host_info_array[0].IsString()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i + << "][0] is not a string: " << host_info_array[0].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + if (!host_info_array[1].IsInt()) { + LOG_ERROR() + << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i + << "][1] is not an int: " << host_info_array[1].ToDebugString(); + return ClusterSlotsResponseStatus::kFail; + } + + ConnectionInfoInt conn_info{ + {GetIpFromHostInfo(host_info_array), static_cast(host_info_array[1].GetInt()), {}} + }; + const SlotInterval slot_interval(array[0].GetInt(), array[1].GetInt()); + if (i == 2) { + const bool is_master_overwritten = + (!res[slot_interval].master.HostPort().first.empty() && + res[slot_interval].master.HostPort().first != "localhost" && + res[slot_interval].master.HostPort() != conn_info.HostPort()); + if (is_master_overwritten) { + const auto message = fmt::format( + "Redis CLUSTER SLOTS reply element[{}][{}] overwrites master '{}' with '{}'", + array_index, + i, + res[slot_interval].master.HostPort().first, + conn_info.HostPort().first + ); + LOG_ERROR() << log_extra() << message; + UASSERT_MSG(false, message); + } + + res[slot_interval].master = std::move(conn_info); + } else { + res[slot_interval].slaves.insert(std::move(conn_info)); + } + } + + ++array_index; + } + return ClusterSlotsResponseStatus::kOk; +} + +void GetClusterSlotsContext::ProcessRequest( + std::shared_ptr> shard_names, + GetClusterSlotsRequest request, + ProcessGetClusterHostsRequestCb callback +) { + auto ids = request.sentinel_shard.GetAllInstancesServerId(); + auto context = std::make_shared( + request.password, + std::move(shard_names), + request.shard_group_name, + std::move(callback), + ids.size() + ); + + for (const auto& id : ids) { + auto cmd = PrepareCommand(request.command.Clone(), [context](const CommandPtr& command, const ReplyPtr& reply) { + context->OnResponse(command, reply); + }); + cmd->control.force_server_id = id; + if (!request.sentinel_shard.AsyncCommand(cmd)) { + context->OnAsyncCommandFailed(); + } + } +} + +GetClusterSlotsContext::GetClusterSlotsContext( + Password password, + std::shared_ptr> shard_names, + std::string shard_group_name, + ProcessGetClusterHostsRequestCb&& callback, + size_t expected_responses_cnt +) + : shard_group_name_(std::move(shard_group_name)), + password_(std::move(password)), + shard_names_(std::move(shard_names)), + callback_(std::move(callback)), + expected_responses_cnt_(expected_responses_cnt) +{} + +void GetClusterSlotsContext::OnAsyncCommandFailed() { + --expected_responses_cnt_; + + ProcessResponses(); +} + +void GetClusterSlotsContext::OnResponse(const CommandPtr&, const ReplyPtr& reply) { + ClusterSlotsResponse response; + switch (ParseClusterSlotsResponse(reply, response, shard_group_name_)) { + case ClusterSlotsResponseStatus::kOk: { + { + const std::lock_guard lock(mutex_); + responses_by_id_[reply->server_id] = std::move(response); + } + responses_parsed_++; + break; + } + case ClusterSlotsResponseStatus::kFail: + break; + case ClusterSlotsResponseStatus::kNonCluster: + is_non_cluster_ = true; + break; + } + + response_got_++; + + ProcessResponses(); +} + +void GetClusterSlotsContext::ProcessResponses() { + if (response_got_ >= expected_responses_cnt_ || is_non_cluster_) { + if (!process_responses_started_.test_and_set()) { + ProcessResponsesOnce(); + } + } +} + +void ProceResponseOnceImpl( + const std::map& responses_by_id, + const ProcessGetClusterHostsRequestCb& callback, + const std::string& shard_group_name, + const std::vector& shard_names, + const Password& password, + size_t expected_responses_cnt, + size_t responses_parsed, + bool is_non_cluster +) { + ClusterShardHostInfos res; + if (is_non_cluster) { + callback(res, expected_responses_cnt, responses_parsed, is_non_cluster); + return; + } + + auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; + std::set slot_bounds; + for (const auto& [_, response] : responses_by_id) { + for (const auto& [interval, _] : response) { + slot_bounds.insert(interval.slot_min); + slot_bounds.insert(interval.slot_max + 1); + } + } + if (slot_bounds.empty()) { + LOG_WARNING() + << log_extra() << "Failed to process CLUSTER SLOTS replies: responses_parsed=" << responses_parsed + << ", no slots info found"; + } else if (*slot_bounds.begin() != 0 || *std::prev(slot_bounds.end()) != kClusterHashSlots) { + LOG_ERROR() + << log_extra() << "Failed to process CLUSTER SLOTS replies: slot bounds begin=" << *slot_bounds.begin() + << ", end=" << *std::prev(slot_bounds.end()); + } + + if (!slot_bounds.empty() && *slot_bounds.begin() == 0 && *std::prev(slot_bounds.end()) == kClusterHashSlots) { + size_t prev = 0; + std::map master_count; + struct ShardInfo { + ConnectionInfoInt master; + std::set slot_intervals; + }; + std::map, ShardInfo> shard_infos; + + for (const size_t bound : slot_bounds) { + if (bound) { + const SlotInterval interval{prev, bound - 1}; + std::map, size_t> shard_stats; + for (const auto& [_, response] : responses_by_id) { + auto it = response.upper_bound(interval); + if (it != response.begin()) { + --it; + auto hosts = it->second.slaves; + hosts.insert(it->second.master); + ++shard_stats[hosts]; + ++master_count[it->second.master]; + } + } + + size_t max_count = 0; + // `best` - most popular set of hosts assigned to the current interval + // of hash slots among all host replies. + const std::set* best = nullptr; + + for (const auto& stat : shard_stats) { + if (stat.second > max_count) { + max_count = stat.second; + best = &stat.first; + } + } + if (best) { + shard_infos[*best].slot_intervals.insert(interval); + } + } + prev = bound; + } + + for (auto& shard_info : shard_infos) { + size_t max_count = 0; + const ConnectionInfoInt* master = nullptr; + for (const auto& host : shard_info.first) { + const size_t current = master_count[host]; + if (current > max_count) { + max_count = current; + master = &host; + } + } + UASSERT(master); + shard_info.second.master = *master; + + ClusterShardHostInfo host_info{shard_info.second.master, {}, {}}; + for (const auto& slave : shard_info.first) { + if (slave != host_info.master) { + host_info.slaves.push_back(slave); + } + } + host_info.slot_intervals = std::move(shard_info.second.slot_intervals); + res.push_back(std::move(host_info)); + } + + size_t shard_index = 0; + if (res.size() > shard_names.size()) { + LOG_ERROR() + << log_extra() << "Too many shards found: " << res.size() << ", maximum: " << shard_names.size(); + res = {}; + } else { + std::sort(res.begin(), res.end()); + for (auto& shard_host_info : res) { + shard_host_info.master.SetPassword(password); + shard_host_info.master.SetName(shard_names[shard_index]); + for (auto& slave : shard_host_info.slaves) { + slave.SetPassword(password); + slave.SetName(shard_names[shard_index]); + } + ++shard_index; + } + } + } + + callback(res, expected_responses_cnt, responses_parsed, is_non_cluster); +} + +void GetClusterSlotsContext::ProcessResponsesOnce() { + ProceResponseOnceImpl( + responses_by_id_, + callback_, + shard_group_name_, + *shard_names_, + password_, + expected_responses_cnt_.load(), + responses_parsed_.load(), + is_non_cluster_.load() + ); +} + +} // namespace storages::redis::impl + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_slots_query.hpp b/redis/src/storages/redis/impl/cluster_slots_query.hpp new file mode 100644 index 000000000000..eb047df9d47d --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_slots_query.hpp @@ -0,0 +1,134 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +#include +#include "shard.hpp" + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis::impl { + +static constexpr size_t kClusterHashSlots = 16384; + +struct GetClusterSlotsRequest { + GetClusterSlotsRequest(Shard& sentinel_shard, Password password, std::string shard_group_name) + : sentinel_shard(sentinel_shard), + command({"CLUSTER", "SLOTS"}), + password(std::move(password)), + shard_group_name(std::move(shard_group_name)) + {} + + Shard& sentinel_shard; + CmdArgs command; + + Password password; + std::string shard_group_name; +}; + +struct SlotInterval { + size_t slot_min; + size_t slot_max; + + SlotInterval(size_t slot_min, size_t slot_max) + : slot_min(slot_min), + slot_max(slot_max) + {} + + bool operator<(const SlotInterval& r) const { return slot_min < r.slot_min; } + bool operator==(const SlotInterval& r) const { return slot_min == r.slot_min && slot_max == r.slot_max; } +}; + +logging::LogHelper& operator<<(logging::LogHelper& log, SlotInterval interval); + +struct ClusterShardHostInfo { + ConnectionInfoInt master; + std::vector slaves; + std::set slot_intervals; + + bool operator<(const ClusterShardHostInfo& r) const { + UASSERT(!slot_intervals.empty()); + UASSERT(!r.slot_intervals.empty()); + return slot_intervals < r.slot_intervals; + } +}; + +using ClusterShardHostInfos = std::vector; + +using ProcessGetClusterHostsRequestCb = std::function< + void(ClusterShardHostInfos shard_infos, size_t requests_sent, size_t responses_parsed, bool is_non_cluster_error)>; + +struct MasterSlavesConnInfos { + ConnectionInfoInt master; + std::set slaves; +}; + +using ClusterSlotsResponse = std::map; + +class GetClusterSlotsContext { +public: + GetClusterSlotsContext( + Password password, + std::shared_ptr> shard_names, + std::string shard_group_name, + ProcessGetClusterHostsRequestCb&& callback, + size_t expected_responses_cnt + ); + + static void ProcessRequest( + std::shared_ptr> shard_names, + GetClusterSlotsRequest request, + ProcessGetClusterHostsRequestCb callback + ); + +private: + void OnAsyncCommandFailed(); + void OnResponse(const CommandPtr&, const ReplyPtr& reply); + void ProcessResponses(); + void ProcessResponsesOnce(); + + const std::string shard_group_name_; + const Password password_; + const std::shared_ptr> shard_names_; + const ProcessGetClusterHostsRequestCb callback_; + std::atomic response_got_{0}; + std::atomic responses_parsed_{0}; + std::atomic_flag process_responses_started_ ATOMIC_FLAG_INIT; + std::atomic expected_responses_cnt_{0}; + std::atomic is_non_cluster_{false}; + + std::mutex mutex_; + std::map responses_by_id_; +}; + +void ProceResponseOnceImpl( + const std::map& responses_by_id, + const ProcessGetClusterHostsRequestCb& callback, + const std::string& shard_group_name, + const std::vector& shard_names, + const Password& password, + size_t expected_responses_cnt, + size_t responses_parsed, + bool is_non_cluster +); + +enum class ClusterSlotsResponseStatus { + kOk, + kFail, + kNonCluster, +}; + +ClusterSlotsResponseStatus ParseClusterSlotsResponse( + const ReplyPtr& reply, + ClusterSlotsResponse& res, + const std::string& shard_group_name +); + +} // namespace storages::redis::impl + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_slots_query_test.cpp b/redis/src/storages/redis/impl/cluster_slots_query_test.cpp new file mode 100644 index 000000000000..369769aaff8d --- /dev/null +++ b/redis/src/storages/redis/impl/cluster_slots_query_test.cpp @@ -0,0 +1,197 @@ +#include +#include +#include + +USERVER_NAMESPACE_BEGIN + +TEST(ClusterSlotsQuery, ParseReplySimpleIps) { + using ReplyData = storages::redis::ReplyData; + using Array = ReplyData::Array; + constexpr auto kIp1 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:aaaa"; + constexpr auto kPort1 = 6379; + + constexpr auto kIp2 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:bbbb"; + constexpr auto kPort2 = 6380; + + constexpr auto kIp3 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:cccc"; + constexpr auto kPort3 = 6381; + + constexpr auto kIp4 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:dddd"; + constexpr auto kPort4 = 6382; + + constexpr auto kIp5 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:eeee"; + constexpr auto kPort5 = 6383; + + constexpr auto kIp6 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:ffff"; + constexpr auto kPort6 = 6384; + + auto reply = std::make_shared( + "CLUSTER SLOTS", + ReplyData{Array{ + Array{ + ReplyData{0}, + ReplyData{5460}, + Array{ + ReplyData{"klg-9.db.net"}, + ReplyData{kPort1}, + ReplyData{"92f260b22c5d1d2b1da1971c0244d268b3aaaaaa"}, + Array{ReplyData{"ip"}, ReplyData{kIp1}}, + }, + Array{ + ReplyData{"vla-i.db.net"}, + ReplyData{kPort2}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22bbbbbb"}, + Array{ReplyData{"ip"}, ReplyData{kIp2}}, + }, + }, + Array{ + ReplyData{5461}, + ReplyData{10922}, + Array{ + ReplyData{"klg-8.db.net"}, + ReplyData{kPort3}, + ReplyData{"294e10240d74f7d7eb9e8583645f08f3bdcccccc"}, + Array{ReplyData{"ip"}, ReplyData{kIp3}}, + }, + Array{ + ReplyData{"vla-3.db.net"}, + ReplyData{kPort4}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22dddddd"}, + Array{ReplyData{"ip"}, ReplyData{kIp4}}, + }, + }, + Array{ + ReplyData{10923}, + ReplyData{16383}, + Array{ + ReplyData{"klg-g.db.net"}, + ReplyData{kPort5}, + ReplyData{"294e10240d74f7d7eb9e8583645f08f3bd000000"}, + Array{ReplyData{"ip"}, ReplyData{kIp5}}, + }, + Array{ + ReplyData{"vla-e.db.net"}, + ReplyData{kPort6}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22111111"}, + Array{ReplyData{"ip"}, ReplyData{kIp6}}, + }, + }, + }} + ); + + namespace impl = storages::redis::impl; + impl::ClusterSlotsResponse response; + ASSERT_EQ(impl::ParseClusterSlotsResponse(reply, response, "redisdb"), impl::ClusterSlotsResponseStatus::kOk); + + using Pair = std::pair; + EXPECT_EQ(response.size(), 3); + + EXPECT_EQ(response[impl::SlotInterval(0, 5460)].master.HostPort(), Pair(kIp1, kPort1)); + ASSERT_EQ(response[impl::SlotInterval(0, 5460)].slaves.size(), 1); + EXPECT_EQ(response[impl::SlotInterval(0, 5460)].slaves.begin()->HostPort(), Pair(kIp2, kPort2)); + + EXPECT_EQ(response[impl::SlotInterval(5461, 10922)].master.HostPort(), Pair(kIp3, kPort3)); + ASSERT_EQ(response[impl::SlotInterval(5461, 10922)].slaves.size(), 1); + EXPECT_EQ(response[impl::SlotInterval(5461, 10922)].slaves.begin()->HostPort(), Pair(kIp4, kPort4)); + + EXPECT_EQ(response[impl::SlotInterval(10923, 16383)].master.HostPort(), Pair(kIp5, kPort5)); + ASSERT_EQ(response[impl::SlotInterval(10923, 16383)].slaves.size(), 1); + EXPECT_EQ(response[impl::SlotInterval(10923, 16383)].slaves.begin()->HostPort(), Pair(kIp6, kPort6)); +} + +TEST(ClusterSlotsQuery, ParseReplySimpleHostname) { + using ReplyData = storages::redis::ReplyData; + using Array = ReplyData::Array; + constexpr auto kIp1 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:aaaa"; + constexpr auto kPort1 = 6379; + + constexpr auto kIp2 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:bbbb"; + constexpr auto kPort2 = 6380; + + constexpr auto kIp3 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:cccc"; + constexpr auto kPort3 = 6381; + + constexpr auto kIp4 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:dddd"; + constexpr auto kPort4 = 6382; + + constexpr auto kIp5 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:eeee"; + constexpr auto kPort5 = 6383; + + constexpr auto kIp6 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:ffff"; + constexpr auto kPort6 = 6384; + + auto reply = std::make_shared( + "CLUSTER SLOTS", + ReplyData{Array{ + Array{ + ReplyData{0}, + ReplyData{5460}, + Array{ + ReplyData{kIp1}, + ReplyData{kPort1}, + ReplyData{"92f260b22c5d1d2b1da1971c0244d268b3aaaaaa"}, + Array{ReplyData{"hostname"}, ReplyData{"klg-9.db.net"}}, + }, + Array{ + ReplyData{kIp2}, + ReplyData{kPort2}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22bbbbbb"}, + Array{ReplyData{"hostname"}, ReplyData{"vla-i.db.net"}}, + }, + }, + Array{ + ReplyData{5461}, + ReplyData{16383}, + Array{ + ReplyData{kIp3}, + ReplyData{kPort3}, + ReplyData{"294e10240d74f7d7eb9e8583645f08f3bdcccccc"}, + Array{ReplyData{"hostname"}, ReplyData{"klg-8.db.net"}}, + }, + Array{ + ReplyData{kIp4}, + ReplyData{kPort4}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22dddddd"}, + Array{ReplyData{"hostname"}, ReplyData{"vla-3.db.net"}}, + }, + Array{ + ReplyData{kIp5}, + ReplyData{kPort5}, + ReplyData{"294e10240d74f7d7eb9e8583645f08f3bd000000"}, + Array{ReplyData{"hostname"}, ReplyData{"klg-g.db.net"}}, + }, + Array{ + ReplyData{kIp6}, + ReplyData{kPort6}, + ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22111111"}, + Array{ReplyData{"hostname"}, ReplyData{"vla-e.db.net"}}, + }, + }, + }} + ); + + namespace impl = storages::redis::impl; + impl::ClusterSlotsResponse response; + ASSERT_EQ(impl::ParseClusterSlotsResponse(reply, response, "redisdb"), impl::ClusterSlotsResponseStatus::kOk); + + using Pair = std::pair; + EXPECT_EQ(response.size(), 2); + + EXPECT_EQ(response[impl::SlotInterval(0, 5460)].master.HostPort(), Pair(kIp1, kPort1)); + ASSERT_EQ(response[impl::SlotInterval(0, 5460)].slaves.size(), 1); + EXPECT_EQ(response[impl::SlotInterval(0, 5460)].slaves.begin()->HostPort(), Pair(kIp2, kPort2)); + + EXPECT_EQ(response[impl::SlotInterval(5461, 16383)].master.HostPort(), Pair(kIp3, kPort3)); + ASSERT_EQ(response[impl::SlotInterval(5461, 16383)].slaves.size(), 3); + + std::set host_ports = { + {kIp4, kPort4}, + {kIp5, kPort5}, + {kIp6, kPort6}, + }; + for (const auto& slave : response[impl::SlotInterval(5461, 16383)].slaves) { + EXPECT_TRUE(host_ports.extract(slave.HostPort())); + } +} + +USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/cluster_topology.hpp b/redis/src/storages/redis/impl/cluster_topology.hpp index 89da780656c4..0b7de94827b9 100644 --- a/redis/src/storages/redis/impl/cluster_topology.hpp +++ b/redis/src/storages/redis/impl/cluster_topology.hpp @@ -11,7 +11,7 @@ #include #include -#include +#include USERVER_NAMESPACE_BEGIN diff --git a/redis/src/storages/redis/impl/cluster_topology_holder.cpp b/redis/src/storages/redis/impl/cluster_topology_holder.cpp index e20de1a8c757..2715e9b7f4f8 100644 --- a/redis/src/storages/redis/impl/cluster_topology_holder.cpp +++ b/redis/src/storages/redis/impl/cluster_topology_holder.cpp @@ -7,6 +7,7 @@ #include #include #include +#include "cluster_shards_query.hpp" USERVER_NAMESPACE_BEGIN @@ -131,7 +132,8 @@ ClusterTopologyHolder::ClusterTopologyHolder( Password password, const std::vector& /*shards*/, const std::vector& conns, - ConnectionSecurity connection_security + ConnectionSecurity connection_security, + TopologyUpdateMethod topology_update_method ) : ev_thread_(sentinel_thread_control), redis_thread_pool_(redis_thread_pool), @@ -139,6 +141,7 @@ ClusterTopologyHolder::ClusterTopologyHolder( password_(std::move(password)), shards_names_(MakeShardNames()), conns_(conns), + topology_update_method_(topology_update_method), statistics_holder_(), update_topology_timer_(ev_thread_, [this] { UpdateClusterTopology(); }, kSentinelGetHostsCheckInterval), update_topology_watch_( @@ -493,17 +496,13 @@ void ClusterTopologyHolder::UpdateClusterTopology() { /// Update sentinel sentinels_->ProcessCreation(redis_thread_pool_); - /// Update controlled topology. Go to CLUSTER SLOTS - /// ... - ProcessGetClusterHostsRequest( - shards_names_, - GetClusterHostsRequest(*sentinels_, GetPassword(), shard_group_name_), + auto callback = [this, reset{std::move(reset_update_cluster_slots) }](ClusterShardHostInfos shard_infos, size_t requests_sent, size_t responses_parsed, bool is_non_cluster_error ) { LOG_DEBUG() - << log_extra_ << "Parsing response from cluster slots: shard_infos.size(): " << shard_infos.size() + << log_extra_ << "Parsing response from cluster shards: shard_infos.size(): " << shard_infos.size() << ", requests_sent=" << requests_sent << ", responses_parsed=" << responses_parsed; const auto deferred = utils::FastScopeGuard([&]() noexcept { ++cluster_slots_call_counter; }); if (is_non_cluster_error) { @@ -590,8 +589,24 @@ void ClusterTopologyHolder::UpdateClusterTopology() { LOG_DEBUG() << log_extra_ << "Cluster topology updated to version" << current_topology_version_.load(); }); - } - ); + }; + + switch (topology_update_method_) { + case TopologyUpdateMethod::kClusterShards: + GetClusterShardsContext::ProcessRequest( + shards_names_, + GetClusterShardsRequest(*sentinels_, GetPassword(), shard_group_name_), + std::move(callback) + ); + break; + case TopologyUpdateMethod::kClusterSlots: + GetClusterSlotsContext::ProcessRequest( + shards_names_, + GetClusterSlotsRequest(*sentinels_, GetPassword(), shard_group_name_), + std::move(callback) + ); + break; + } } void ClusterTopologyHolder::GetStatistics(SentinelStatistics& stats, const MetricsSettings& settings) const { diff --git a/redis/src/storages/redis/impl/cluster_topology_holder.hpp b/redis/src/storages/redis/impl/cluster_topology_holder.hpp index 8f91d836e183..9b26a07aabb0 100644 --- a/redis/src/storages/redis/impl/cluster_topology_holder.hpp +++ b/redis/src/storages/redis/impl/cluster_topology_holder.hpp @@ -8,6 +8,7 @@ #include #include +#include USERVER_NAMESPACE_BEGIN @@ -28,7 +29,8 @@ class ClusterTopologyHolder final : public TopologyHolderBase { Password password, const std::vector& /*shards*/, const std::vector& conns, - ConnectionSecurity connection_security + ConnectionSecurity connection_security, + TopologyUpdateMethod topology_update_method ); ~ClusterTopologyHolder() override = default; @@ -64,6 +66,7 @@ class ClusterTopologyHolder final : public TopologyHolderBase { concurrent::Variable password_; std::shared_ptr> shards_names_; const std::vector conns_; + const TopologyUpdateMethod topology_update_method_; std::shared_ptr sentinels_; StatisticsHolder statistics_holder_; diff --git a/redis/src/storages/redis/impl/mock_server_test.cpp b/redis/src/storages/redis/impl/mock_server_test.cpp index 95f55e5cb316..80844a7f0df1 100644 --- a/redis/src/storages/redis/impl/mock_server_test.cpp +++ b/redis/src/storages/redis/impl/mock_server_test.cpp @@ -292,6 +292,40 @@ MockRedisServer::HandlerPtr MockRedisServer::RegisterClusterNodes( }); } RegisterHandlerWithConstReply("CLUSTER", {"SLOTS"}, {std::move(slots_reply_data)}); + + std::vector shards_reply_data; + shards_reply_data.reserve(masters.size()); + for (std::size_t i = 0; i < masters.size(); ++i) { + // Build slot interval for this shard (same as in CLUSTER SLOTS) + const int slot_min = utils::numeric_cast(i * slots_chunk_size); + const int + slot_max = utils::numeric_cast(i + 1 == masters.size() ? kSlotsCount - 1 : (i + 1) * slots_chunk_size); + // Master node description + storages::redis::ReplyData master_node = storages::redis::ReplyData::Array{ + {"ip"}, + {masters[i].ip}, + {"port"}, + {masters[i].port}, + {"role"}, + {"master"}, + {"replication-offset"}, + {0}, + {"health"}, + {"online"}, + }; + // Slave node description + storages::redis::ReplyData slave_node = + storages::redis::ReplyData::Array{{"ip"}, {slaves[i].ip}, {"port"}, {slaves[i].port}, {"role"}, {"slave"}}; + // Assemble the shard entry: a map with "slots" and "nodes" + storages::redis::ReplyData shard = storages::redis::ReplyData::Array{ + {"slots"}, + storages::redis::ReplyData::Array{slot_min, slot_max}, + {"nodes"}, + storages::redis::ReplyData::Array{std::move(master_node), std::move(slave_node)} + }; + shards_reply_data.emplace_back(std::move(shard)); + } + RegisterHandlerWithConstReply("CLUSTER", {"SHARDS"}, {std::move(shards_reply_data)}); return handler; } diff --git a/redis/src/storages/redis/impl/sentinel.cpp b/redis/src/storages/redis/impl/sentinel.cpp index 674d5b620453..21b6967cc6d7 100644 --- a/redis/src/storages/redis/impl/sentinel.cpp +++ b/redis/src/storages/redis/impl/sentinel.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -72,7 +73,8 @@ Sentinel::Sentinel( KeyShardFactory key_shard_factory, CommandControl command_control, const testsuite::RedisControl& testsuite_redis_control, - std::size_t database_index + std::size_t database_index, + TopologyUpdateMethod topology_update_method ) : shard_group_name_(shard_group_name), thread_pools_(thread_pools), @@ -105,7 +107,8 @@ Sentinel::Sentinel( connection_security, std::move(key_shard_factory), dynamic_config_source, - database_index + database_index, + topology_update_method ); }); } @@ -132,7 +135,8 @@ std::shared_ptr Sentinel::CreateSentinel( const std::string& client_name, KeyShardFactory key_shard_factory, const CommandControl& command_control, - const testsuite::RedisControl& testsuite_redis_control + const testsuite::RedisControl& testsuite_redis_control, + TopologyUpdateMethod topology_update_method ) { const auto& password = settings.password; const auto& sentinel_password = settings.sentinel_password; @@ -174,7 +178,8 @@ std::shared_ptr Sentinel::CreateSentinel( std::move(key_shard_factory), command_control, testsuite_redis_control, - settings.database_index + settings.database_index, + topology_update_method ); client->Start(); } diff --git a/redis/src/storages/redis/impl/sentinel.hpp b/redis/src/storages/redis/impl/sentinel.hpp index 2afb6e4da9f4..a054881aec3a 100644 --- a/redis/src/storages/redis/impl/sentinel.hpp +++ b/redis/src/storages/redis/impl/sentinel.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -69,7 +70,8 @@ class Sentinel { KeyShardFactory key_shard_factory, CommandControl command_control, const testsuite::RedisControl& testsuite_redis_control, - std::size_t database_index + std::size_t database_index, + TopologyUpdateMethod topology_update_method ); virtual ~Sentinel(); @@ -99,7 +101,8 @@ class Sentinel { const std::string& client_name, KeyShardFactory key_shard_factory, const CommandControl& command_control = {}, - const testsuite::RedisControl& testsuite_redis_control = {} + const testsuite::RedisControl& testsuite_redis_control = {}, + TopologyUpdateMethod topology_update_method = TopologyUpdateMethod::kClusterSlots ); void AsyncCommand(CommandPtr command, bool master = true, size_t shard = 0); diff --git a/redis/src/storages/redis/impl/sentinel_impl.cpp b/redis/src/storages/redis/impl/sentinel_impl.cpp index 18b4defca826..cb3d40bd02b4 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.cpp +++ b/redis/src/storages/redis/impl/sentinel_impl.cpp @@ -225,7 +225,8 @@ SentinelImpl::SentinelImpl( ConnectionSecurity connection_security, KeyShardFactory&& key_shard_factory, dynamic_config::Source dynamic_config_source, - std::size_t database_index + std::size_t database_index, + TopologyUpdateMethod topology_update_method ) : sentinel_obj_(sentinel), ev_thread_(sentinel_thread_control), @@ -255,7 +256,8 @@ SentinelImpl::SentinelImpl( password, shards, conns, - connection_security + connection_security, + topology_update_method ); } else if (key_shard_type == ShardingStrategy::kRedisStandalone) { LOG_DEBUG() << log_extra_ << "Construct Standalone topology holder"; diff --git a/redis/src/storages/redis/impl/sentinel_impl.hpp b/redis/src/storages/redis/impl/sentinel_impl.hpp index c9bc73415a9d..e0e140362b56 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.hpp +++ b/redis/src/storages/redis/impl/sentinel_impl.hpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "shard.hpp" @@ -66,7 +67,8 @@ class SentinelImpl { ConnectionSecurity connection_security, KeyShardFactory&& key_shard_factory, dynamic_config::Source dynamic_config_source, - std::size_t database_index + std::size_t database_index, + TopologyUpdateMethod topology_update_method ); ~SentinelImpl(); diff --git a/redis/src/storages/redis/impl/sentinel_query.cpp b/redis/src/storages/redis/impl/sentinel_query.cpp index 0697753e5df2..266f0fb3fdad 100644 --- a/redis/src/storages/redis/impl/sentinel_query.cpp +++ b/redis/src/storages/redis/impl/sentinel_query.cpp @@ -237,150 +237,8 @@ std::string InstanceUpChecker::GetReason() const { UINVARIANT(false, "Unknown reason"); } - -std::optional GetIpFromMeta(const ReplyData::Array& host_info_array) { - if (host_info_array.size() < 4) { - return std::nullopt; - } - const auto& meta = host_info_array[3]; - if (!meta.IsArray() || meta.GetSize() < 2) { - return std::nullopt; - } - - const auto& array = meta.GetArray(); - if (!array[0].IsString() || !array[1].IsString() || array[0].GetString() != "ip") { - return std::nullopt; - } - return array[1].GetString(); -} - -std::string GetIpFromHostInfo(const ReplyData::Array& host_info_array) { - auto from_meta = GetIpFromMeta(host_info_array); - if (from_meta) { - return *from_meta; - } - return host_info_array[0].GetString(); -} - } // namespace -ClusterSlotsResponseStatus ParseClusterSlotsResponse( - const ReplyPtr& reply, - ClusterSlotsResponse& res, - const std::string& shard_group_name -) { - UASSERT(reply); - auto log_extra = [&shard_group_name] { return logging::LogExtra({{"shard_group_name", shard_group_name}}); }; - - LOG_TRACE() << log_extra() << "Got reply to CLUSTER SLOTS: " << reply->data.ToDebugString(); - if (reply->IsUnknownCommandError()) { - LOG_ERROR() - << log_extra() - << "Redis CLUSTER SLOTS reply contains unknown command error: " << reply->data.ToDebugString(); - return ClusterSlotsResponseStatus::kNonCluster; - } - if (reply->status != ReplyStatus::kOk) { - LOG_ERROR() << log_extra() << "Redis CLUSTER SLOTS reply contains error: " << reply->data.ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - if (!reply->data.IsArray()) { - LOG_ERROR() << log_extra() << "Redis CLUSTER SLOTS reply is not an array: " << reply->data.ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - std::size_t array_index = 0; - for (const auto& reply_interval : reply->data.GetArray()) { - if (!reply_interval.IsArray()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index - << "] is not an array: " << reply_interval.ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - const auto& array = reply_interval.GetArray(); - if (array.size() < 3) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index - << "] is an array of size less than 3: " << reply_interval.ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - if (!array[0].IsInt()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index - << "][0] is not an int: " << array[0].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - if (!array[1].IsInt()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index - << "][1] is not an int: " << array[1].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - for (std::size_t i = 2; i < array.size(); i++) { - if (!array[i].IsArray()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i - << "] is not an array: " << array[i].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - const auto& host_info_array = array[i].GetArray(); - if (host_info_array.size() < 2) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i - << "] is an array of size less than 2: " << array[i].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - if (!host_info_array[0].IsString()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i - << "][0] is not a string: " << host_info_array[0].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - if (!host_info_array[1].IsInt()) { - LOG_ERROR() - << log_extra() << "Redis CLUSTER SLOTS reply element[" << array_index << "][" << i - << "][1] is not an int: " << host_info_array[1].ToDebugString(); - return ClusterSlotsResponseStatus::kFail; - } - - ConnectionInfoInt conn_info{ - {GetIpFromHostInfo(host_info_array), static_cast(host_info_array[1].GetInt()), {}} - }; - const SlotInterval slot_interval(array[0].GetInt(), array[1].GetInt()); - if (i == 2) { - const bool is_master_overwritten = - (!res[slot_interval].master.HostPort().first.empty() && - res[slot_interval].master.HostPort().first != "localhost" && - res[slot_interval].master.HostPort() != conn_info.HostPort()); - if (is_master_overwritten) { - const auto message = fmt::format( - "Redis CLUSTER SLOTS reply element[{}][{}] overwrites master '{}' with '{}'", - array_index, - i, - res[slot_interval].master.HostPort().first, - conn_info.HostPort().first - ); - LOG_ERROR() << log_extra() << message; - UASSERT_MSG(false, message); - } - - res[slot_interval].master = std::move(conn_info); - } else { - res[slot_interval].slaves.insert(std::move(conn_info)); - } - } - - ++array_index; - } - return ClusterSlotsResponseStatus::kOk; -} - GetHostsContext::GetHostsContext( bool allow_empty, const Password& password, @@ -482,199 +340,6 @@ void ProcessGetHostsRequest(GetHostsRequest request, ProcessGetHostsRequestCb ca } } -logging::LogHelper& operator<<(logging::LogHelper& log, SlotInterval interval) { - log << '[' << interval.slot_min << ',' << interval.slot_max << ']'; - return log; -} - -void ProcessGetClusterHostsRequest( - std::shared_ptr> shard_names, - GetClusterHostsRequest request, - ProcessGetClusterHostsRequestCb callback -) { - auto ids = request.sentinel_shard.GetAllInstancesServerId(); - auto context = std::make_shared( - request.password, - std::move(shard_names), - request.shard_group_name, - std::move(callback), - ids.size() - ); - - for (const auto& id : ids) { - auto cmd = PrepareCommand(request.command.Clone(), [context](const CommandPtr& command, const ReplyPtr& reply) { - context->OnResponse(command, reply); - }); - cmd->control.force_server_id = id; - if (!request.sentinel_shard.AsyncCommand(cmd)) { - context->OnAsyncCommandFailed(); - } - } -} - -GetClusterHostsContext::GetClusterHostsContext( - Password password, - std::shared_ptr> shard_names, - std::string shard_group_name, - ProcessGetClusterHostsRequestCb&& callback, - size_t expected_responses_cnt -) - : shard_group_name_(std::move(shard_group_name)), - password_(std::move(password)), - shard_names_(std::move(shard_names)), - callback_(std::move(callback)), - expected_responses_cnt_(expected_responses_cnt) -{} - -void GetClusterHostsContext::OnAsyncCommandFailed() { - --expected_responses_cnt_; - - ProcessResponses(); -} - -void GetClusterHostsContext::OnResponse(const CommandPtr&, const ReplyPtr& reply) { - ClusterSlotsResponse response; - switch (ParseClusterSlotsResponse(reply, response, shard_group_name_)) { - case ClusterSlotsResponseStatus::kOk: { - { - const std::lock_guard lock(mutex_); - responses_by_id_[reply->server_id] = std::move(response); - } - responses_parsed_++; - break; - } - case ClusterSlotsResponseStatus::kFail: - break; - case ClusterSlotsResponseStatus::kNonCluster: - is_non_cluster_ = true; - break; - } - - response_got_++; - - ProcessResponses(); -} - -void GetClusterHostsContext::ProcessResponses() { - if (response_got_ >= expected_responses_cnt_ || is_non_cluster_) { - if (!process_responses_started_.test_and_set()) { - ProcessResponsesOnce(); - } - } -} - -void GetClusterHostsContext::ProcessResponsesOnce() { - ClusterShardHostInfos res; - if (is_non_cluster_) { - callback_(res, expected_responses_cnt_, responses_parsed_, is_non_cluster_); - return; - } - - auto log_extra = [this] { return logging::LogExtra({{"shard_group_name", shard_group_name_}}); }; - std::set slot_bounds; - for (const auto& [_, response] : responses_by_id_) { - for (const auto& [interval, _] : response) { - slot_bounds.insert(interval.slot_min); - slot_bounds.insert(interval.slot_max + 1); - } - } - if (slot_bounds.empty()) { - LOG_WARNING() - << log_extra() << "Failed to process CLUSTER SLOTS replies: responses_parsed=" << responses_parsed_ - << ", no slots info found"; - } else if (*slot_bounds.begin() != 0 || *std::prev(slot_bounds.end()) != kClusterHashSlots) { - LOG_ERROR() - << log_extra() << "Failed to process CLUSTER SLOTS replies: slot bounds begin=" << *slot_bounds.begin() - << ", end=" << *std::prev(slot_bounds.end()); - } - - if (!slot_bounds.empty() && *slot_bounds.begin() == 0 && *std::prev(slot_bounds.end()) == kClusterHashSlots) { - size_t prev = 0; - std::map master_count; - struct ShardInfo { - ConnectionInfoInt master; - std::set slot_intervals; - }; - std::map, ShardInfo> shard_infos; - - for (const size_t bound : slot_bounds) { - if (bound) { - const SlotInterval interval{prev, bound - 1}; - std::map, size_t> shard_stats; - for (const auto& [_, response] : responses_by_id_) { - auto it = response.upper_bound(interval); - if (it != response.begin()) { - --it; - auto hosts = it->second.slaves; - hosts.insert(it->second.master); - ++shard_stats[hosts]; - ++master_count[it->second.master]; - } - } - - size_t max_count = 0; - // `best` - most popular set of hosts assigned to the current interval - // of hash slots among all host replies. - const std::set* best = nullptr; - - for (const auto& stat : shard_stats) { - if (stat.second > max_count) { - max_count = stat.second; - best = &stat.first; - } - } - if (best) { - shard_infos[*best].slot_intervals.insert(interval); - } - } - prev = bound; - } - - for (auto& shard_info : shard_infos) { - size_t max_count = 0; - const ConnectionInfoInt* master = nullptr; - for (const auto& host : shard_info.first) { - const size_t current = master_count[host]; - if (current > max_count) { - max_count = current; - master = &host; - } - } - UASSERT(master); - shard_info.second.master = *master; - - ClusterShardHostInfo host_info{shard_info.second.master, {}, {}}; - for (const auto& slave : shard_info.first) { - if (slave != host_info.master) { - host_info.slaves.push_back(slave); - } - } - host_info.slot_intervals = std::move(shard_info.second.slot_intervals); - res.push_back(std::move(host_info)); - } - - size_t shard_index = 0; - if (res.size() > shard_names_->size()) { - LOG_ERROR() - << log_extra() << "Too many shards found: " << res.size() << ", maximum: " << shard_names_->size(); - res = {}; - } else { - std::sort(res.begin(), res.end()); - for (auto& shard_host_info : res) { - shard_host_info.master.SetPassword(password_); - shard_host_info.master.SetName((*shard_names_)[shard_index]); - for (auto& slave : shard_host_info.slaves) { - slave.SetPassword(password_); - slave.SetName((*shard_names_)[shard_index]); - } - ++shard_index; - } - } - } - - callback_(res, expected_responses_cnt_, responses_parsed_, is_non_cluster_); -} - } // namespace storages::redis::impl USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/sentinel_query.hpp b/redis/src/storages/redis/impl/sentinel_query.hpp index 077efb2cbf3d..45a86d0d187b 100644 --- a/redis/src/storages/redis/impl/sentinel_query.hpp +++ b/redis/src/storages/redis/impl/sentinel_query.hpp @@ -88,116 +88,6 @@ class GetHostsContext : public std::enable_shared_from_this { std::map responses_by_name_; }; -static constexpr size_t kClusterHashSlots = 16384; - -struct GetClusterHostsRequest { - GetClusterHostsRequest(Shard& sentinel_shard, Password password, std::string shard_group_name) - : sentinel_shard(sentinel_shard), - command({"CLUSTER", "SLOTS"}), - password(std::move(password)), - shard_group_name(std::move(shard_group_name)) - {} - - Shard& sentinel_shard; - CmdArgs command; - - Password password; - std::string shard_group_name; -}; - -struct SlotInterval { - size_t slot_min; - size_t slot_max; - - SlotInterval(size_t slot_min, size_t slot_max) - : slot_min(slot_min), - slot_max(slot_max) - {} - - bool operator<(const SlotInterval& r) const { return slot_min < r.slot_min; } - bool operator==(const SlotInterval& r) const { return slot_min == r.slot_min && slot_max == r.slot_max; } -}; - -logging::LogHelper& operator<<(logging::LogHelper& log, SlotInterval interval); - -struct ClusterShardHostInfo { - ConnectionInfoInt master; - std::vector slaves; - std::set slot_intervals; - - bool operator<(const ClusterShardHostInfo& r) const { - UASSERT(!slot_intervals.empty()); - UASSERT(!r.slot_intervals.empty()); - return slot_intervals < r.slot_intervals; - } -}; - -using ClusterShardHostInfos = std::vector; - -using ProcessGetClusterHostsRequestCb = std::function< - void(ClusterShardHostInfos shard_infos, size_t requests_sent, size_t responses_parsed, bool is_non_cluster_error)>; - -void ProcessGetClusterHostsRequest( - std::shared_ptr> shard_names, - GetClusterHostsRequest request, - ProcessGetClusterHostsRequestCb callback -); - -struct MasterSlavesConnInfos { - ConnectionInfoInt master; - std::set slaves; -}; - -using ClusterSlotsResponse = std::map; - -class GetClusterHostsContext { -public: - GetClusterHostsContext( - Password password, - std::shared_ptr> shard_names, - std::string shard_group_name, - ProcessGetClusterHostsRequestCb&& callback, - size_t expected_responses_cnt - ); - -private: - friend void ProcessGetClusterHostsRequest( - std::shared_ptr> shard_names, - GetClusterHostsRequest request, - ProcessGetClusterHostsRequestCb callback - ); - - void OnAsyncCommandFailed(); - void OnResponse(const CommandPtr&, const ReplyPtr& reply); - void ProcessResponses(); - void ProcessResponsesOnce(); - - const std::string shard_group_name_; - const Password password_; - const std::shared_ptr> shard_names_; - const ProcessGetClusterHostsRequestCb callback_; - std::atomic response_got_{0}; - std::atomic responses_parsed_{0}; - std::atomic_flag process_responses_started_ ATOMIC_FLAG_INIT; - std::atomic expected_responses_cnt_{0}; - std::atomic is_non_cluster_{false}; - - std::mutex mutex_; - std::map responses_by_id_; -}; - -enum class ClusterSlotsResponseStatus { - kOk, - kFail, - kNonCluster, -}; - -ClusterSlotsResponseStatus ParseClusterSlotsResponse( - const ReplyPtr& reply, - ClusterSlotsResponse& res, - const std::string& shard_group_name -); - } // namespace storages::redis::impl USERVER_NAMESPACE_END diff --git a/redis/src/storages/redis/impl/sentinel_query_test.cpp b/redis/src/storages/redis/impl/sentinel_query_test.cpp index 0d24bd72ee40..4b2bcd6fcdec 100644 --- a/redis/src/storages/redis/impl/sentinel_query_test.cpp +++ b/redis/src/storages/redis/impl/sentinel_query_test.cpp @@ -26,196 +26,6 @@ TEST(SentinelQuery, SingleBadReply) { EXPECT_EQ(1, called); } -TEST(SentinelQuery, ParseReplySimpleIps) { - using ReplyData = storages::redis::ReplyData; - using Array = ReplyData::Array; - constexpr auto kIp1 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:aaaa"; - constexpr auto kPort1 = 6379; - - constexpr auto kIp2 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:bbbb"; - constexpr auto kPort2 = 6380; - - constexpr auto kIp3 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:cccc"; - constexpr auto kPort3 = 6381; - - constexpr auto kIp4 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:dddd"; - constexpr auto kPort4 = 6382; - - constexpr auto kIp5 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:eeee"; - constexpr auto kPort5 = 6383; - - constexpr auto kIp6 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:ffff"; - constexpr auto kPort6 = 6384; - - auto reply = std::make_shared( - "CLUSTER SLOTS", - ReplyData{Array{ - Array{ - ReplyData{0}, - ReplyData{5460}, - Array{ - ReplyData{"klg-9.db.net"}, - ReplyData{kPort1}, - ReplyData{"92f260b22c5d1d2b1da1971c0244d268b3aaaaaa"}, - Array{ReplyData{"ip"}, ReplyData{kIp1}}, - }, - Array{ - ReplyData{"vla-i.db.net"}, - ReplyData{kPort2}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22bbbbbb"}, - Array{ReplyData{"ip"}, ReplyData{kIp2}}, - }, - }, - Array{ - ReplyData{5461}, - ReplyData{10922}, - Array{ - ReplyData{"klg-8.db.net"}, - ReplyData{kPort3}, - ReplyData{"294e10240d74f7d7eb9e8583645f08f3bdcccccc"}, - Array{ReplyData{"ip"}, ReplyData{kIp3}}, - }, - Array{ - ReplyData{"vla-3.db.net"}, - ReplyData{kPort4}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22dddddd"}, - Array{ReplyData{"ip"}, ReplyData{kIp4}}, - }, - }, - Array{ - ReplyData{10923}, - ReplyData{16383}, - Array{ - ReplyData{"klg-g.db.net"}, - ReplyData{kPort5}, - ReplyData{"294e10240d74f7d7eb9e8583645f08f3bd000000"}, - Array{ReplyData{"ip"}, ReplyData{kIp5}}, - }, - Array{ - ReplyData{"vla-e.db.net"}, - ReplyData{kPort6}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22111111"}, - Array{ReplyData{"ip"}, ReplyData{kIp6}}, - }, - }, - }} - ); - - namespace impl = storages::redis::impl; - impl::ClusterSlotsResponse response; - ASSERT_EQ(impl::ParseClusterSlotsResponse(reply, response, "redisdb"), impl::ClusterSlotsResponseStatus::kOk); - - using Pair = std::pair; - EXPECT_EQ(response.size(), 3); - - EXPECT_EQ(response[impl::SlotInterval(0, 5460)].master.HostPort(), Pair(kIp1, kPort1)); - ASSERT_EQ(response[impl::SlotInterval(0, 5460)].slaves.size(), 1); - EXPECT_EQ(response[impl::SlotInterval(0, 5460)].slaves.begin()->HostPort(), Pair(kIp2, kPort2)); - - EXPECT_EQ(response[impl::SlotInterval(5461, 10922)].master.HostPort(), Pair(kIp3, kPort3)); - ASSERT_EQ(response[impl::SlotInterval(5461, 10922)].slaves.size(), 1); - EXPECT_EQ(response[impl::SlotInterval(5461, 10922)].slaves.begin()->HostPort(), Pair(kIp4, kPort4)); - - EXPECT_EQ(response[impl::SlotInterval(10923, 16383)].master.HostPort(), Pair(kIp5, kPort5)); - ASSERT_EQ(response[impl::SlotInterval(10923, 16383)].slaves.size(), 1); - EXPECT_EQ(response[impl::SlotInterval(10923, 16383)].slaves.begin()->HostPort(), Pair(kIp6, kPort6)); -} - -TEST(SentinelQuery, ParseReplySimpleHostname) { - using ReplyData = storages::redis::ReplyData; - using Array = ReplyData::Array; - constexpr auto kIp1 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:aaaa"; - constexpr auto kPort1 = 6379; - - constexpr auto kIp2 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:bbbb"; - constexpr auto kPort2 = 6380; - - constexpr auto kIp3 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:cccc"; - constexpr auto kPort3 = 6381; - - constexpr auto kIp4 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:dddd"; - constexpr auto kPort4 = 6382; - - constexpr auto kIp5 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:eeee"; - constexpr auto kPort5 = 6383; - - constexpr auto kIp6 = "2a02:6b8:c2d:3d21:7a01:1405:4c4f:ffff"; - constexpr auto kPort6 = 6384; - - auto reply = std::make_shared( - "CLUSTER SLOTS", - ReplyData{Array{ - Array{ - ReplyData{0}, - ReplyData{5460}, - Array{ - ReplyData{kIp1}, - ReplyData{kPort1}, - ReplyData{"92f260b22c5d1d2b1da1971c0244d268b3aaaaaa"}, - Array{ReplyData{"hostname"}, ReplyData{"klg-9.db.net"}}, - }, - Array{ - ReplyData{kIp2}, - ReplyData{kPort2}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22bbbbbb"}, - Array{ReplyData{"hostname"}, ReplyData{"vla-i.db.net"}}, - }, - }, - Array{ - ReplyData{5461}, - ReplyData{16383}, - Array{ - ReplyData{kIp3}, - ReplyData{kPort3}, - ReplyData{"294e10240d74f7d7eb9e8583645f08f3bdcccccc"}, - Array{ReplyData{"hostname"}, ReplyData{"klg-8.db.net"}}, - }, - Array{ - ReplyData{kIp4}, - ReplyData{kPort4}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22dddddd"}, - Array{ReplyData{"hostname"}, ReplyData{"vla-3.db.net"}}, - }, - Array{ - ReplyData{kIp5}, - ReplyData{kPort5}, - ReplyData{"294e10240d74f7d7eb9e8583645f08f3bd000000"}, - Array{ReplyData{"hostname"}, ReplyData{"klg-g.db.net"}}, - }, - Array{ - ReplyData{kIp6}, - ReplyData{kPort6}, - ReplyData{"4732a74a0c4c9fe245eb8098ff6c2b1b22111111"}, - Array{ReplyData{"hostname"}, ReplyData{"vla-e.db.net"}}, - }, - }, - }} - ); - - namespace impl = storages::redis::impl; - impl::ClusterSlotsResponse response; - ASSERT_EQ(impl::ParseClusterSlotsResponse(reply, response, "redisdb"), impl::ClusterSlotsResponseStatus::kOk); - - using Pair = std::pair; - EXPECT_EQ(response.size(), 2); - - EXPECT_EQ(response[impl::SlotInterval(0, 5460)].master.HostPort(), Pair(kIp1, kPort1)); - ASSERT_EQ(response[impl::SlotInterval(0, 5460)].slaves.size(), 1); - EXPECT_EQ(response[impl::SlotInterval(0, 5460)].slaves.begin()->HostPort(), Pair(kIp2, kPort2)); - - EXPECT_EQ(response[impl::SlotInterval(5461, 16383)].master.HostPort(), Pair(kIp3, kPort3)); - ASSERT_EQ(response[impl::SlotInterval(5461, 16383)].slaves.size(), 3); - - std::set host_ports = { - {kIp4, kPort4}, - {kIp5, kPort5}, - {kIp6, kPort6}, - }; - for (const auto& slave : response[impl::SlotInterval(5461, 16383)].slaves) { - EXPECT_TRUE(host_ports.extract(slave.HostPort())); - } -} - std::shared_ptr GenerateReply( const std::string& ip, bool master, diff --git a/redis/src/storages/redis/impl/subscribe_sentinel.cpp b/redis/src/storages/redis/impl/subscribe_sentinel.cpp index 916dc3d48fca..a6c9d06fd48a 100644 --- a/redis/src/storages/redis/impl/subscribe_sentinel.cpp +++ b/redis/src/storages/redis/impl/subscribe_sentinel.cpp @@ -45,7 +45,8 @@ SubscribeSentinel::SubscribeSentinel( KeyShardFactory key_shard_factory, bool is_cluster_mode, CommandControl command_control, - const testsuite::RedisControl& testsuite_redis_control + const testsuite::RedisControl& testsuite_redis_control, + TopologyUpdateMethod topology_update_method ) : Sentinel( thread_pools, @@ -59,7 +60,8 @@ SubscribeSentinel::SubscribeSentinel( std::move(key_shard_factory), command_control, testsuite_redis_control, - kSubscriptionDatabaseIndex + kSubscriptionDatabaseIndex, + topology_update_method ), storage_(CreateSubscriptionStorage(thread_pools, shards, is_cluster_mode)) { @@ -79,7 +81,8 @@ std::shared_ptr SubscribeSentinel::Create( const std::string& client_name, storages::redis::ShardingStrategy sharding_strategy, const CommandControl& command_control, - const testsuite::RedisControl& testsuite_redis_control + const testsuite::RedisControl& testsuite_redis_control, + TopologyUpdateMethod topology_update_method ) { const auto& password = settings.password; const auto& sentinel_password = settings.sentinel_password; @@ -127,7 +130,8 @@ std::shared_ptr SubscribeSentinel::Create( std::move(keys_shard_factory), is_cluster_mode, command_control, - testsuite_redis_control + testsuite_redis_control, + topology_update_method ); subscribe_sentinel->Start(); return subscribe_sentinel; diff --git a/redis/src/storages/redis/impl/subscribe_sentinel.hpp b/redis/src/storages/redis/impl/subscribe_sentinel.hpp index 41c70a86d478..c3f6437768a6 100644 --- a/redis/src/storages/redis/impl/subscribe_sentinel.hpp +++ b/redis/src/storages/redis/impl/subscribe_sentinel.hpp @@ -8,6 +8,7 @@ #include #include +#include USERVER_NAMESPACE_BEGIN @@ -27,7 +28,8 @@ class SubscribeSentinel : protected Sentinel { KeyShardFactory key_shard_factory, bool is_cluster_mode, CommandControl command_control, - const testsuite::RedisControl& testsuite_redis_control + const testsuite::RedisControl& testsuite_redis_control, + TopologyUpdateMethod topology_update_method ); ~SubscribeSentinel() override; @@ -39,7 +41,8 @@ class SubscribeSentinel : protected Sentinel { const std::string& client_name, storages::redis::ShardingStrategy sharding_strategy, const CommandControl& command_control, - const testsuite::RedisControl& testsuite_redis_control + const testsuite::RedisControl& testsuite_redis_control, + TopologyUpdateMethod topology_update_method = TopologyUpdateMethod::kClusterSlots ); SubscriptionToken Subscribe( diff --git a/redis/src/storages/redis/topology_update_method.cpp b/redis/src/storages/redis/topology_update_method.cpp new file mode 100644 index 000000000000..77b522d450b2 --- /dev/null +++ b/redis/src/storages/redis/topology_update_method.cpp @@ -0,0 +1,39 @@ +#include + +#include + +#include + +#include + +USERVER_NAMESPACE_BEGIN + +namespace storages::redis { + +namespace { + +constexpr utils::TrivialBiMap kToTopologyUpdateMethod = [](auto selector) { + return selector() + .Case("cluster_shards", TopologyUpdateMethod::kClusterShards) + .Case("cluster_slots", TopologyUpdateMethod::kClusterSlots); +}; + +} // namespace + +TopologyUpdateMethod ToTopologyUpdateMethod(std::string_view view) { + if (auto ret = kToTopologyUpdateMethod.TryFind(view); ret.has_value()) { + return ret.value(); + } + + throw std::invalid_argument("Unknown topology update method: '" + std::string(view) + "'"); +} + +std::string_view ToStringView(TopologyUpdateMethod value) { + auto ret = kToTopologyUpdateMethod.TryFind(value); + UINVARIANT(ret.has_value(), fmt::format("Invalid topology update method {}", static_cast(value))); + return ret.value(); +} + +} // namespace storages::redis + +USERVER_NAMESPACE_END From e73a3ebc083d928cfa261c8d126553591982cfd3 Mon Sep 17 00:00:00 2001 From: antonyzhilin Date: Wed, 29 Apr 2026 11:54:47 +0300 Subject: [PATCH 7/8] refactor engine: advise TaskBuilder usage for complex task settings * Remove some obscure overloads of `engine::Async` and `utils::Async` variants * If you were using an `Async*` overload that has been removed, then please use @ref utils::TaskBuilder instead * Advise usage of @ref utils::TaskBuilder::HideSpan instead of @ref engine::AsyncNoSpan commit_hash:2578bdc78a802c95481cebba0b179dc7c4209309 --- core/include/userver/engine/async.hpp | 147 ++------------ .../engine/impl/task_context_factory.hpp | 13 +- core/include/userver/utils/async.hpp | 188 +++++------------- .../userver/utils/impl/wrapped_call.hpp | 20 +- core/include/userver/utils/task_builder.hpp | 14 +- core/src/clients/http/request_state.cpp | 1 + .../src/components/component_context_impl.cpp | 5 +- .../concurrent/async_event_channel_test.cpp | 1 + .../background_task_storage_test.cpp | 1 + core/src/concurrent/mp_queue_benchmark.cpp | 1 + core/src/concurrent/mpsc_queue_test.cpp | 9 +- core/src/concurrent/mutex_set_test.cpp | 4 +- core/src/concurrent/queue_test.cpp | 1 + core/src/engine/condition_variable_test.cpp | 3 +- core/src/engine/sleep_benchmark.cpp | 7 +- core/src/engine/task/async_test.cpp | 14 +- core/src/engine/task/cancel_test.cpp | 25 ++- core/src/engine/task/task_test.cpp | 3 +- core/src/os_signals/processor.cpp | 3 +- core/src/server/handlers/restart.cpp | 3 +- core/src/testsuite/cache_control.cpp | 27 +-- rocks/src/storages/rocks/client.cpp | 1 + .../tests/src/cmd/utask/gdb_test_utask.cpp | 1 + ydb/tests/parallel_data_query_test.cpp | 1 + 24 files changed, 161 insertions(+), 332 deletions(-) diff --git a/core/include/userver/engine/async.hpp b/core/include/userver/engine/async.hpp index f336ab3b1df7..cd6f7cbb1840 100644 --- a/core/include/userver/engine/async.hpp +++ b/core/include/userver/engine/async.hpp @@ -8,34 +8,11 @@ #include #include #include -#include USERVER_NAMESPACE_BEGIN namespace engine { -namespace impl { - -template