From afa8f8d69369fecd2bd2e013620b98a9627eda87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=B5=D1=80=D0=B3=D0=B8=D0=B5=D0=BD=D0=BA=D0=BE=20?= =?UTF-8?q?=D0=A2=D0=B8=D1=85=D0=BE=D0=BD=20=D0=94=D0=BC=D0=B8=D1=82=D1=80?= =?UTF-8?q?=D0=B8=D0=B5=D0=B2=D0=B8=D1=87?= Date: Thu, 25 Jan 2024 02:19:41 +0300 Subject: [PATCH 1/3] Added SELECT redis command --- .../userver/storages/redis/impl/base.hpp | 7 +- .../storages/redis/impl/secdist_redis.hpp | 1 + redis/src/storages/redis/impl/redis.cpp | 80 +++++++++++++++---- redis/src/storages/redis/impl/redis.hpp | 3 +- redis/src/storages/redis/impl/redis_stats.cpp | 1 + redis/src/storages/redis/impl/sentinel.cpp | 28 ++++--- redis/src/storages/redis/impl/sentinel.hpp | 3 +- .../src/storages/redis/impl/sentinel_impl.cpp | 8 +- .../src/storages/redis/impl/sentinel_impl.hpp | 4 +- redis/src/storages/redis/impl/server_test.cpp | 51 ++++++++++++ redis/src/storages/redis/impl/shard.cpp | 11 ++- redis/src/storages/redis/impl/shard.hpp | 3 + redis/src/storages/redis/redis_secdist.cpp | 9 +++ 13 files changed, 173 insertions(+), 36 deletions(-) diff --git a/redis/include/userver/storages/redis/impl/base.hpp b/redis/include/userver/storages/redis/impl/base.hpp index aa044c3a6c05..eaad3dfe18f9 100644 --- a/redis/include/userver/storages/redis/impl/base.hpp +++ b/redis/include/userver/storages/redis/impl/base.hpp @@ -27,16 +27,19 @@ struct ConnectionInfo { bool read_only = false; ConnectionSecurity connection_security = ConnectionSecurity::kNone; using HostVector = std::vector; + std::optional database_index{}; ConnectionInfo() = default; ConnectionInfo(std::string host, int port, Password password, bool read_only = false, - ConnectionSecurity security = ConnectionSecurity::kNone) + ConnectionSecurity security = ConnectionSecurity::kNone, + std::optional database_index = {}) : host{std::move(host)}, port{port}, password{std::move(password)}, read_only{read_only}, - connection_security(security) {} + connection_security(security), + database_index{database_index} {} }; struct Stat { diff --git a/redis/include/userver/storages/redis/impl/secdist_redis.hpp b/redis/include/userver/storages/redis/impl/secdist_redis.hpp index ebe2c71b0648..8dbfffd22a84 100644 --- a/redis/include/userver/storages/redis/impl/secdist_redis.hpp +++ b/redis/include/userver/storages/redis/impl/secdist_redis.hpp @@ -21,6 +21,7 @@ struct RedisSettings { std::vector shards; std::vector sentinels; + std::optional database_index; redis::Password password{std::string()}; redis::ConnectionSecurity secure_connection{redis::ConnectionSecurity::kNone}; }; diff --git a/redis/src/storages/redis/impl/redis.cpp b/redis/src/storages/redis/impl/redis.cpp index 3ea2d64930a3..aff505ca083d 100644 --- a/redis/src/storages/redis/impl/redis.cpp +++ b/redis/src/storages/redis/impl/redis.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include "command_control_impl.hpp" @@ -140,7 +141,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { ~RedisImpl(); void Connect(const ConnectionInfo::HostVector& host_addrs, int port, - const Password& password); + const Password& password, std::optional database_index); void Disconnect(); bool AsyncCommand(const CommandPtr& command); @@ -227,6 +228,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { void ProcessCommand(const CommandPtr& command); void Authenticate(); + void SelectDatabase(); void SendReadOnly(); void FreeCommands(); @@ -245,7 +247,8 @@ class Redis::RedisImpl : public std::enable_shared_from_this { static bool WatchCommandTimerEnabled( const CommandsBufferingSettings& commands_buffering_settings); - bool Connect(const std::string& host, int port, const Password& password); + bool Connect(const std::string& host, int port, const Password& password, + std::optional database_index); Redis* redis_obj_; engine::ev::ThreadControl ev_thread_control_; @@ -266,6 +269,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { uint16_t port_ = 0; std::string server_; Password password_{std::string()}; + std::optional database_index_; std::atomic commands_size_ = 0; size_t sent_count_ = 0; size_t cmd_counter_ = 0; @@ -341,8 +345,9 @@ Redis::~Redis() { } void Redis::Connect(const ConnectionInfo::HostVector& host_addrs, int port, - const Password& password) { - impl_->Connect(host_addrs, port, password); + const Password& password, + std::optional database_index) { + impl_->Connect(host_addrs, port, password, database_index); } bool Redis::AsyncCommand(const CommandPtr& command) { @@ -450,9 +455,10 @@ void Redis::RedisImpl::Detach() { } void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs, - int port, const Password& password) { + int port, const Password& password, + std::optional database_index) { for (const auto& host : host_addrs) - if (Connect(host, port, password)) return; + if (Connect(host, port, password, database_index)) return; LOG_ERROR() << "error async connect to Redis server (host addrs =" << host_addrs << ", port=" << port << ")"; @@ -460,7 +466,8 @@ void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs, } bool Redis::RedisImpl::Connect(const std::string& host, int port, - const Password& password) { + const Password& password, + std::optional database_index) { UASSERT(context_ == nullptr); UASSERT(state_ == State::kInit); @@ -471,6 +478,7 @@ bool Redis::RedisImpl::Connect(const std::string& host, int port, log_extra_.Extend("redis_server", GetServer()); log_extra_.Extend("server_id", GetServerId().GetId()); password_ = password; + database_index_ = database_index; LOG_INFO() << log_extra_ << "Async connect to Redis server=" << GetServer(); context_ = redisAsyncConnect(host.c_str(), port); @@ -1046,19 +1054,13 @@ bool Redis::RedisImpl::InitSecureConnection() { void Redis::RedisImpl::Authenticate() { if (password_.GetUnderlying().empty()) { - if (send_readonly_) - SendReadOnly(); - else - SetState(State::kConnected); + SendReadOnly(); } else { ProcessCommand(PrepareCommand( CmdArgs{"AUTH", password_.GetUnderlying()}, [this](const CommandPtr&, ReplyPtr reply) { if (*reply && reply->data.IsStatus()) { - if (send_readonly_) - SendReadOnly(); - else - SetState(State::kConnected); + SendReadOnly(); } else { if (*reply) { if (reply->IsUnknownCommandError()) { @@ -1085,12 +1087,17 @@ void Redis::RedisImpl::Authenticate() { } void Redis::RedisImpl::SendReadOnly() { + if (!send_readonly_) { + SelectDatabase(); + return; + } + LOG_DEBUG() << "Send READONLY command to slave " << GetServerId().GetDescription() << " in cluster mode"; ProcessCommand(PrepareCommand(CmdArgs{"READONLY"}, [this](const CommandPtr&, ReplyPtr reply) { if (*reply && reply->data.IsStatus()) { - SetState(State::kConnected); + SelectDatabase(); } else { if (*reply) { LOG_LIMITED_ERROR() @@ -1107,6 +1114,47 @@ void Redis::RedisImpl::SendReadOnly() { })); } +void Redis::RedisImpl::SelectDatabase() { + if (!database_index_) { + SetState(RedisState::kConnected); + return; + } + + ProcessCommand(PrepareCommand( + CmdArgs{"SELECT", *database_index_}, + [this](const CommandPtr&, ReplyPtr reply) { + if (*reply && reply->data.IsStatus()) { + SetState(RedisState::kConnected); + LOG_INFO() << log_extra_ + << "Selected redis logical database with index " + << *database_index_; + return; + } + + const utils::ScopeGuard auto_disconnect([this]() { Disconnect(); }); + + if (!*reply) { + LOG_LIMITED_ERROR() + << "SELECT failed with status " << reply->status << " (" + << reply->status_string << ") " << log_extra_; + return; + } + + if (reply->IsUnknownCommandError()) { + LOG_WARNING() << log_extra_ + << "SELECT failed: unknown command `SELECT` - " + "possible when connecting to Sentinel instead " + "of Redis master or slave instance"; + return; + } + + LOG_LIMITED_ERROR() + << log_extra_ + << "SELECT failed: response type=" << reply->data.GetTypeString() + << " msg=" << reply->data.ToDebugString(); + })); +} + void Redis::RedisImpl::OnRedisReply(redisAsyncContext* c, void* r, void* privdata) noexcept { auto* impl = static_cast(c->data); diff --git a/redis/src/storages/redis/impl/redis.hpp b/redis/src/storages/redis/impl/redis.hpp index c3aeaaa4ba66..7e730565a46d 100644 --- a/redis/src/storages/redis/impl/redis.hpp +++ b/redis/src/storages/redis/impl/redis.hpp @@ -36,7 +36,8 @@ class Redis { Redis(Redis&& o) = delete; void Connect(const ConnectionInfo::HostVector& host_addrs, int port, - const Password& password); + const Password& password, + std::optional database_index = {}); bool AsyncCommand(const CommandPtr& command); size_t GetRunningCommands() const; diff --git a/redis/src/storages/redis/impl/redis_stats.cpp b/redis/src/storages/redis/impl/redis_stats.cpp index 86e830d1d4cb..4b3aa6a7bc27 100644 --- a/redis/src/storages/redis/impl/redis_stats.cpp +++ b/redis/src/storages/redis/impl/redis_stats.cpp @@ -76,6 +76,7 @@ const std::string_view kCommandTypes[] = { "scan", "scard", "script", + "select", "sentinel", "set", "setex", diff --git a/redis/src/storages/redis/impl/sentinel.cpp b/redis/src/storages/redis/impl/sentinel.cpp index d0a70d990b08..9648834a01fb 100644 --- a/redis/src/storages/redis/impl/sentinel.cpp +++ b/redis/src/storages/redis/impl/sentinel.cpp @@ -40,15 +40,18 @@ void ThrowIfCancelled() { } // namespace -Sentinel::Sentinel( - const std::shared_ptr& thread_pools, - const std::vector& shards, - const std::vector& conns, std::string shard_group_name, - const std::string& client_name, const Password& password, - ConnectionSecurity connection_security, ReadyChangeCallback ready_callback, - dynamic_config::Source dynamic_config_source, - std::unique_ptr&& key_shard, CommandControl command_control, - const testsuite::RedisControl& testsuite_redis_control, ConnectionMode mode) +Sentinel::Sentinel(const std::shared_ptr& thread_pools, + const std::vector& shards, + const std::vector& conns, + std::string shard_group_name, const std::string& client_name, + const Password& password, + ConnectionSecurity connection_security, + ReadyChangeCallback ready_callback, + dynamic_config::Source dynamic_config_source, + std::unique_ptr&& key_shard, + CommandControl command_control, + const testsuite::RedisControl& testsuite_redis_control, + ConnectionMode mode, std::optional database_index) : thread_pools_(thread_pools), secdist_default_command_control_(command_control), testsuite_redis_control_(testsuite_redis_control) { @@ -82,7 +85,7 @@ Sentinel::Sentinel( *sentinel_thread_control_, thread_pools_->GetRedisThreadPool(), *this, shards, conns, std::move(shard_group_name), client_name, password, connection_security, std::move(ready_callback), std::move(key_shard), - dynamic_config_source, mode); + dynamic_config_source, mode, database_index); } }); } @@ -152,7 +155,7 @@ std::shared_ptr Sentinel::CreateSentinel( // sentinels in cluster mode. conns.emplace_back(sentinel.host, sentinel.port, (key_shard ? Password("") : password), false, - settings.secure_connection); + settings.secure_connection, std::nullopt); } LOG_DEBUG() << "redis command_control:" << command_control.ToString(); @@ -162,7 +165,8 @@ std::shared_ptr Sentinel::CreateSentinel( thread_pools, shards, conns, std::move(shard_group_name), client_name, password, settings.secure_connection, std::move(ready_callback), dynamic_config_source, std::move(key_shard), command_control, - testsuite_redis_control); + testsuite_redis_control, ConnectionMode::kCommands, + settings.database_index); client->Start(); } diff --git a/redis/src/storages/redis/impl/sentinel.hpp b/redis/src/storages/redis/impl/sentinel.hpp index dd61440981a6..2ab96c4f259c 100644 --- a/redis/src/storages/redis/impl/sentinel.hpp +++ b/redis/src/storages/redis/impl/sentinel.hpp @@ -59,7 +59,8 @@ class Sentinel { std::unique_ptr&& key_shard = nullptr, CommandControl command_control = {}, const testsuite::RedisControl& testsuite_redis_control = {}, - ConnectionMode mode = ConnectionMode::kCommands); + ConnectionMode mode = ConnectionMode::kCommands, + std::optional database_index = {}); virtual ~Sentinel(); void Start(); diff --git a/redis/src/storages/redis/impl/sentinel_impl.cpp b/redis/src/storages/redis/impl/sentinel_impl.cpp index bde70d6c92b4..29c69274797d 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.cpp +++ b/redis/src/storages/redis/impl/sentinel_impl.cpp @@ -64,7 +64,8 @@ SentinelImpl::SentinelImpl( const std::string& client_name, const Password& password, ConnectionSecurity connection_security, ReadyChangeCallback ready_callback, std::unique_ptr&& key_shard, - dynamic_config::Source dynamic_config_source, ConnectionMode mode) + dynamic_config::Source dynamic_config_source, ConnectionMode mode, + std::optional database_index) : sentinel_obj_(sentinel), ev_thread_(sentinel_thread_control), shard_group_name_(std::move(shard_group_name)), @@ -79,7 +80,8 @@ SentinelImpl::SentinelImpl( key_shard_(std::move(key_shard)), connection_mode_(mode), slot_info_(IsInClusterMode() ? std::make_unique() : nullptr), - dynamic_config_source_(dynamic_config_source) { + dynamic_config_source_(dynamic_config_source), + database_index_(database_index) { for (size_t i = 0; i < init_shards_->size(); ++i) { shards_[(*init_shards_)[i]] = i; connected_statuses_.push_back(std::make_unique()); @@ -637,6 +639,7 @@ void SentinelImpl::ReadSentinels() { for (auto shard_conn : info) { if (shards_.find(shard_conn.Name()) != shards_.end()) { shard_conn.SetConnectionSecurity(connection_security_); + shard_conn.SetDatabaseIndex(database_index_); shard_found[shards_[shard_conn.Name()]] = true; watcher->host_port_to_shard[shard_conn.HostPort()] = shards_[shard_conn.Name()]; @@ -675,6 +678,7 @@ void SentinelImpl::ReadSentinels() { shard_conn.SetName(shard); shard_conn.SetReadOnly(true); shard_conn.SetConnectionSecurity(connection_security_); + shard_conn.SetDatabaseIndex(database_index_); if (shards_.find(shard_conn.Name()) != shards_.end()) watcher->host_port_to_shard[shard_conn.HostPort()] = shards_[shard_conn.Name()]; diff --git a/redis/src/storages/redis/impl/sentinel_impl.hpp b/redis/src/storages/redis/impl/sentinel_impl.hpp index cb4d4316da6e..239a8bfbc345 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.hpp +++ b/redis/src/storages/redis/impl/sentinel_impl.hpp @@ -113,7 +113,8 @@ class SentinelImpl : public SentinelImplBase { ReadyChangeCallback ready_callback, std::unique_ptr&& key_shard, dynamic_config::Source dynamic_config_source, - ConnectionMode mode = ConnectionMode::kCommands); + ConnectionMode mode = ConnectionMode::kCommands, + std::optional database_index = {}); ~SentinelImpl() override; std::unordered_map @@ -286,6 +287,7 @@ class SentinelImpl : public SentinelImplBase { std::optional commands_buffering_settings_; dynamic_config::Source dynamic_config_source_; std::atomic publish_shard_{0}; + std::optional database_index_; }; } // namespace redis diff --git a/redis/src/storages/redis/impl/server_test.cpp b/redis/src/storages/redis/impl/server_test.cpp index 060bb3f4f538..3f9a29602d10 100644 --- a/redis/src/storages/redis/impl/server_test.cpp +++ b/redis/src/storages/redis/impl/server_test.cpp @@ -18,6 +18,7 @@ constexpr std::chrono::milliseconds kSmallPeriod{500}; constexpr std::chrono::milliseconds kWaitPeriod{10}; constexpr auto kWaitRetries = 100; constexpr auto kCheckCount = 10; +constexpr auto kRedisDatabaseIndex = 46; const std::string kLocalhost = "127.0.0.1"; @@ -106,6 +107,56 @@ TEST(Redis, AuthTimeout) { PeriodicCheck([&] { return !IsConnected(*redis); }); } +TEST(Redis, Select) { + MockRedisServer server; + auto ping_handler = server.RegisterPingHandler(); + auto select_handler = server.RegisterStatusReplyHandler("SELECT", "OK"); + + auto pool = std::make_shared(1, 1); + redis::RedisCreationSettings redis_settings; + auto redis = std::make_shared(pool->GetRedisThreadPool(), + redis_settings); + redis->Connect({kLocalhost}, server.GetPort(), {}, kRedisDatabaseIndex); + + EXPECT_TRUE(select_handler->WaitForFirstReply(kSmallPeriod)); + EXPECT_TRUE(ping_handler->WaitForFirstReply(kSmallPeriod)); +} + +TEST(Redis, SelectFail) { + MockRedisServer server; + auto ping_handler = server.RegisterPingHandler(); + auto select_error_handler = + server.RegisterErrorReplyHandler("SELECT", "NO PASARAN"); + + auto pool = std::make_shared(1, 1); + redis::RedisCreationSettings redis_settings; + auto redis = std::make_shared(pool->GetRedisThreadPool(), + redis_settings); + redis->Connect({kLocalhost}, server.GetPort(), {}, kRedisDatabaseIndex); + + EXPECT_TRUE(select_error_handler->WaitForFirstReply(kSmallPeriod)); + PeriodicCheck([&] { return !IsConnected(*redis); }); +} + +TEST(Redis, SelectTimeout) { + MockRedisServer server; + redis::CommandControl cc{}; + auto ping_handler = server.RegisterPingHandler(); + auto sleep_period = cc.timeout_single + std::chrono::milliseconds(30); + auto select_error_handler = + server.RegisterTimeoutHandler("SELECT", sleep_period); + + auto pool = std::make_shared(1, 1); + redis::RedisCreationSettings redis_settings; + auto redis = std::make_shared(pool->GetRedisThreadPool(), + redis_settings); + redis->Connect({kLocalhost}, server.GetPort(), {}, kRedisDatabaseIndex); + + EXPECT_TRUE( + select_error_handler->WaitForFirstReply(sleep_period + kSmallPeriod)); + PeriodicCheck([&] { return !IsConnected(*redis); }); +} + TEST(Redis, SlaveREADONLY) { MockRedisServer server; auto ping_handler = server.RegisterPingHandler(); diff --git a/redis/src/storages/redis/impl/shard.cpp b/redis/src/storages/redis/impl/shard.cpp index 0b73b692166d..c8fd846ec4ae 100644 --- a/redis/src/storages/redis/impl/shard.cpp +++ b/redis/src/storages/redis/impl/shard.cpp @@ -34,6 +34,14 @@ void ConnectionInfoInt::SetPassword(Password password) { conn_info_.password = std::move(password); } +void ConnectionInfoInt::SetDatabaseIndex(std::optional index) { + conn_info_.database_index = index; +} + +std::optional ConnectionInfoInt::DatabaseIndex() const { + return conn_info_.database_index; +} + bool ConnectionInfoInt::IsReadOnly() const { return conn_info_.read_only; } void ConnectionInfoInt::SetReadOnly(bool value) { @@ -51,7 +59,8 @@ ConnectionSecurity ConnectionInfoInt::GetConnectionSecurity() const { const std::string& ConnectionInfoInt::Fulltext() const { return fulltext_; } void ConnectionInfoInt::Connect(Redis& instance) const { - instance.Connect({conn_info_.host}, conn_info_.port, conn_info_.password); + instance.Connect({conn_info_.host}, conn_info_.port, conn_info_.password, + conn_info_.database_index); } bool operator==(const ConnectionInfoInt& lhs, const ConnectionInfoInt& rhs) { diff --git a/redis/src/storages/redis/impl/shard.hpp b/redis/src/storages/redis/impl/shard.hpp index e74b516e30ee..cefcbed0eaca 100644 --- a/redis/src/storages/redis/impl/shard.hpp +++ b/redis/src/storages/redis/impl/shard.hpp @@ -28,6 +28,9 @@ class ConnectionInfoInt { void SetPassword(Password); + void SetDatabaseIndex(std::optional index); + std::optional DatabaseIndex() const; + bool IsReadOnly() const; void SetReadOnly(bool); diff --git a/redis/src/storages/redis/redis_secdist.cpp b/redis/src/storages/redis/redis_secdist.cpp index 4cd588e857d8..3eb0b0bd7f36 100644 --- a/redis/src/storages/redis/redis_secdist.cpp +++ b/redis/src/storages/redis/redis_secdist.cpp @@ -1,5 +1,6 @@ #include "redis_secdist.hpp" +#include #include #include #include @@ -38,6 +39,14 @@ RedisMapSettings::RedisMapSettings(const formats::json::Value& doc) { ? USERVER_NAMESPACE::redis::ConnectionSecurity::kTLS : USERVER_NAMESPACE::redis::ConnectionSecurity::kNone; + settings.database_index = + client_settings["database_index"].As>(); + if (settings.database_index && *settings.database_index == 0) { + // To get rid of the redundant `SELECT 0` command + // since 0 is the default database index and it will be set automatically + settings.database_index = std::nullopt; + } + const auto& shards = client_settings["shards"]; CheckIsArray(shards, "shards"); for (const auto& shard : shards) { From 97a741a670824e759afc88095b6cf748ee18f263 Mon Sep 17 00:00:00 2001 From: Tikhon Date: Sun, 4 Feb 2024 21:23:43 +0300 Subject: [PATCH 2/3] fix compile error --- redis/src/storages/redis/impl/server_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/src/storages/redis/impl/server_test.cpp b/redis/src/storages/redis/impl/server_test.cpp index 3f9a29602d10..ea93a398cb55 100644 --- a/redis/src/storages/redis/impl/server_test.cpp +++ b/redis/src/storages/redis/impl/server_test.cpp @@ -142,7 +142,7 @@ TEST(Redis, SelectTimeout) { MockRedisServer server; redis::CommandControl cc{}; auto ping_handler = server.RegisterPingHandler(); - auto sleep_period = cc.timeout_single + std::chrono::milliseconds(30); + auto sleep_period = redis::kDefaultTimeoutSingle + std::chrono::milliseconds(30); auto select_error_handler = server.RegisterTimeoutHandler("SELECT", sleep_period); From ca6036115ee39316368dbeff86f95435b19d4eb2 Mon Sep 17 00:00:00 2001 From: Tikhon Date: Sun, 21 Apr 2024 00:26:18 +0300 Subject: [PATCH 3/3] review fixes --- .../userver/storages/redis/impl/base.hpp | 4 ++-- .../storages/redis/impl/secdist_redis.hpp | 2 +- redis/src/storages/redis/impl/redis.cpp | 20 ++++++++++--------- redis/src/storages/redis/impl/redis.hpp | 2 +- redis/src/storages/redis/impl/sentinel.cpp | 4 ++-- redis/src/storages/redis/impl/sentinel.hpp | 2 +- .../src/storages/redis/impl/sentinel_impl.cpp | 2 +- .../src/storages/redis/impl/sentinel_impl.hpp | 4 ++-- redis/src/storages/redis/impl/shard.cpp | 4 ++-- redis/src/storages/redis/impl/shard.hpp | 4 ++-- redis/src/storages/redis/redis_secdist.cpp | 9 +-------- samples/redis_service/tests/conftest.py | 1 + 12 files changed, 27 insertions(+), 31 deletions(-) diff --git a/redis/include/userver/storages/redis/impl/base.hpp b/redis/include/userver/storages/redis/impl/base.hpp index eaad3dfe18f9..b407d4e6a383 100644 --- a/redis/include/userver/storages/redis/impl/base.hpp +++ b/redis/include/userver/storages/redis/impl/base.hpp @@ -27,13 +27,13 @@ struct ConnectionInfo { bool read_only = false; ConnectionSecurity connection_security = ConnectionSecurity::kNone; using HostVector = std::vector; - std::optional database_index{}; + size_t database_index = 0; ConnectionInfo() = default; ConnectionInfo(std::string host, int port, Password password, bool read_only = false, ConnectionSecurity security = ConnectionSecurity::kNone, - std::optional database_index = {}) + size_t database_index = {}) : host{std::move(host)}, port{port}, password{std::move(password)}, diff --git a/redis/include/userver/storages/redis/impl/secdist_redis.hpp b/redis/include/userver/storages/redis/impl/secdist_redis.hpp index 8dbfffd22a84..d798a3e9fdd3 100644 --- a/redis/include/userver/storages/redis/impl/secdist_redis.hpp +++ b/redis/include/userver/storages/redis/impl/secdist_redis.hpp @@ -21,7 +21,7 @@ struct RedisSettings { std::vector shards; std::vector sentinels; - std::optional database_index; + size_t database_index{0}; redis::Password password{std::string()}; redis::ConnectionSecurity secure_connection{redis::ConnectionSecurity::kNone}; }; diff --git a/redis/src/storages/redis/impl/redis.cpp b/redis/src/storages/redis/impl/redis.cpp index 370ee068bb0f..972fa7cf9dcc 100644 --- a/redis/src/storages/redis/impl/redis.cpp +++ b/redis/src/storages/redis/impl/redis.cpp @@ -147,7 +147,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { ~RedisImpl(); void Connect(const ConnectionInfo::HostVector& host_addrs, int port, - const Password& password, std::optional database_index); + const Password& password, size_t database_index); void Disconnect(); bool AsyncCommand(const CommandPtr& command); @@ -254,7 +254,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { const CommandsBufferingSettings& commands_buffering_settings); bool Connect(const std::string& host, int port, const Password& password, - std::optional database_index); + size_t database_index); Redis* redis_obj_; engine::ev::ThreadControl ev_thread_control_; @@ -275,7 +275,7 @@ class Redis::RedisImpl : public std::enable_shared_from_this { uint16_t port_ = 0; std::string server_; Password password_{std::string()}; - std::optional database_index_; + std::size_t database_index_ = 0; std::atomic commands_size_ = 0; size_t sent_count_ = 0; size_t cmd_counter_ = 0; @@ -341,7 +341,7 @@ Redis::~Redis() { void Redis::Connect(const ConnectionInfo::HostVector& host_addrs, int port, const Password& password, - std::optional database_index) { + size_t database_index) { impl_->Connect(host_addrs, port, password, database_index); } @@ -451,7 +451,7 @@ void Redis::RedisImpl::Detach() { void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs, int port, const Password& password, - std::optional database_index) { + size_t database_index) { for (const auto& host : host_addrs) if (Connect(host, port, password, database_index)) return; @@ -462,7 +462,7 @@ void Redis::RedisImpl::Connect(const ConnectionInfo::HostVector& host_addrs, bool Redis::RedisImpl::Connect(const std::string& host, int port, const Password& password, - std::optional database_index) { + size_t database_index) { UASSERT(context_ == nullptr); UASSERT(state_ == State::kInit); @@ -1109,19 +1109,21 @@ void Redis::RedisImpl::SendReadOnly() { } void Redis::RedisImpl::SelectDatabase() { - if (!database_index_) { + // To get rid of the redundant `SELECT 0` command + // since 0 is the default database index, and it will be set automatically + if (database_index_ == 0) { SetState(RedisState::kConnected); return; } ProcessCommand(PrepareCommand( - CmdArgs{"SELECT", *database_index_}, + CmdArgs{"SELECT", database_index_}, [this](const CommandPtr&, ReplyPtr reply) { if (*reply && reply->data.IsStatus()) { SetState(RedisState::kConnected); LOG_INFO() << log_extra_ << "Selected redis logical database with index " - << *database_index_; + << database_index_; return; } diff --git a/redis/src/storages/redis/impl/redis.hpp b/redis/src/storages/redis/impl/redis.hpp index d72c467a3dcd..2220b0200c70 100644 --- a/redis/src/storages/redis/impl/redis.hpp +++ b/redis/src/storages/redis/impl/redis.hpp @@ -34,7 +34,7 @@ class Redis { void Connect(const ConnectionInfo::HostVector& host_addrs, int port, const Password& password, - std::optional database_index = {}); + size_t database_index = 0); bool AsyncCommand(const CommandPtr& command); size_t GetRunningCommands() const; diff --git a/redis/src/storages/redis/impl/sentinel.cpp b/redis/src/storages/redis/impl/sentinel.cpp index b0b36344b539..63683be36955 100644 --- a/redis/src/storages/redis/impl/sentinel.cpp +++ b/redis/src/storages/redis/impl/sentinel.cpp @@ -74,7 +74,7 @@ Sentinel::Sentinel(const std::shared_ptr& thread_pools, std::unique_ptr&& key_shard, CommandControl command_control, const testsuite::RedisControl& testsuite_redis_control, - ConnectionMode mode, std::optional database_index) + ConnectionMode mode, size_t database_index) : thread_pools_(thread_pools), secdist_default_command_control_(command_control), testsuite_redis_control_(testsuite_redis_control) { @@ -169,7 +169,7 @@ std::shared_ptr Sentinel::CreateSentinel( // sentinels in cluster mode. conns.emplace_back(sentinel.host, sentinel.port, (key_shard ? Password("") : password), false, - settings.secure_connection, std::nullopt); + settings.secure_connection); } LOG_DEBUG() << "redis command_control:" << command_control.ToString(); diff --git a/redis/src/storages/redis/impl/sentinel.hpp b/redis/src/storages/redis/impl/sentinel.hpp index 8b54a5a77077..8c186c4c55bb 100644 --- a/redis/src/storages/redis/impl/sentinel.hpp +++ b/redis/src/storages/redis/impl/sentinel.hpp @@ -72,7 +72,7 @@ class Sentinel { CommandControl command_control = {}, const testsuite::RedisControl& testsuite_redis_control = {}, ConnectionMode mode = ConnectionMode::kCommands, - std::optional database_index = {}); + size_t database_index = 0); virtual ~Sentinel(); void Start(); diff --git a/redis/src/storages/redis/impl/sentinel_impl.cpp b/redis/src/storages/redis/impl/sentinel_impl.cpp index 7f99e06a4823..7f78d423b5c8 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.cpp +++ b/redis/src/storages/redis/impl/sentinel_impl.cpp @@ -65,7 +65,7 @@ SentinelImpl::SentinelImpl( ConnectionSecurity connection_security, ReadyChangeCallback ready_callback, std::unique_ptr&& key_shard, dynamic_config::Source dynamic_config_source, ConnectionMode mode, - std::optional database_index) + size_t database_index) : sentinel_obj_(sentinel), ev_thread_(sentinel_thread_control), shard_group_name_(std::move(shard_group_name)), diff --git a/redis/src/storages/redis/impl/sentinel_impl.hpp b/redis/src/storages/redis/impl/sentinel_impl.hpp index 029bc4a4a960..d285dc7265c6 100644 --- a/redis/src/storages/redis/impl/sentinel_impl.hpp +++ b/redis/src/storages/redis/impl/sentinel_impl.hpp @@ -113,7 +113,7 @@ class SentinelImpl : public SentinelImplBase { std::unique_ptr&& key_shard, dynamic_config::Source dynamic_config_source, ConnectionMode mode = ConnectionMode::kCommands, - std::optional database_index = {}); + size_t database_index = 0); ~SentinelImpl() override; std::unordered_map @@ -286,7 +286,7 @@ class SentinelImpl : public SentinelImplBase { std::optional commands_buffering_settings_; dynamic_config::Source dynamic_config_source_; std::atomic publish_shard_{0}; - std::optional database_index_; + std::size_t database_index_{0}; }; } // namespace redis diff --git a/redis/src/storages/redis/impl/shard.cpp b/redis/src/storages/redis/impl/shard.cpp index ce5d04c55ea7..d5a2063e87c0 100644 --- a/redis/src/storages/redis/impl/shard.cpp +++ b/redis/src/storages/redis/impl/shard.cpp @@ -34,11 +34,11 @@ void ConnectionInfoInt::SetPassword(Password password) { conn_info_.password = std::move(password); } -void ConnectionInfoInt::SetDatabaseIndex(std::optional index) { +void ConnectionInfoInt::SetDatabaseIndex(size_t index) { conn_info_.database_index = index; } -std::optional ConnectionInfoInt::DatabaseIndex() const { +size_t ConnectionInfoInt::DatabaseIndex() const { return conn_info_.database_index; } diff --git a/redis/src/storages/redis/impl/shard.hpp b/redis/src/storages/redis/impl/shard.hpp index 00567e9d3aeb..10b98204db66 100644 --- a/redis/src/storages/redis/impl/shard.hpp +++ b/redis/src/storages/redis/impl/shard.hpp @@ -28,8 +28,8 @@ class ConnectionInfoInt { void SetPassword(Password); - void SetDatabaseIndex(std::optional index); - std::optional DatabaseIndex() const; + void SetDatabaseIndex(size_t index); + size_t DatabaseIndex() const; bool IsReadOnly() const; void SetReadOnly(bool); diff --git a/redis/src/storages/redis/redis_secdist.cpp b/redis/src/storages/redis/redis_secdist.cpp index 99e6258e549f..088e7c7050db 100644 --- a/redis/src/storages/redis/redis_secdist.cpp +++ b/redis/src/storages/redis/redis_secdist.cpp @@ -1,6 +1,5 @@ #include "redis_secdist.hpp" -#include #include #include #include @@ -44,13 +43,7 @@ RedisMapSettings::RedisMapSettings(const formats::json::Value& doc) { ? USERVER_NAMESPACE::redis::ConnectionSecurity::kTLS : USERVER_NAMESPACE::redis::ConnectionSecurity::kNone; - settings.database_index = - client_settings["database_index"].As>(); - if (settings.database_index && *settings.database_index == 0) { - // To get rid of the redundant `SELECT 0` command - // since 0 is the default database index and it will be set automatically - settings.database_index = std::nullopt; - } + settings.database_index = client_settings["database_index"].As(0); const auto& shards = client_settings["shards"]; CheckIsArray(shards, "shards"); diff --git a/samples/redis_service/tests/conftest.py b/samples/redis_service/tests/conftest.py index e21bbc27b66b..991b8ef67061 100644 --- a/samples/redis_service/tests/conftest.py +++ b/samples/redis_service/tests/conftest.py @@ -14,6 +14,7 @@ def service_env(redis_sentinels): 'redis_settings': { 'taxi-tmp': { 'password': '', + 'database_index': 0, 'sentinels': redis_sentinels, 'shards': [{'name': 'test_master0'}], },