Implement additional fanout for SRRoute

Summary: as title Reviewed By: disylh Differential Revision: D61860873 fbshipit-source-id: f78632c33c1919aeef4fea66b97ce16fdff0fabe
facebook · Sep 3, 2024 · d80da5d · d80da5d
1 parent 53dd319
commit d80da5d
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 22 deletions.
diff --git a/mcrouter/routes/McRouteHandleProvider-inl.h b/mcrouter/routes/McRouteHandleProvider-inl.h
@@ -38,6 +38,7 @@
 #include "mcrouter/routes/PoolRouteUtils.h"
 #include "mcrouter/routes/RateLimitRoute.h"
 #include "mcrouter/routes/RateLimiter.h"
+#include "mcrouter/routes/RoutingUtils.h"
 #include "mcrouter/routes/ShadowRoute.h"
 #include "mcrouter/routes/ShardHashFunc.h"
 #include "mcrouter/routes/ShardSplitRoute.h"
@@ -47,8 +48,6 @@ namespace facebook {
 namespace memcache {
 namespace mcrouter {
 
-static constexpr uint32_t kMaxTotalFanout = 32 * 1024;
-
 extern template MemcacheRouterInfo::RouteHandlePtr
 createHashRoute<MemcacheRouterInfo>(
     const folly::dynamic& json,
@@ -284,7 +283,6 @@ McRouteHandleProvider<RouterInfo>::makePool(
     // servers
     auto jhostnames = json.get_ptr("hostnames");
     auto jfailureDomains = json.get_ptr("failure_domains");
-    auto jAdditionalFanout = json.get_ptr("additional_fanout");
     checkLogic(
         !jfailureDomains || jfailureDomains->isArray(),
         "failure_domains is not an array");
@@ -304,25 +302,8 @@ McRouteHandleProvider<RouterInfo>::makePool(
         jservers->size(),
         jfailureDomains ? jfailureDomains->size() : 0);
 
-    checkLogic(
-        !jAdditionalFanout || jAdditionalFanout->isInt(),
-        "additional_fanout is not an integer");
-    uint32_t additionalFanout = 0;
-    if (jAdditionalFanout) {
-      additionalFanout = jAdditionalFanout->getInt();
-    }
-    checkLogic(
-        static_cast<uint64_t>(additionalFanout + 1) *
-                static_cast<uint64_t>(proxy_.router().opts().num_proxies) <=
-            kMaxTotalFanout,
-        "(additional_fanout={} + 1) * num_proxies={} must be <= {}",
-        additionalFanout,
-        proxy_.router().opts().num_proxies,
-        kMaxTotalFanout);
-
-    checkLogic(
-        additionalFanout == 0 || !proxy_.router().opts().thread_affinity,
-        "additional_fanout is not supported with thread_affinity");
+    uint32_t additionalFanout =
+        getAdditionalFanout(json, proxy_.router().opts());
 
     int32_t poolStatIndex = proxy_.router().getStatsEnabledPoolIndex(name);
 

diff --git a/mcrouter/routes/RoutingUtils.cpp b/mcrouter/routes/RoutingUtils.cpp
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "mcrouter/routes/RoutingUtils.h"
+
+#include <folly/json/dynamic.h>
+#include "mcrouter/lib/fbi/cpp/util.h"
+#include "mcrouter/options.h"
+
+namespace facebook {
+namespace memcache {
+namespace mcrouter {
+
+static constexpr uint32_t kMaxTotalFanout = 32 * 1024;
+
+uint32_t getAdditionalFanout(
+    const folly::dynamic& json,
+    const McrouterOptions& opts) {
+  auto jAdditionalFanout = json.get_ptr("additional_fanout");
+  if (!jAdditionalFanout) {
+    return 0;
+  }
+
+  checkLogic(jAdditionalFanout->isInt(), "additional_fanout is not an integer");
+  uint32_t additionalFanout = jAdditionalFanout->getInt();
+
+  checkLogic(
+      static_cast<uint64_t>(additionalFanout + 1) *
+              static_cast<uint64_t>(opts.num_proxies) <=
+          kMaxTotalFanout,
+      "(additional_fanout={} + 1) * num_proxies={} must be <= {}",
+      additionalFanout,
+      opts.num_proxies,
+      kMaxTotalFanout);
+
+  checkLogic(
+      additionalFanout == 0 || !opts.thread_affinity,
+      "additional_fanout is not supported with thread_affinity");
+  return additionalFanout;
+}
+
+} // namespace mcrouter
+} // namespace memcache
+} // namespace facebook
diff --git a/mcrouter/routes/RoutingUtils.h b/mcrouter/routes/RoutingUtils.h
@@ -7,7 +7,9 @@
 
 #pragma once
 
+#include <folly/json/dynamic.h>
 #include "mcrouter/McrouterFiberContext.h"
+#include "mcrouter/options.h"
 
 namespace facebook {
 namespace memcache {
@@ -141,6 +143,10 @@ folly::StringPiece routingKeyFiberLocal(const Request& req) {
   return req.key_ref()->routingKey();
 }
 
+uint32_t getAdditionalFanout(
+    const folly::dynamic& json,
+    const McrouterOptions& opts);
+
 } // namespace mcrouter
 } // namespace memcache
 } // namespace facebook
diff --git a/mcrouter/test/cpp_unit_tests/mc_route_handle_provider_test.cpp b/mcrouter/test/cpp_unit_tests/mc_route_handle_provider_test.cpp
@@ -56,6 +56,13 @@ const char* const kPoolRoute =
    "hash": { "hash_func": "Crc32" }
  })";
 
+const char* const kPoolRouteInvalidFanout =
+    R"({
+   "type": "PoolRoute",
+   "pool": { "name": "mock", "servers": [ ], "additional_fanout": 40000},
+   "hash": { "hash_func": "Crc32" },
+ })";
+
 const char* const kBucketizedSRRoute =
     R"({
    "type": "SRRoute",
@@ -163,6 +170,16 @@ TEST(McRouteHandleProvider, pool_route) {
   EXPECT_EQ("asynclog:mock", asynclogRoutes["mock"]->routeName());
 }
 
+TEST(McRouteHandleProvider, pool_route_with_invalid_fanout) {
+  try {
+    TestSetup setup;
+    auto rh = setup.getRoute(kPoolRouteInvalidFanout);
+  } catch (const std::logic_error&) {
+    return;
+  }
+  FAIL() << "No exception thrown";
+}
+
 TEST(McRouteHandleProvider, bucketized_sr_route_and_mcreplay_asynclogRoutes) {
   TestSetup setup;
   auto rh = setup.getRoute(kBucketizedSRRoute);