From 044ad01536931294650f3c45e6e37da73f688abf Mon Sep 17 00:00:00 2001 From: Inseok Lee Date: Fri, 13 Sep 2024 10:23:06 +0900 Subject: [PATCH] redis-proxy: support eval_ro, evalsha_ro command --- changelogs/current.yaml | 3 ++ .../arch_overview/other_protocols/redis.rst | 2 + .../network/common/redis/supported_commands.h | 2 +- .../clusters/redis/redis_cluster_lb_test.cc | 25 +++++++++++ .../redis_proxy/command_splitter_impl_test.cc | 44 +++++++++++++++++++ 5 files changed, 75 insertions(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index b773fb76f626e..2912c16438d3a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -460,5 +460,8 @@ new_features: WIP: Added implementation of :ref:`client_side_weighted_round_robin ` load balancing policy that uses ``OrcaLoadReport`` provided by the upstream host to calculate host load balancing weight. +- area: redis_proxy + change: | + Added support to ``eval_ro`` and ``evalsha_ro`` command. deprecated: diff --git a/docs/root/intro/arch_overview/other_protocols/redis.rst b/docs/root/intro/arch_overview/other_protocols/redis.rst index afe9a50cf66e5..72ad21a1fa015 100644 --- a/docs/root/intro/arch_overview/other_protocols/redis.rst +++ b/docs/root/intro/arch_overview/other_protocols/redis.rst @@ -205,7 +205,9 @@ For details on each command's usage see the official RPUSHX, List PUBLISH, Pubsub EVAL, Scripting + EVAL_RO, Scripting EVALSHA, Scripting + EVALSHA_RO, Scripting SADD, Set SCARD, Set SISMEMBER, Set diff --git a/source/extensions/filters/network/common/redis/supported_commands.h b/source/extensions/filters/network/common/redis/supported_commands.h index bbda434d72abe..c23dcacb77a01 100644 --- a/source/extensions/filters/network/common/redis/supported_commands.h +++ b/source/extensions/filters/network/common/redis/supported_commands.h @@ -41,7 +41,7 @@ struct SupportedCommands { * @return commands which hash on the fourth argument */ static const absl::flat_hash_set& evalCommands() { - CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "eval", "evalsha"); + CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "eval", "evalsha", "eval_ro", "evalsha_ro"); } /** diff --git a/test/extensions/clusters/redis/redis_cluster_lb_test.cc b/test/extensions/clusters/redis/redis_cluster_lb_test.cc index f837f774f0591..4b1bd809f3083 100644 --- a/test/extensions/clusters/redis/redis_cluster_lb_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_lb_test.cc @@ -561,6 +561,31 @@ TEST_F(RedisLoadBalancerContextImplTest, EnforceHashTag) { EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, context2.readPolicy()); } +TEST_F(RedisLoadBalancerContextImplTest, ReadOnlyCommand) { + std::vector eval_ro_foo(4); + eval_ro_foo[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[0].asString() = "eval_ro"; + eval_ro_foo[1].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[1].asString() = "return {KEYS[1]}"; + eval_ro_foo[2].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[2].asString() = "foo"; + eval_ro_foo[3].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[3].asString() = "0"; + + NetworkFilters::Common::Redis::RespValue eval_ro_request; + eval_ro_request.type(NetworkFilters::Common::Redis::RespType::Array); + eval_ro_request.asArray().swap(eval_ro_foo); + + RedisLoadBalancerContextImpl context1( + "foo", true, true, eval_ro_request, + NetworkFilters::Common::Redis::Client::ReadPolicy::PreferReplica); + + EXPECT_EQ(absl::optional(44950), context1.computeHashKey()); + EXPECT_EQ(true, context1.isReadCommand()); + EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::PreferReplica, + context1.readPolicy()); +} + } // namespace Redis } // namespace Clusters } // namespace Extensions diff --git a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc index 7a24ed7416ba5..4c0d88c6d643a 100644 --- a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc @@ -491,6 +491,50 @@ TEST_F(RedisSingleServerRequestTest, EvalShaSuccess) { store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); }; +TEST_F(RedisSingleServerRequestTest, EvalRoSuccess) { + InSequence s; + + Common::Redis::RespValuePtr request{new Common::Redis::RespValue()}; + makeBulkStringArray(*request, {"eval_ro", "return {ARGV[1]}", "1", "key", "arg"}); + makeRequest("key", std::move(request)); + EXPECT_NE(nullptr, handle_); + + std::string lower_command = absl::AsciiStrToLower("eval_ro"); + + time_system_.setMonotonicTime(std::chrono::milliseconds(10)); + EXPECT_CALL(store_, deliverHistogramToSinks( + Property(&Stats::Metric::name, + fmt::format("redis.foo.command.{}.latency", lower_command)), + 10)); + respond(); + + EXPECT_EQ(1UL, store_.counter(fmt::format("redis.foo.command.{}.total", lower_command)).value()); + EXPECT_EQ(1UL, + store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); +}; + +TEST_F(RedisSingleServerRequestTest, EvalShaRoSuccess) { + InSequence s; + + Common::Redis::RespValuePtr request{new Common::Redis::RespValue()}; + makeBulkStringArray(*request, {"EVALSHA_RO", "return {ARGV[1]}", "1", "keykey", "arg"}); + makeRequest("keykey", std::move(request)); + EXPECT_NE(nullptr, handle_); + + std::string lower_command = absl::AsciiStrToLower("evalsha_ro"); + + time_system_.setMonotonicTime(std::chrono::milliseconds(10)); + EXPECT_CALL(store_, deliverHistogramToSinks( + Property(&Stats::Metric::name, + fmt::format("redis.foo.command.{}.latency", lower_command)), + 10)); + respond(); + + EXPECT_EQ(1UL, store_.counter(fmt::format("redis.foo.command.{}.total", lower_command)).value()); + EXPECT_EQ(1UL, + store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); +}; + TEST_F(RedisSingleServerRequestTest, EvalWrongNumberOfArgs) { InSequence s;