Skip to content

Commit

Permalink
Allow using EagerShardSelectionRoute with LatestRoute
Browse files Browse the repository at this point in the history 
Summary: Add support for nesting LatestRoute in EagerShardSelectionRoute.

Reviewed By: glamtechie

Differential Revision: D6832737

fbshipit-source-id: 94f8f7d9b78d76d470e19923c2ccb23f16c94a7d
  • Loading branch information
@andreazevedo @facebook-github-bot
andreazevedo authored and facebook-github-bot committed Jan 29, 2018
1 parent a80ba5b commit 729f2f8
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 56 deletions.
15 changes: 12 additions & 3 deletions mcrouter/routes/LatestRoute.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, Facebook, Inc.
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
Expand Down Expand Up @@ -56,9 +56,9 @@ std::vector<std::shared_ptr<RouteHandleIf>> getTargets(
} // detail

template <class RouterInfo>
std::shared_ptr<typename RouterInfo::RouteHandleIf> createLatestRoute(
typename RouterInfo::RouteHandlePtr createLatestRoute(
const folly::dynamic& json,
std::vector<std::shared_ptr<typename RouterInfo::RouteHandleIf>> targets,
std::vector<typename RouterInfo::RouteHandlePtr> targets,
size_t threadId) {
size_t failoverCount = 5;
size_t failoverThreadId = 0;
Expand Down Expand Up @@ -117,6 +117,15 @@ std::shared_ptr<typename RouterInfo::RouteHandleIf> makeLatestRoute(
json, std::move(children), factory.getThreadId());
}

template <class RouterInfo>
typename RouterInfo::RouteHandlePtr createLatestRoute(
RouteHandleFactory<typename RouterInfo::RouteHandleIf>& factory,
const folly::dynamic& json,
std::vector<typename RouterInfo::RouteHandlePtr> children) {
return createLatestRoute<RouterInfo>(
json, std::move(children), factory.getThreadId());
}

} // mcrouter
} // memcache
} // facebook
14 changes: 11 additions & 3 deletions mcrouter/routes/LoadBalancerRoute.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ class LoadBalancerRoute {
};

