From ca6036115ee39316368dbeff86f95435b19d4eb2 Mon Sep 17 00:00:00 2001 From: Tikhon Date: Sun, 21 Apr 2024 00:26:18 +0300 Subject: [PATCH] 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'}], },