diff --git a/mcrouter/ProxyConfig-inl.h b/mcrouter/ProxyConfig-inl.h index 2dafb3524..bd96a445f 100644 --- a/mcrouter/ProxyConfig-inl.h +++ b/mcrouter/ProxyConfig-inl.h @@ -107,28 +107,29 @@ ProxyConfig::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>( proxy, routeSelectors, - enableDeleteDistribution, - enableCrossRegionDeleteRpc); + RootRouteRolloutOpts{ + .enablePolicyMapV2 = enablePolicyMapV2, + .enableDeleteDistribution = enableDeleteDistribution, + .enableCrossRegionDeleteRpc = enableCrossRegionDeleteRpc}); serviceInfo_ = std::make_shared>(proxy, *this); } diff --git a/mcrouter/mcrouter_options_list.h b/mcrouter/mcrouter_options_list.h index 5e97215ac..72d79e8e9 100644 --- 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 index e49a3dc92..ea902ad64 100644 --- 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 ProxyRoute::ProxyRoute( Proxy& proxy, const RouteSelectorMap& routeSelectors, - bool enableDeleteDistribution, - bool enableCrossRegionDeleteRpc) + RootRouteRolloutOpts rolloutOpts) : proxy_(proxy), root_(makeRouteHandleWithInfo( proxy_, routeSelectors, - enableDeleteDistribution, - enableCrossRegionDeleteRpc)) { + rolloutOpts)) { if (proxy_.getRouterOptions().big_value_split_threshold != 0) { root_ = detail::wrapWithBigValueRoute( std::move(root_), proxy_.getRouterOptions()); diff --git a/mcrouter/routes/ProxyRoute.h b/mcrouter/routes/ProxyRoute.h index 2da9d111f..e02a0a8d2 100644 --- 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& proxy, const RouteSelectorMap& routeSelectors, - bool enableDeleteDistribution = false, - bool enableCrossRegionDeleteRpc = true); + RootRouteRolloutOpts rolloutOpts); template bool traverse( diff --git a/mcrouter/routes/RootRoute.h b/mcrouter/routes/RootRoute.h index 06837497d..7f948eb3e 100644 --- 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 RootRoute { public: @@ -37,17 +43,16 @@ class RootRoute { ProxyBase& proxy, const RouteSelectorMap& 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 bool traverse( diff --git a/mcrouter/routes/test/RootRouteTest.cpp b/mcrouter/routes/test/RootRouteTest.cpp index 5666f1b05..0805138d7 100644 --- a/mcrouter/routes/test/RootRouteTest.cpp +++ b/mcrouter/routes/test/RootRouteTest.cpp @@ -76,10 +76,12 @@ class RootRouteTest : public ::testing::Test { RouteSelectorMap routeSelectors; }; +constexpr RootRouteRolloutOpts defaultOpts; + TEST_F(RootRouteTest, NoRoutingPrefixGetRoutesToDefault) { mockFiberContext(); auto proxy = &fiber_local::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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::getSharedCtx()->proxy(); - RootRoute rr{*proxy, getRouteSelectors()}; + RootRoute rr{*proxy, getRouteSelectors(), defaultOpts}; TestFiberManager 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 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 rr{ *proxy, getRouteSelectors(), - /*enableDeleteDistribution*/ true, - /*enableBroadcastDeleteRpc*/ true}; + RootRouteRolloutOpts{ + .enablePolicyMapV2 = true, + .enableDeleteDistribution = true, + .enableCrossRegionDeleteRpc = true, + }}; TestFiberManager fm; fm.runAll({[&]() { rr.route(McDeleteRequest("/*/*/delReq")); }}); EXPECT_FALSE(getTestHandle(0)->saw_keys.empty()); @@ -314,8 +322,11 @@ TEST_F( RootRoute rr{ *proxy, getRouteSelectors(), - /*enableDeleteDistribution*/ true, - /*enableBroadcastDeleteRpc*/ true}; + RootRouteRolloutOpts{ + .enablePolicyMapV2 = true, + .enableDeleteDistribution = true, + .enableCrossRegionDeleteRpc = true, + }}; TestFiberManager fm; fm.runAll({[&]() { rr.route(McDeleteRequest("/virginia/c/delReq")); }}); EXPECT_FALSE(getTestHandle(0)->saw_keys.empty()); @@ -341,8 +352,11 @@ TEST_F( RootRoute rr{ *proxy, getRouteSelectors(), - /*enableDeleteDistribution*/ true, - /*enableBroadcastDeleteRpc*/ false}; + RootRouteRolloutOpts{ + .enablePolicyMapV2 = true, + .enableDeleteDistribution = true, + .enableCrossRegionDeleteRpc = false, + }}; TestFiberManager fm; fm.runAll({[&]() { rr.route(McDeleteRequest("/*/*/delReq")); }}); EXPECT_FALSE(getTestHandle(0)->saw_keys.empty()); @@ -366,8 +380,11 @@ TEST_F( RootRoute rr{ *proxy, getRouteSelectors(), - /*enableDeleteDistribution*/ true, - /*enableBroadcastDeleteRpc*/ false}; + RootRouteRolloutOpts{ + .enablePolicyMapV2 = true, + .enableDeleteDistribution = true, + .enableCrossRegionDeleteRpc = false, + }}; TestFiberManager fm; fm.runAll({[&]() { rr.route(McDeleteRequest("/virginia/c/delReq")); }}); EXPECT_FALSE(getTestHandle(0)->saw_keys.empty());