template <class RouterInfo>
std::shared_ptr<typename RouterInfo::RouteHandleIf> createLoadBalancerRoute(
typename RouterInfo::RouteHandlePtr createLoadBalancerRoute(
const folly::dynamic& json,
std::vector<std::shared_ptr<typename RouterInfo::RouteHandleIf>> rh) {
std::vector<typename RouterInfo::RouteHandlePtr> rh) {
assert(json.isObject());

std::string salt;
Expand Down Expand Up @@ -188,7 +188,15 @@ std::shared_ptr<typename RouterInfo::RouteHandleIf> createLoadBalancerRoute(
}

template <class RouterInfo>
std::shared_ptr<typename RouterInfo::RouteHandleIf> makeLoadBalancerRoute(
typename RouterInfo::RouteHandlePtr createLoadBalancerRoute(
RouteHandleFactory<typename RouterInfo::RouteHandleIf>& /* factory */,
const folly::dynamic& json,
std::vector<typename RouterInfo::RouteHandlePtr> rh) {
return createLoadBalancerRoute<RouterInfo>(json, std::move(rh));
}

template <class RouterInfo>
typename RouterInfo::RouteHandlePtr makeLoadBalancerRoute(
RouteHandleFactory<typename RouterInfo::RouteHandleIf>& factory,
const folly::dynamic& json) {
checkLogic(json.isObject(), "LoadBalancerRoute is not an object");
Expand Down
35 changes: 20 additions & 15 deletions mcrouter/routes/ShardSelectionRouteFactory-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "mcrouter/lib/SelectionRouteFactory.h"
#include "mcrouter/lib/config/RouteHandleFactory.h"
#include "mcrouter/routes/ErrorRoute.h"
#include "mcrouter/routes/LatestRoute.h"
#include "mcrouter/routes/LoadBalancerRoute.h"

namespace facebook {
Expand Down Expand Up @@ -203,18 +204,7 @@ typename RouterInfo::RouteHandlePtr createEagerShardSelectionRoute(
return jChildType->stringPiece();
}();

using CreateRouteFunc = typename RouterInfo::RouteHandlePtr (*)(
const folly::dynamic&, std::vector<typename RouterInfo::RouteHandlePtr>);
CreateRouteFunc createRoute;
if (childrenType == "LoadBalancerRoute") {
createRoute = createLoadBalancerRoute<RouterInfo>;
} else {
throwLogic(
"EagerShardSelectionRoute: 'children_type' {} not supported",
childrenType);
}

const auto childrenSettings = [&json]() {
const auto& childrenSettings = [&json]() {
auto jSettings = json.get_ptr("children_settings");
checkLogic(
jSettings && jSettings->isObject(),
Expand All @@ -223,14 +213,29 @@ typename RouterInfo::RouteHandlePtr createEagerShardSelectionRoute(
return *jSettings;
}();

auto shardToDestinationIndexMap =
using CreateRouteFunc = typename RouterInfo::RouteHandlePtr (*)(
RouteHandleFactory<typename RouterInfo::RouteHandleIf> & factory,
const folly::dynamic&,
std::vector<typename RouterInfo::RouteHandlePtr>);
CreateRouteFunc createRouteFn;
if (childrenType == "LoadBalancerRoute") {
createRouteFn = createLoadBalancerRoute<RouterInfo>;
} else if (childrenType == "LatestRoute") {
createRouteFn = createLatestRoute<RouterInfo>;
} else {
throwLogic(
"EagerShardSelectionRoute: 'children_type' {} not supported",
childrenType);
}

MapType shardToDestinationIndexMap =
detail::prepareMap<MapType>(shardMap.size());
std::vector<typename RouterInfo::RouteHandlePtr> destinations;
std::for_each(shardMap.begin(), shardMap.end(), [&](auto& item) {
auto shardId = item.first;
auto childrenRouteHandles = std::move(item.second);
destinations.push_back(
createRoute(childrenSettings, std::move(childrenRouteHandles)));
destinations.push_back(createRouteFn(
factory, childrenSettings, std::move(childrenRouteHandles)));
shardToDestinationIndexMap[shardId] = destinations.size() - 1;
});

Expand Down
3 changes: 2 additions & 1 deletion mcrouter/routes/ShardSelectionRouteFactory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
namespace facebook {
namespace memcache {
namespace mcrouter {

/**
* Create a route handle for sharding requests to servers.
*
Expand Down Expand Up @@ -89,7 +90,7 @@ typename RouterInfo::RouteHandlePtr createShardSelectionRoute(
* "out_of_range": "ErrorRoute"
* }
*
* "Shards" can be an array of strings of comma-separated
* "shards" can be an array of strings of comma-separated
* shard ids, as stated in ShardSelectionRoute above
*
* NOTE:
Expand Down
193 changes: 159 additions & 34 deletions mcrouter/routes/test/EagerShardSelectionRouteTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*
*/
#include <unordered_set>
#include <vector>

#include <gtest/gtest.h>
Expand Down Expand Up @@ -154,7 +155,7 @@ TEST_F(EagerShardSelectionRouteTest, createMissingHost) {
}
}

TEST_F(EagerShardSelectionRouteTest, createMissingHostPools) {
TEST_F(EagerShardSelectionRouteTest, createEmptyServersAndShards) {
constexpr folly::StringPiece kSelectionRouteConfig = R"(
{
"children_type": "LoadBalancerRoute",
Expand Down Expand Up @@ -187,7 +188,8 @@ TEST_F(EagerShardSelectionRouteTest, createMissingHostPools) {
}
)";

// should configure fine because the first pool is still valid
// should configure fine because number of servers and number of shards
// matches in both cases.
try {
testCreate(kSelectionRouteConfig);
} catch (const std::exception& e) {
Expand All @@ -196,19 +198,21 @@ TEST_F(EagerShardSelectionRouteTest, createMissingHostPools) {
}
}

TEST_F(EagerShardSelectionRouteTest, routeArray) {
TEST_F(EagerShardSelectionRouteTest, traverseAndCheckChildrenIsLoadBalancer) {
constexpr folly::StringPiece kSelectionRouteConfig = R"(
{
"children_type": "LoadBalancerRoute",
"pools": [
{
"pool": [
"NullRoute",
"ErrorRoute"
],
"pool": {
"type": "Pool",
"name": "pool1",
"servers": [ "localhost:12345", "localhost:12325" ],
"protocol": "caret"
},
"shards": [
[1, 2, 3],
[2, 3, 6]
"1, 2, 3",
"3, 5, 6"
]
}
],
Expand All @@ -224,45 +228,166 @@ TEST_F(EagerShardSelectionRouteTest, routeArray) {
EXPECT_EQ("selection|basic-shard-selector", rh->routeName());

GoodbyeRequest req;
GoodbyeReply reply;

req.shardId() = 1;
size_t cnt = 0;
size_t iterations = 0;
RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf> t{
[&cnt](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
++cnt;
if (cnt == 1) {
[&iterations](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
++iterations;
if (iterations == 1) {
EXPECT_EQ("loadbalancer", r.routeName());
}
}};
rh->traverse(req, t);
EXPECT_GE(cnt, 1);
EXPECT_GE(iterations, 1);
}

req.shardId() = 1;
reply = rh->route(req);
EXPECT_EQ(mc_res_notfound, reply.result());
TEST_F(EagerShardSelectionRouteTest, traverseAndCheckChildrenIsFailover) {
constexpr folly::StringPiece kSelectionRouteConfig = R"(
{
"children_type": "LatestRoute",
"pools": [
{
"pool": {
"type": "Pool",
"name": "pool1",
"servers": [ "localhost:12301", "localhost:35601" ],
"protocol": "caret"
},
"shards": [
"1, 2, 3",
"3, 5"
]
},
{
"pool": {
"type": "Pool",
"name": "pool2",
"servers": [ "localhost:12302", "localhost:35602" ],
"protocol": "caret"
},
"shards": [
"1, 2, 3",
"3, 5, 6"
]
},
{
"pool": {
"type": "Pool",
"name": "pool3",
"servers": [ "localhost:12303", "localhost:35603"],
"protocol": "caret"
},
"shards": [
"1, 2, 3",
"3"
]
}
],
"children_settings" : {
"failover_count": 2
}
}
)";

req.shardId() = 2;
reply = rh->route(req);
EXPECT_EQ(mc_res_notfound, reply.result());
auto rh = getEagerShardSelectionRoute(kSelectionRouteConfig);
ASSERT_TRUE(rh);
EXPECT_EQ("selection|basic-shard-selector", rh->routeName());

GoodbyeRequest req;

// Shards 1 and 2 are served by 3 servers, with name starting
// with "localhost:123"
for (auto shardId : {1, 2}) {
req.shardId() = shardId;
size_t iterations = 0;
std::unordered_set<std::string> children;
RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf> t{
[&iterations,
&children](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
EXPECT_EQ(children.end(), children.find(r.routeName()));
children.emplace(r.routeName());
if (++iterations == 1) {
EXPECT_EQ("failover", r.routeName());
} else if (iterations > 1) {
EXPECT_TRUE(r.routeName().find("host|") != std::string::npos);
EXPECT_TRUE(
r.routeName().find("localhost:123") != std::string::npos);
}
}};
rh->traverse(req, t);
// We should iterate 3 times, once for FailoveRoute, and 2 for hosts,
// as failover_count is 2.
EXPECT_EQ(iterations, 3);
}

// load hasnt changed much, still NullRoute
req.shardId() = 2;
reply = rh->route(req);
EXPECT_EQ(mc_res_notfound, reply.result());
// Shard 3 is served by all 6 servers.
req.shardId() = 5;
size_t iterations = 0;
std::unordered_set<std::string> children;
RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf> t{
[&iterations, &children](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
EXPECT_EQ(children.end(), children.find(r.routeName()));
children.emplace(r.routeName());
if (++iterations == 1) {
EXPECT_EQ("failover", r.routeName());
} else if (iterations > 1) {
EXPECT_TRUE(r.routeName().find("host|") != std::string::npos);
EXPECT_TRUE(
(r.routeName().find("localhost:123") != std::string::npos) ||
(r.routeName().find("localhost:356") != std::string::npos));
}
}};
rh->traverse(req, t);
// We should iterate 3 times, once for FailoveRoute, and 2 for hosts,
// as failover_count is 2.
EXPECT_EQ(iterations, 3);

req.shardId() = 3;
reply = rh->route(req);
EXPECT_EQ(mc_res_notfound, reply.result());
// There is no shard 4.
req.shardId() = 4;
iterations = 0;
t = RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf>{
[&iterations](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
++iterations;
EXPECT_TRUE(r.routeName().find("error|") != std::string::npos);
}};
rh->traverse(req, t);
// We should iterate just once, for ErrorRoute
EXPECT_EQ(iterations, 1);

// load hasnt changed much, still ErrorRoute
req.shardId() = 3;
reply = rh->route(req);
EXPECT_EQ(mc_res_notfound, reply.result());
// Shard 5 is served by 2 servers, with name starting with "localhost:356"
req.shardId() = 5;
iterations = 0;
children.clear();
t = RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf>{
[&iterations, &children](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
EXPECT_EQ(children.end(), children.find(r.routeName()));
children.emplace(r.routeName());
if (++iterations == 1) {
EXPECT_EQ("failover", r.routeName());
} else if (iterations > 1) {
EXPECT_TRUE(r.routeName().find("host|") != std::string::npos);
EXPECT_TRUE(r.routeName().find("localhost:356") != std::string::npos);
}
}};
rh->traverse(req, t);
// We should iterate 3 times, once for FailoveRoute, and 2 for hosts,
// as failover_count is 2.
EXPECT_EQ(iterations, 3);

// Shard 6 is served only by server "localhost:35602"
req.shardId() = 6;
reply = rh->route(req);
EXPECT_EQ(mc_res_local_error, reply.result());
iterations = 0;
t = RouteHandleTraverser<HelloGoodbyeRouterInfo::RouteHandleIf>{
[&iterations](const HelloGoodbyeRouterInfo::RouteHandleIf& r) {
++iterations;
EXPECT_TRUE(r.routeName().find("host|") != std::string::npos);
EXPECT_TRUE(r.routeName().find("localhost:35602") != std::string::npos);
}};
rh->traverse(req, t);
// We should iterate just once, for host "localhost:35602"
// (FailoverRoute is optimized away).
EXPECT_EQ(iterations, 1);
}

} // namespace mcrouter
Expand Down

0 comments on commit 729f2f8

Please sign in to comment.