redis-proxy: support eval_ro, evalsha_ro command

envoyproxy · Sep 27, 2024 · 044ad01 · 044ad01
1 parent cb18060
commit 044ad01
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 1 deletion.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -460,5 +460,8 @@ new_features:
     WIP: Added implementation of :ref:`client_side_weighted_round_robin
     <envoy_v3_api_msg_extensions.load_balancing_policies.client_side_weighted_round_robin.v3.ClientSideWeightedRoundRobin>`
     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
@@ -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
@@ -41,7 +41,7 @@ struct SupportedCommands {
    * @return commands which hash on the fourth argument
    */
   static const absl::flat_hash_set<std::string>& evalCommands() {
-    CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set<std::string>, "eval", "evalsha");
+    CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set<std::string>, "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
@@ -561,6 +561,31 @@ TEST_F(RedisLoadBalancerContextImplTest, EnforceHashTag) {
   EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, context2.readPolicy());
 }
 
+TEST_F(RedisLoadBalancerContextImplTest, ReadOnlyCommand) {
+  std::vector<NetworkFilters::Common::Redis::RespValue> 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<uint64_t>(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
@@ -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;