route policy map v2 kill switch

Summary: As discussed here: https://www.internalfb.com/diff/D50802118?dst_version_fbid=709812621040534&transaction_fbid=309906798576253 do a config based enablement. I also rewrote bool parameters to struct because keeping track of 3 booleans was too much. Reviewed By: disylh Differential Revision: D51497361 fbshipit-source-id: ea065ab1fe91e7eed358faec25b5905aee0af544
facebook · Nov 22, 2023 · d855ebc · d855ebc
1 parent ff4e533
commit d855ebc
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 53 deletions.
diff --git a/mcrouter/ProxyConfig-inl.h b/mcrouter/ProxyConfig-inl.h
@@ -107,28 +107,29 @@ ProxyConfig<RouterInfo>::ProxyConfig(
     partialConfigs_ = provider.releasePartialConfigs();
   }
   accessPoints_ = provider.releaseAccessPoints();
-  bool enableCrossRegionDeleteRpc = true;
-  if (auto* jEnableCrossRegionDeleteRpc =
-          json.get_ptr("enable_cross_region_delete_rpc")) {
-    enableCrossRegionDeleteRpc = parseBool(
-        *jEnableCrossRegionDeleteRpc, "enable_cross_region_delete_rpc");
-  }
+  auto readBool = [&](std::string_view key, bool defaultValue) {
+    if (auto* j = json.get_ptr(key)) {
+      return parseBool(*j, key);
+    }
+    return defaultValue;
+  };
 
-  bool enableDeleteDistribution = false;
-  if (auto* jEnableDeleteDistribution =
-          json.get_ptr("enable_delete_distribution")) {
-    enableDeleteDistribution =
-        parseBool(*jEnableDeleteDistribution, "enable_delete_distribution");
-  }
+  bool enableCrossRegionDeleteRpc =
+      readBool("enable_cross_region_delete_rpc", true);
+  bool enableDeleteDistribution = readBool("enable_delete_distribution", false);
   checkLogic(
       enableDeleteDistribution || enableCrossRegionDeleteRpc,
       "ProxyConfig: cannot disable cross-region delete rpc if distribution is disabled");
 
+  bool enablePolicyMapV2 = readBool("enable_policy_map_v2", false);
+
   proxyRoute_ = std::make_shared<ProxyRoute<RouterInfo>>(
       proxy,
       routeSelectors,
-      enableDeleteDistribution,
-      enableCrossRegionDeleteRpc);
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = enablePolicyMapV2,
+          .enableDeleteDistribution = enableDeleteDistribution,
+          .enableCrossRegionDeleteRpc = enableCrossRegionDeleteRpc});
   serviceInfo_ = std::make_shared<ServiceInfo<RouterInfo>>(proxy, *this);
 }
 

diff --git a/mcrouter/mcrouter_options_list.h b/mcrouter/mcrouter_options_list.h
@@ -924,14 +924,6 @@ MCROUTER_OPTION_INTEGER(
     no_short,
     "1 in S non-error connection samples will be logged")
 
-MCROUTER_OPTION_TOGGLE(
-    enable_route_policy_v2,
-    false,
-    "enable-route-policy-v2",
-    no_short,
-    "Enable Route Policy V2")
-
-
 #ifdef ADDITIONAL_OPTIONS_FILE
 #include ADDITIONAL_OPTIONS_FILE
 #endif

diff --git a/mcrouter/routes/ProxyRoute-inl.h b/mcrouter/routes/ProxyRoute-inl.h
@@ -14,7 +14,6 @@
 #include "mcrouter/routes/BigValueRoute.h"
 #include "mcrouter/routes/LoggingRoute.h"
 #include "mcrouter/routes/McRouteHandleBuilder.h"
