remove bucketize_until parameter

Summary:
This param was intended for rollout of the bucketization.
Now that it is everywhere, there's no need for it. Furthermore, we want to change the bucket id to be string literals not always convertible to integers, so we won't be able to do math on them.

Reviewed By: stuclar

Differential Revision: D52550763

fbshipit-source-id: 0a70fa3f69a3285c0e0a1b1d41db977a7365a38c

Author: Lenar Fatikhov

third-party/mcrouter/src/mcrouter/routes/McBucketRoute-inl.h

@@ -17,9 +17,6 @@ namespace facebook::memcache::mcrouter {
  *   McBucketRoute Config:
  * - bucketize(bool)        - enable the bucketization
  * - total_buckets(int)     - total number of buckets
- * - bucketize_until(int)   - enable the handle for buckets until (exclusive)
- *                            this number. Must be less than total_buckets.
- *                            Needed for gradual migration.
  * - bucketization_keyspace - can be used to separate into multiple
  *                            bucketization domains decoupled from each other,
  *                            e.g. one keyspace for each Memache pool.

third-party/mcrouter/src/mcrouter/routes/McBucketRoute.cpp

@@ -20,12 +20,6 @@ McBucketRouteSettings parseMcBucketRouteSettings(const folly::dynamic& json) {
       1,
       std::numeric_limits<int64_t>::max());
 
-  auto bucketizeUntil = 0;
-  if (json.count("bucketize_until")) {
-    bucketizeUntil = parseInt(
-        *json.get_ptr("bucketize_until"), "bucketize_until", 0, totalBuckets);
-  }
-
   auto* bucketizationKeyspacePtr = json.get_ptr("bucketization_keyspace");
   checkLogic(
       bucketizationKeyspacePtr && bucketizationKeyspacePtr->isString(),
@@ -38,7 +32,6 @@ McBucketRouteSettings parseMcBucketRouteSettings(const folly::dynamic& json) {
   }
 
   settings.totalBuckets = totalBuckets;
-  settings.bucketizeUntil = bucketizeUntil;
   return settings;
 }
 } // namespace mcrouter

third-party/mcrouter/src/mcrouter/routes/McBucketRoute.h

@@ -19,7 +19,6 @@ namespace facebook::memcache::mcrouter {
 
 struct McBucketRouteSettings {
   size_t totalBuckets;
-  size_t bucketizeUntil;
   std::string salt;
   std::string bucketizationKeyspace;
 };
@@ -52,16 +51,14 @@ class McBucketRoute {
   McBucketRoute(RouteHandlePtr rh, McBucketRouteSettings& settings)
       : rh_(std::move(rh)),
         totalBuckets_(settings.totalBuckets),
-        bucketizeUntil_(settings.bucketizeUntil),
         salt_(settings.salt),
         ch3_(totalBuckets_),
         bucketizationKeyspace_(settings.bucketizationKeyspace) {}
 
   std::string routeName() const {
     return fmt::format(
-        "bucketize|total_buckets={}|bucketize_until={}|salt={}|bucketization_keyspace={}",
+        "bucketize|total_buckets={}|salt={}|bucketization_keyspace={}",
         totalBuckets_,
-        bucketizeUntil_,
         salt_,
         bucketizationKeyspace_);
   }
@@ -82,20 +79,16 @@ class McBucketRoute {
         return ch3_(getRoutingKey<Request>(req, this->salt_));
       }
     });
-    if (bucketId < bucketizeUntil_) {
-      if (auto* ctx = fiber_local<RouterInfo>::getTraverseCtx()) {
-        ctx->recordBucketizationData(
-            req.key_ref()->keyWithoutRoute().str(),
-            bucketId,
-            bucketizationKeyspace_);
-      }
-      return fiber_local<RouterInfo>::runWithLocals(
-          [this, &req, &t, bucketId]() {
-            fiber_local<RouterInfo>::setBucketId(bucketId);
-            return t(*rh_, req);
-          });
+    if (auto* ctx = fiber_local<RouterInfo>::getTraverseCtx()) {
+      ctx->recordBucketizationData(
+          req.key_ref()->keyWithoutRoute().str(),
+          bucketId,
+          bucketizationKeyspace_);
     }
-    return t(*rh_, req);
+    return fiber_local<RouterInfo>::runWithLocals([this, &req, &t, bucketId]() {
+      fiber_local<RouterInfo>::setBucketId(bucketId);
+      return t(*rh_, req);
+    });
   }
 
   template <class Request>
