diff --git a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.cpp b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.cpp index 3153a5292..f05d8d37b 100644 --- a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.cpp +++ b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.cpp @@ -25,7 +25,7 @@ std::ostream& operator<<(std::ostream& os, const SmallPrefix& self) { } LowerBoundPrefixMapCommon::LowerBoundPrefixMapCommon( - std::span sortedUniquePrefixes) { + const std::vector& sortedUniquePrefixes) { smallPrefixes_.reserve(sortedUniquePrefixes.size() + 1); markers_.reserve(sortedUniquePrefixes.size() + 1); previousPrefix_.reserve(sortedUniquePrefixes.size()); diff --git a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h index 32857d98b..7c7f7124d 100644 --- a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h +++ b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h @@ -8,16 +8,15 @@ #pragma once #include +#include #include #include #include #include -#include #include #include #include -#include #include #include #include @@ -45,8 +44,38 @@ class SmallPrefix { data_ = folly::Endian::swap(data_); } - bool operator==(const SmallPrefix&) const = default; - auto operator<=>(const SmallPrefix&) const = default; + // default operator== and operator<=> are ok here + // but not everything is C++20. + + FOLLY_ALWAYS_INLINE + friend bool operator==(const SmallPrefix& x, const SmallPrefix& y) { + return x.data_ == y.data_; + } + + FOLLY_ALWAYS_INLINE + friend bool operator!=(const SmallPrefix& x, const SmallPrefix& y) { + return !(x == y); + } + + FOLLY_ALWAYS_INLINE + friend bool operator<(const SmallPrefix& x, const SmallPrefix& y) { + return x.data_ < y.data_; + } + + FOLLY_ALWAYS_INLINE + friend bool operator<=(const SmallPrefix& x, const SmallPrefix& y) { + return !(y < x); + } + + FOLLY_ALWAYS_INLINE + friend bool operator>=(const SmallPrefix& x, const SmallPrefix& y) { + return !(x < y); + } + + FOLLY_ALWAYS_INLINE + friend bool operator>(const SmallPrefix& x, const SmallPrefix& y) { + return y < x; + } friend std::ostream& operator<<(std::ostream& os, const SmallPrefix& self); @@ -72,7 +101,7 @@ class SmallPrefix { struct LowerBoundPrefixMapCommon { LowerBoundPrefixMapCommon() = default; explicit LowerBoundPrefixMapCommon( - std::span sortedUniquePrefixes); + const std::vector& sortedUniquePrefixes); // returns 1 based indexes, 0 if not found. std::uint32_t findPrefix(std::string_view query) const noexcept; @@ -298,8 +327,7 @@ LowerBoundPrefixMap::LowerBoundPrefixMap( prefix2value.rend(), [](const auto& x, const auto& y) { return x.first == y.first; }); - std::span> sortedUnique{ - rend.base(), prefix2value.end()}; + folly::Range sortedUnique{rend.base(), prefix2value.end()}; std::vector sortedPrefixes; sortedPrefixes.reserve(sortedUnique.size()); diff --git a/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp b/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp index 6742dd618..6626a9a6d 100644 --- a/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp +++ b/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp @@ -201,6 +201,8 @@ TEST(LowerBoundPrefixMapTest, OverrideValues) { TEST(LowerBoundPrefixMapTest, EmptyMap) { LowerBoundPrefixMap lbMap; ASSERT_EQ(lbMap.findPrefix("a"), lbMap.end()); + ASSERT_EQ(lbMap.size(), 0); + ASSERT_TRUE(lbMap.empty()); } } // namespace diff --git a/mcrouter/mcrouter_options_list.h b/mcrouter/mcrouter_options_list.h index 72d79e8e9..5e97215ac 100644 --- a/mcrouter/mcrouter_options_list.h +++ b/mcrouter/mcrouter_options_list.h @@ -924,6 +924,14 @@ 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/RootRoute.h b/mcrouter/routes/RootRoute.h index c4e434a62..84fffd57a 100644 --- a/mcrouter/routes/RootRoute.h +++ b/mcrouter/routes/RootRoute.h @@ -41,7 +41,8 @@ class RootRoute { rhMap_( routeSelectors, opts_.default_route, - opts_.send_invalid_route_to_default), + opts_.send_invalid_route_to_default, + opts_.enable_route_policy_v2), disableBroadcastDeleteRpc_(disableBroadcastDeleteRpc) {} template diff --git a/mcrouter/routes/RouteHandleMap-inl.h b/mcrouter/routes/RouteHandleMap-inl.h index 7d30f69fa..7e9a0a7ff 100644 --- a/mcrouter/routes/RouteHandleMap-inl.h +++ b/mcrouter/routes/RouteHandleMap-inl.h @@ -56,12 +56,14 @@ using UniqueVectorMap = std::unordered_map< template std::shared_ptr> makePolicyMap( UniqueVectorMap& uniqueVectors, - const RouteSelectorVector& v) { + const RouteSelectorVector& v, + bool enableRoutePolicyV2) { auto it = uniqueVectors.find(v); if (it != uniqueVectors.end()) { return it->second; } - return uniqueVectors[v] = std::make_shared>(v); + return uniqueVectors[v] = std::make_shared>( + v, enableRoutePolicyV2); } } // namespace detail @@ -70,9 +72,11 @@ template RouteHandleMap::RouteHandleMap( const RouteSelectorMap& routeSelectors, const RoutingPrefix& defaultRoute, - bool sendInvalidRouteToDefault) + bool sendInvalidRouteToDefault, + bool enableRoutePolicyV2) : defaultRoute_(defaultRoute), - sendInvalidRouteToDefault_(sendInvalidRouteToDefault) { + sendInvalidRouteToDefault_(sendInvalidRouteToDefault), + enableRoutePolicyV2_(enableRoutePolicyV2) { checkLogic( routeSelectors.find(defaultRoute_) != routeSelectors.end(), "invalid default route: {}", @@ -111,12 +115,16 @@ RouteHandleMap::RouteHandleMap( // create corresponding RoutePolicyMaps detail::UniqueVectorMap uniqueVectors; - allRoutes_ = makePolicyMap(uniqueVectors, allRoutes); + allRoutes_ = makePolicyMap(uniqueVectors, allRoutes, enableRoutePolicyV2_); for (const auto& it : byRegion) { - byRegion_.emplace(it.first, makePolicyMap(uniqueVectors, it.second)); + byRegion_.emplace( + it.first, + makePolicyMap(uniqueVectors, it.second, enableRoutePolicyV2_)); } for (const auto& it : byRoute) { - byRoute_.emplace(it.first, makePolicyMap(uniqueVectors, it.second)); + byRoute_.emplace( + it.first, + makePolicyMap(uniqueVectors, it.second, enableRoutePolicyV2_)); } assert(byRoute_.find(defaultRoute_) != byRoute_.end()); diff --git a/mcrouter/routes/RouteHandleMap.h b/mcrouter/routes/RouteHandleMap.h index 2ef69c109..5ad60c629 100644 --- a/mcrouter/routes/RouteHandleMap.h +++ b/mcrouter/routes/RouteHandleMap.h @@ -34,7 +34,8 @@ class RouteHandleMap { RouteHandleMap( const RouteSelectorMap& routeSelectors, const RoutingPrefix& defaultRoute, - bool sendInvalidRouteToDefault); + bool sendInvalidRouteToDefault, + bool enableRoutePolicyV2); /** * @return pointer to a precalculated vector of route handles that a request @@ -57,6 +58,7 @@ class RouteHandleMap { const std::vector> emptyV_; const RoutingPrefix& defaultRoute_; bool sendInvalidRouteToDefault_; + bool enableRoutePolicyV2_; std::shared_ptr> defaultRouteMap_; std::shared_ptr> allRoutes_; diff --git a/mcrouter/routes/RoutePolicyMap-inl.h b/mcrouter/routes/RoutePolicyMap-inl.h index 1140b9174..7700d58b4 100644 --- a/mcrouter/routes/RoutePolicyMap-inl.h +++ b/mcrouter/routes/RoutePolicyMap-inl.h @@ -50,7 +50,13 @@ std::vector> overrideItems( template RoutePolicyMap::RoutePolicyMap( const std::vector>>& - clusters) { + clusters, + bool useV2) { + if (useV2) { + v2_ = RoutePolicyMapV2(clusters); + return; + } + // wildcards of all clusters std::vector> wildcards; wildcards.reserve(clusters.size()); @@ -90,9 +96,84 @@ RoutePolicyMap::RoutePolicyMap( template const std::vector>& RoutePolicyMap::getTargetsForKey(folly::StringPiece key) const { + // empty means not initialized - i.e. the flag was not enabled. + if (!v2_.empty()) { + return v2_.getTargetsForKey(key); + } auto result = ut_.findPrefix(key); return result == ut_.end() ? emptyV_ : result->second; } + +template +RoutePolicyMapV2::RoutePolicyMapV2( + const std::vector& clusters) { + typename LBRouteMap::Builder builder; + + // not enough but it's not a problem, we will avoid at least some + // reallocations. + builder.reserve(clusters.size() + 1); + builder.insert({"", populateWildCards(clusters)}); + + // Combining routes for each key + folly::F14FastSet seen; + for (const auto& cluster : clusters) { + for (const auto& [key, _] : cluster->policies) { + if (seen.insert(key).second) { + builder.insert({key, populateRoutesForKey(key, clusters)}); + } + } + } + + ut_ = std::move(builder).build(); +} + +template +// static +auto RoutePolicyMapV2::populateWildCards( + const std::vector& clusters) + -> std::vector { + std::vector res; + res.reserve(clusters.size()); + + folly::F14FastSet seen; + for (const auto& cluster : clusters) { + const SharedRoutePtr& ptr = cluster->wildcard; + if (ptr && seen.insert(ptr.get()).second) { + res.push_back(cluster->wildcard); + } + } + + // This code is cold but the vector stays around for a while + res.shrink_to_fit(); + return res; +} + +template +// static +auto RoutePolicyMapV2::populateRoutesForKey( + std::string_view key, + const std::vector& clusters) + -> std::vector { + std::vector> res; + res.reserve(clusters.size()); + + folly::F14FastSet seen; + seen.reserve(clusters.size()); + + for (const auto& cluster : clusters) { + auto found = cluster->policies.findPrefix(key); + const SharedRoutePtr& ptr = + found == cluster->policies.end() ? cluster->wildcard : found->second; + if (ptr && seen.insert(ptr.get()).second) { + res.push_back(ptr); + } + } + + // This code is cold but the vector stays around for a while + res.shrink_to_fit(); + return res; +} + } // namespace mcrouter } // namespace memcache } // namespace facebook diff --git a/mcrouter/routes/RoutePolicyMap.h b/mcrouter/routes/RoutePolicyMap.h index 66add32ef..387b62cff 100644 --- a/mcrouter/routes/RoutePolicyMap.h +++ b/mcrouter/routes/RoutePolicyMap.h @@ -8,10 +8,13 @@ #pragma once #include +#include #include #include +#include +#include "mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h" #include "mcrouter/lib/fbi/cpp/Trie.h" namespace facebook { @@ -44,12 +47,49 @@ class PrefixSelectorRoute; * 2) use getTargetsForKey to get precalculated vector of targets. * Complexity is O(min(longest key prefix in config, key length)) */ +template +class RoutePolicyMapV2 { + public: + using SharedRoutePtr = std::shared_ptr; + using SharedClusterPtr = std::shared_ptr>; + + RoutePolicyMapV2() = default; + explicit RoutePolicyMapV2(const std::vector& clusters); + + // NOTE: change to span when the migration is over + const std::vector& getTargetsForKey( + std::string_view key) const { + // has to be there by construction + auto found = ut_.findPrefix(key); + // RoutePolicyMap always has wildcard which matches everything. + CHECK(found != ut_.end()); + return found->value(); + } + + bool empty() const { + return ut_.empty(); + } + + private: + static std::vector populateWildCards( + const std::vector& clusters); + + static std::vector populateRoutesForKey( + std::string_view key, + const std::vector& clusters); + + using LBRouteMap = LowerBoundPrefixMap>; + + LBRouteMap ut_; +}; + template class RoutePolicyMap { public: explicit RoutePolicyMap( const std::vector>>& - clusters); + clusters, + bool useV2); /** * @return vector of route handles that a request with given key should be @@ -59,6 +99,7 @@ class RoutePolicyMap { folly::StringPiece key) const; private: + RoutePolicyMapV2 v2_; const std::vector> emptyV_; /** * This Trie contains targets for each key prefix. It is built like this: @@ -68,6 +109,7 @@ class RoutePolicyMap { */ Trie>> ut_; }; + } // namespace mcrouter } // namespace memcache } // namespace facebook diff --git a/mcrouter/routes/test/RoutePolicyMapTest.cpp b/mcrouter/routes/test/RoutePolicyMapTest.cpp index c3f4a9074..2f118a7ce 100644 --- a/mcrouter/routes/test/RoutePolicyMapTest.cpp +++ b/mcrouter/routes/test/RoutePolicyMapTest.cpp @@ -48,23 +48,38 @@ struct MockPrefixSelectorRoute { } }; -RoutePolicyMap makeMap( - const std::vector& routes) { +struct MapsToTest { + explicit MapsToTest(const std::vector& clusters) + : m0(clusters, false), m1(clusters, true), m2(clusters) {} + + RoutePolicyMap m0; + RoutePolicyMap m1; + RoutePolicyMapV2 m2; +}; + +MapsToTest makeMap(const std::vector& routes) { std::vector clusters(routes.begin(), routes.end()); - return RoutePolicyMap(clusters); + return MapsToTest(clusters); } -std::vector routesFor( - const RoutePolicyMap& m, - folly::StringPiece key) { - std::vector res; - auto actual = m.getTargetsForKey(key); - res.reserve(actual.size()); - for (const auto& x : actual) { - EXPECT_NE(x, nullptr); - res.push_back(x != nullptr ? *x : -1); - } - return res; +std::vector routesFor(MapsToTest& maps, folly::StringPiece key) { + auto resForMap = [&](const auto& m) { + std::vector res; + auto actual = m.getTargetsForKey(key); + res.reserve(actual.size()); + for (const auto& x : actual) { + EXPECT_NE(x, nullptr); + res.push_back(x != nullptr ? *x : -1); + } + return res; + }; + + auto r0 = resForMap(maps.m0); + auto r1 = resForMap(maps.m1); + auto r2 = resForMap(maps.m2); + EXPECT_EQ(r0, r1) << key; + EXPECT_EQ(r0, r2) << key; + return r0; } TEST(RoutePolicyMapTest, NoPolicies) { @@ -117,5 +132,13 @@ TEST(RoutePolicyMapTest, TwoPolicies) { ASSERT_THAT(routesFor(m, "bc"), ::testing::ElementsAre(10, 8)); } +TEST(RoutePolicyMapTest, RepeatedWildcard) { + auto m = makeMap( + {{.wildcard = 10, .policies = {{"a", 1}}}, + {.wildcard = 10, .policies = {{"a", 2}}}}); + ASSERT_THAT(routesFor(m, "b"), ::testing::ElementsAre(10)); + ASSERT_THAT(routesFor(m, "a"), ::testing::ElementsAre(1, 2)); +} + } // namespace } // namespace facebook::memcache::mcrouter