-#include "mcrouter/routes/RootRoute.h"
 
 namespace facebook {
 namespace memcache {
@@ -42,14 +41,12 @@ template <class RouterInfo>
 ProxyRoute<RouterInfo>::ProxyRoute(
     Proxy<RouterInfo>& proxy,
     const RouteSelectorMap<typename RouterInfo::RouteHandleIf>& routeSelectors,
-    bool enableDeleteDistribution,
-    bool enableCrossRegionDeleteRpc)
+    RootRouteRolloutOpts rolloutOpts)
     : proxy_(proxy),
       root_(makeRouteHandleWithInfo<RouterInfo, RootRoute>(
           proxy_,
           routeSelectors,
-          enableDeleteDistribution,
-          enableCrossRegionDeleteRpc)) {
+          rolloutOpts)) {
   if (proxy_.getRouterOptions().big_value_split_threshold != 0) {
     root_ = detail::wrapWithBigValueRoute<RouterInfo>(
         std::move(root_), proxy_.getRouterOptions());

diff --git a/mcrouter/routes/ProxyRoute.h b/mcrouter/routes/ProxyRoute.h
@@ -20,6 +20,7 @@
 #include "mcrouter/lib/network/gen/MemcacheMessages.h"
 #include "mcrouter/lib/routes/AllSyncRoute.h"
 #include "mcrouter/routes/BigValueRouteIf.h"
+#include "mcrouter/routes/RootRoute.h"
 #include "mcrouter/routes/RouteSelectorMap.h"
 #include "mcrouter/stats.h"
 
@@ -44,8 +45,7 @@ class ProxyRoute {
       Proxy<RouterInfo>& proxy,
       const RouteSelectorMap<typename RouterInfo::RouteHandleIf>&
           routeSelectors,
-      bool enableDeleteDistribution = false,
-      bool enableCrossRegionDeleteRpc = true);
+      RootRouteRolloutOpts rolloutOpts);
 
   template <class Request>
   bool traverse(

diff --git a/mcrouter/routes/RootRoute.h b/mcrouter/routes/RootRoute.h
@@ -26,6 +26,12 @@ namespace facebook {
 namespace memcache {
 namespace mcrouter {
 
+struct RootRouteRolloutOpts {
+  bool enablePolicyMapV2 = false;
+  bool enableDeleteDistribution = false;
+  bool enableCrossRegionDeleteRpc = true;
+};
+
 template <class RouterInfo>
 class RootRoute {
  public:
@@ -37,17 +43,16 @@ class RootRoute {
       ProxyBase& proxy,
       const RouteSelectorMap<typename RouterInfo::RouteHandleIf>&
           routeSelectors,
-      bool enableDeleteDistribution = false,
-      bool enableCrossRegionDeleteRpc = true)
+      RootRouteRolloutOpts rolloutOpts)
       : opts_(proxy.getRouterOptions()),
         rhMap_(
             routeSelectors,
             opts_.default_route,
             opts_.send_invalid_route_to_default,
-            opts_.enable_route_policy_v2),
+            rolloutOpts.enablePolicyMapV2),
         defaultRoute_(opts_.default_route),
-        enableDeleteDistribution_(enableDeleteDistribution),
-        enableCrossRegionDeleteRpc_(enableCrossRegionDeleteRpc) {}
+        enableDeleteDistribution_(rolloutOpts.enableDeleteDistribution),
+        enableCrossRegionDeleteRpc_(rolloutOpts.enableCrossRegionDeleteRpc) {}
 
   template <class Request>
   bool traverse(

diff --git a/mcrouter/routes/test/RootRouteTest.cpp b/mcrouter/routes/test/RootRouteTest.cpp
@@ -76,10 +76,12 @@ class RootRouteTest : public ::testing::Test {
   RouteSelectorMap<typename MemcacheRouterInfo::RouteHandleIf> routeSelectors;
 };
 
+constexpr RootRouteRolloutOpts defaultOpts;
+
 TEST_F(RootRouteTest, NoRoutingPrefixGetRoutesToDefault) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   auto reply = rr.route(McGetRequest("getReq"));
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
   EXPECT_TRUE(getTestHandle(1)->saw_keys.empty());
@@ -97,7 +99,7 @@ TEST_F(RootRouteTest, NoRoutingPrefixGetRoutesToDefault) {
 TEST_F(RootRouteTest, BroadcastPrefixGetRoutesToAll) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McGetRequest("/*/*/getReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -120,7 +122,7 @@ TEST_F(RootRouteTest, BroadcastPrefixGetRoutesToAll) {
 TEST_F(RootRouteTest, DirectedCrossRegionPrefixGetRoutesToOne) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McGetRequest("/virginia/c/getReq")); }});
   EXPECT_TRUE(getTestHandle(0)->saw_keys.empty());
@@ -139,7 +141,7 @@ TEST_F(RootRouteTest, DirectedCrossRegionPrefixGetRoutesToOne) {
 TEST_F(RootRouteTest, NoRoutingPrefixSetRoutesToDefault) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   auto reply = rr.route(McSetRequest("setReq"));
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
   EXPECT_TRUE(getTestHandle(1)->saw_keys.empty());
@@ -157,7 +159,7 @@ TEST_F(RootRouteTest, NoRoutingPrefixSetRoutesToDefault) {
 TEST_F(RootRouteTest, BroadcastPrefixSetRoutesToAll) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McSetRequest("/*/*/setReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -180,7 +182,7 @@ TEST_F(RootRouteTest, BroadcastPrefixSetRoutesToAll) {
 TEST_F(RootRouteTest, DirectedCrossRegionPrefixSetRoutesToOne) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McSetRequest("/virginia/c/setReq")); }});
   EXPECT_TRUE(getTestHandle(0)->saw_keys.empty());
@@ -199,7 +201,7 @@ TEST_F(RootRouteTest, DirectedCrossRegionPrefixSetRoutesToOne) {
 TEST_F(RootRouteTest, NoRoutingPrefixDeleteRoutesToDefault) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   auto reply = rr.route(McDeleteRequest("delReq"));
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
   EXPECT_TRUE(getTestHandle(1)->saw_keys.empty());
@@ -217,7 +219,7 @@ TEST_F(RootRouteTest, NoRoutingPrefixDeleteRoutesToDefault) {
 TEST_F(RootRouteTest, BroadcastPrefixDeleteRoutesToAll) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/*/*/delReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -240,7 +242,7 @@ TEST_F(RootRouteTest, BroadcastPrefixDeleteRoutesToAll) {
 TEST_F(RootRouteTest, DirectedCrossRegionPrefixDeleteRoutesToOne) {
   mockFiberContext();
   auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors()};
+  RootRoute<MemcacheRouterInfo> rr{*proxy, getRouteSelectors(), defaultOpts};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/virginia/c/delReq")); }});
   EXPECT_TRUE(getTestHandle(0)->saw_keys.empty());
@@ -262,8 +264,11 @@ TEST_F(RootRouteTest, NoRoutingPrefixDeleteWithDistributionOnRoutesToDefault) {
   RootRoute<MemcacheRouterInfo> rr{
       *proxy,
       getRouteSelectors(),
-      /*enableDeleteDistribution*/ true,
-      /*enableBroadcastDeleteRpc*/ true};
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = true,
+          .enableDeleteDistribution = true,
+          .enableCrossRegionDeleteRpc = true,
+      }};
   auto reply = rr.route(McDeleteRequest("delReq"));
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
   EXPECT_TRUE(getTestHandle(1)->saw_keys.empty());
@@ -284,8 +289,11 @@ TEST_F(RootRouteTest, BroadcastPrefixDeleteWithDistributionOnRoutesToAll) {
   RootRoute<MemcacheRouterInfo> rr{
       *proxy,
       getRouteSelectors(),
-      /*enableDeleteDistribution*/ true,
-      /*enableBroadcastDeleteRpc*/ true};
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = true,
+          .enableDeleteDistribution = true,
+          .enableCrossRegionDeleteRpc = true,
+      }};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/*/*/delReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -314,8 +322,11 @@ TEST_F(
   RootRoute<MemcacheRouterInfo> rr{
       *proxy,
       getRouteSelectors(),
-      /*enableDeleteDistribution*/ true,
-      /*enableBroadcastDeleteRpc*/ true};
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = true,
+          .enableDeleteDistribution = true,
+          .enableCrossRegionDeleteRpc = true,
+      }};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/virginia/c/delReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -341,8 +352,11 @@ TEST_F(
   RootRoute<MemcacheRouterInfo> rr{
       *proxy,
       getRouteSelectors(),
-      /*enableDeleteDistribution*/ true,
-      /*enableBroadcastDeleteRpc*/ false};
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = true,
+          .enableDeleteDistribution = true,
+          .enableCrossRegionDeleteRpc = false,
+      }};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/*/*/delReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());
@@ -366,8 +380,11 @@ TEST_F(
   RootRoute<MemcacheRouterInfo> rr{
       *proxy,
       getRouteSelectors(),
-      /*enableDeleteDistribution*/ true,
-      /*enableBroadcastDeleteRpc*/ false};
+      RootRouteRolloutOpts{
+          .enablePolicyMapV2 = true,
+          .enableDeleteDistribution = true,
+          .enableCrossRegionDeleteRpc = false,
+      }};
   TestFiberManager<MemcacheRouterInfo> fm;
   fm.runAll({[&]() { rr.route(McDeleteRequest("/virginia/c/delReq")); }});
   EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());