@@ -121,21 +114,17 @@ class McBucketRoute {
 
   template <class Request>
   ReplyT<Request> routeImpl(const Request& req, const size_t bucketId) const {
-    if (bucketId < bucketizeUntil_) {
-      auto proxy = &fiber_local<RouterInfo>::getSharedCtx()->proxy();
-      proxy->stats().increment(bucketized_routing_stat);
-      return fiber_local<RouterInfo>::runWithLocals([this, &req, bucketId]() {
-        fiber_local<RouterInfo>::setBucketId(bucketId);
-        return rh_->route(req);
-      });
-    }
-    return rh_->route(req);
+    auto proxy = &fiber_local<RouterInfo>::getSharedCtx()->proxy();
+    proxy->stats().increment(bucketized_routing_stat);
+    return fiber_local<RouterInfo>::runWithLocals([this, &req, bucketId]() {
+      fiber_local<RouterInfo>::setBucketId(bucketId);
+      return rh_->route(req);
+    });
   }
 
  private:
   const RouteHandlePtr rh_;
   const size_t totalBuckets_{0};
-  const size_t bucketizeUntil_{0};
   const std::string salt_;
   const Ch3HashFunc ch3_;
   const std::string bucketizationKeyspace_;

third-party/mcrouter/src/mcrouter/routes/test/McBucketRouteTest.cpp

@@ -35,7 +35,6 @@ TEST(McBucketRouteTest, bucketIdShouldPropagate) {
   {
     "bucketize": true,
     "total_buckets": 100,
-    "bucketize_until": 29,
     "bucketization_keyspace": "testRegion:testPool"
   }
   )";
@@ -51,32 +50,6 @@ TEST(McBucketRouteTest, bucketIdShouldPropagate) {
   EXPECT_EQ(28, srHandleVec[0]->sawBucketIds[0]);
 }
 
-TEST(McBucketRouteTest, bucketIdShouldNotPropagate) {
-  std::vector<std::shared_ptr<TestHandle>> srHandleVec{
-      std::make_shared<TestHandle>(
-          GetRouteTestData(carbon::Result::FOUND, "a")),
-  };
-  auto mockSrHandle = get_route_handles(srHandleVec)[0];
-
-  constexpr folly::StringPiece kMcBucketRouteConfig = R"(
-  {
-    "bucketize": true,
-    "total_buckets": 100,
-    "bucketize_until": 27,
-    "bucketization_keyspace": "testRegion:testPool"
-  }
-  )";
-
-  auto rh = makeMcBucketRoute<MemcacheRouterInfo>(
-      mockSrHandle, folly::parseJson(kMcBucketRouteConfig));
-  ASSERT_TRUE(rh);
-  mockFiberContext();
-  rh->route(McGetRequest("getReq")); // bucketId == 28
-  auto proxy = &fiber_local<MemcacheRouterInfo>::getSharedCtx()->proxy();
-  EXPECT_EQ(0, proxy->stats().getValue(bucketized_routing_stat));
-  EXPECT_TRUE(srHandleVec[0]->sawBucketIds.empty());
-}
-
 TEST(McBucketRouteTest, bucketIdShouldPropagateInTraverse) {
   std::vector<std::shared_ptr<TestHandle>> srHandleVec{
       std::make_shared<TestHandle>(
@@ -88,7 +61,6 @@ TEST(McBucketRouteTest, bucketIdShouldPropagateInTraverse) {
   {
     "bucketize": true,
     "total_buckets": 100,
-    "bucketize_until": 29,
     "bucketization_keyspace": "testRegion:testPool"
   }
   )";
@@ -102,51 +74,24 @@ TEST(McBucketRouteTest, bucketIdShouldPropagateInTraverse) {
   EXPECT_EQ(28, srHandleVec[0]->sawBucketIds[0]);
 }
 
-TEST(McBucketRouteTest, bucketIdShouldNotPropagateInTraverse) {
-  std::vector<std::shared_ptr<TestHandle>> srHandleVec{
-      std::make_shared<TestHandle>(
-          GetRouteTestData(carbon::Result::FOUND, "a")),
-  };
-  auto mockSrHandle = get_route_handles(srHandleVec)[0];
-
-  constexpr folly::StringPiece kMcBucketRouteConfig = R"(
-  {
-    "bucketize": true,
-    "total_buckets": 100,
-    "bucketize_until": 28,
-    "bucketization_keyspace": "testRegion:testPool"
-  }
-  )";
-
-  auto rh = makeMcBucketRoute<MemcacheRouterInfo>(
-      mockSrHandle, folly::parseJson(kMcBucketRouteConfig));
-  mockFiberContext();
-  RouteHandleTraverser<MemcacheRouterInfo::RouteHandleIf> t{};
-  rh->traverse(McGetRequest("getReq"), t); // bucketId == 28
-  EXPECT_TRUE(srHandleVec[0]->sawBucketIds.empty());
-}
-
 TEST(McBucketRouteTest, checkParams) {
   std::vector<std::shared_ptr<TestHandle>> srHandleVec{
       std::make_shared<TestHandle>(
           GetRouteTestData(carbon::Result::FOUND, "a")),
   };
   auto mockSrHandle = get_route_handles(srHandleVec)[0];
   std::string_view total = "100";
-  std::string_view until = "29";
   std::string_view keyspace = "testReg:testPool";
 
   std::string kMcBucketRouteConfig = folly::sformat(
       R"(
   {{
     "bucketize": true,
     "total_buckets": {},
-    "bucketize_until": {},
     "bucketization_keyspace": "{}"
   }}
   )",
       total,
-      until,
       keyspace);
 
   auto rh = makeMcBucketRoute<MemcacheRouterInfo>(
@@ -155,13 +100,12 @@ TEST(McBucketRouteTest, checkParams) {
   auto name = rh->routeName();
   std::vector<std::string> params;
   folly::split('|', name, params, true);
-  EXPECT_TRUE(params.size() == 5);
+  EXPECT_TRUE(params.size() == 4);
   EXPECT_EQ(params[0], "bucketize");
   EXPECT_EQ(params[1], folly::to<std::string>("total_buckets=", total));
-  EXPECT_EQ(params[2], folly::to<std::string>("bucketize_until=", until));
-  EXPECT_EQ(params[3], folly::to<std::string>("salt="));
+  EXPECT_EQ(params[2], folly::to<std::string>("salt="));
   EXPECT_EQ(
-      params[4], folly::to<std::string>("bucketization_keyspace=", keyspace));
+      params[3], folly::to<std::string>("bucketization_keyspace=", keyspace));
 }
 
 TEST(McBucketRouteTest, recordBucketizationData) {
@@ -175,7 +119,6 @@ TEST(McBucketRouteTest, recordBucketizationData) {
   {
     "bucketize": true,
     "total_buckets": 100,
-    "bucketize_until": 100,
     "bucketization_keyspace": "testRegionBucketKeyspace"
   }
   )";

third-party/mcrouter/src/mcrouter/test/cpp_unit_tests/mc_route_handle_provider_test.cpp

@@ -65,7 +65,6 @@ const char* const kBucketizedSRRoute =
    "axonlog": false,
    "bucketize": true,
    "total_buckets": 1000,
-   "bucketize_until": 1000,
    "bucketization_keyspace": "tst"
 })";
 
@@ -79,7 +78,6 @@ const char* const kBucketizedPoolRoute =
    "axonlog": false,
    "bucketize": true,
    "total_buckets": 1000,
-   "bucketize_until": 1000,
    "bucketization_keyspace": "tst"
 })";
 
@@ -156,12 +154,12 @@ TEST(McRouteHandleProvider, bucketized_sr_route_and_mcreplay_asynclogRoutes) {
   auto rh = setup.getRoute(kBucketizedSRRoute);
   EXPECT_TRUE(rh != nullptr);
   EXPECT_EQ(
-      "bucketize|total_buckets=1000|bucketize_until=1000|salt=|bucketization_keyspace=tst",
+      "bucketize|total_buckets=1000|salt=|bucketization_keyspace=tst",
       rh->routeName());
   auto asynclogRoutes = setup.provider().releaseAsyncLogRoutes();
   EXPECT_EQ(1, asynclogRoutes.size());
   EXPECT_EQ(
-      "bucketize|total_buckets=1000|bucketize_until=1000|salt=|bucketization_keyspace=tst",
+      "bucketize|total_buckets=1000|salt=|bucketization_keyspace=tst",
       asynclogRoutes["test.asynclog"]->routeName());
 }
 
@@ -170,11 +168,11 @@ TEST(McRouteHandleProvider, bucketized_pool_route_and_mcreplay_asynclogRoutes) {
   auto rh = setup.getRoute(kBucketizedPoolRoute);
   EXPECT_TRUE(rh != nullptr);
   EXPECT_EQ(
-      "bucketize|total_buckets=1000|bucketize_until=1000|salt=|bucketization_keyspace=tst",
+      "bucketize|total_buckets=1000|salt=|bucketization_keyspace=tst",
       rh->routeName());
   auto asynclogRoutes = setup.provider().releaseAsyncLogRoutes();
   EXPECT_EQ(1, asynclogRoutes.size());
   EXPECT_EQ(
-      "bucketize|total_buckets=1000|bucketize_until=1000|salt=|bucketization_keyspace=tst",
+      "bucketize|total_buckets=1000|salt=|bucketization_keyspace=tst",
       asynclogRoutes["test.asynclog"]->routeName());
 }

third-party/mcrouter/src/mcrouter/test/mcrouter_test_bucketized_poolroute_bucketized.json

@@ -10,7 +10,6 @@
     "pool": "foo",
     "bucketize": true,
     "total_buckets": 1000,
-    "bucketize_until": 1000,
     "bucketization_keyspace": "test",
     "hash": "WeightedCh3"
   }

third-party/mcrouter/src/mcrouter/test/test_axon_log.json

@@ -10,7 +10,6 @@
     "pool": "A",
     "protocol": "thrift",
     "bucketize": true,
-    "bucketize_until": 1000,
     "bucketization_keyspace": "test",
     "total_buckets": 1000,
     "hash": "WeightedCh3",

third-party/mcrouter/src/mcrouter/test/test_axon_log_alldelete.json

@@ -10,7 +10,6 @@
     "pool": "A",
     "protocol": "thrift",
     "bucketize": true,
-    "bucketize_until": 1000,
     "bucketization_keyspace": "test",
     "total_buckets": 1000,
     "hash": "WeightedCh3",