Skip to content

Commit

Permalink
RoutingGroups: generate explicit false specializations; remove default
Browse files Browse the repository at this point in the history
Summary:
Two problems with the current "default false" approach:
1) If the specialization file is not included in a translation unit, the request will incorrectly default to false. In this new approach this will be a compile error - the correct routing groups file must be included.
2) Default false means some bugs snuck in (using GetLike with something that's not a request type, e.g. a reply or void by mistake) - see stacked diffs for fixes.

Changes in this diff:
1) New special routing group "no_group" to list requests for which all *Like traits should be false. The invariant is that every request must belong to one and only one group.
2) Removes default false, and introduces explicit true/false specializations for all requests.

Reviewed By: disylh, stuclar, antonf

Differential Revision: D62321258

fbshipit-source-id: b27f1fe3c61513b31578ba48cf16ed8a2df28138
  • Loading branch information
Anton Likhtarov authored and facebook-github-bot committed Oct 3, 2024
1 parent 914a238 commit 2bf49f0
Show file tree
Hide file tree
Showing 21 changed files with 765 additions and 12 deletions.
16 changes: 4 additions & 12 deletions mcrouter/lib/carbon/RoutingGroups.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ constexpr auto kUpdateKey = folly::makeFixedString("update_like");
* Request is get-like.
*/
template <typename Request = void>
struct GetLike {
static const bool value = false;
};
struct GetLike;

template <typename Request = void>
using GetLikeT = typename GetLike<Request>::Type;
Expand All @@ -57,9 +55,7 @@ using GetLikeT = typename GetLike<Request>::Type;
* Request is update-like.
*/
template <typename Request = void>
struct UpdateLike {
static const bool value = false;
};
struct UpdateLike;

template <typename Request = void>
using UpdateLikeT = typename UpdateLike<Request>::Type;
Expand All @@ -74,9 +70,7 @@ using UpdateLikeT = typename UpdateLike<Request>::Type;
* Request is delete-like.
*/
template <typename Request = void>
struct DeleteLike {
static const bool value = false;
};
struct DeleteLike;

template <typename Request = void>
using DeleteLikeT = typename DeleteLike<Request>::Type;
Expand All @@ -92,9 +86,7 @@ using DeleteLikeT = typename DeleteLike<Request>::Type;
* Request is arithmetic-like.
*/
template <typename Request = void>
struct ArithmeticLike {
static const bool value = false;
};
struct ArithmeticLike;

template <typename Request = void>
using ArithmeticLikeT = typename ArithmeticLike<Request>::Type;
Expand Down
6 changes: 6 additions & 0 deletions mcrouter/lib/carbon/example/HelloGoodbye.idl
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ service {
DuplicateRoute @ "mcrouter/lib/carbon/example/DuplicateRoute.h",
CarbonLookasideRoute @ "mcrouter/lib/carbon/example/CarbonLookasideRoute.h"
];
routing_groups: {
no_group: [
HelloRequest,
GoodbyeRequest,
]
};
gen_client_tool: true;
compression_by_type: true;
};
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/example/gen/HelloGoodbyeRouterInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "mcrouter/lib/carbon/example/gen/HelloGoodbyeRouteHandleIf.h"
#include "mcrouter/lib/carbon/example/gen/HelloGoodbyeRouterStats.h"
#include "mcrouter/lib/carbon/example/gen/HelloGoodbyeRoutingGroups.h"

// Forward declarations
namespace folly {
Expand Down
66 changes: 66 additions & 0 deletions mcrouter/lib/carbon/example/gen/HelloGoodbyeRoutingGroups.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
*/

/*
* THIS FILE IS AUTOGENERATED. DO NOT MODIFY IT; ALL CHANGES WILL BE LOST IN
* VAIN.
*
* @generated
*/
#pragma once

#include <mcrouter/lib/carbon/RoutingGroups.h>

#include "mcrouter/lib/carbon/example/gen/HelloGoodbyeMessages.h"

namespace carbon {

// ArithmeticLike
template <>
struct ArithmeticLike<hellogoodbye::GoodbyeRequest> {
static const bool value = false;
};

template <>
struct ArithmeticLike<hellogoodbye::HelloRequest> {
static const bool value = false;
};

// DeleteLike
template <>
struct DeleteLike<hellogoodbye::GoodbyeRequest> {
static const bool value = false;
};

template <>
struct DeleteLike<hellogoodbye::HelloRequest> {
static const bool value = false;
};

// GetLike
template <>
struct GetLike<hellogoodbye::GoodbyeRequest> {
static const bool value = false;
};

template <>
struct GetLike<hellogoodbye::HelloRequest> {
static const bool value = false;
};

// UpdateLike
template <>
struct UpdateLike<hellogoodbye::GoodbyeRequest> {
static const bool value = false;
};

template <>
struct UpdateLike<hellogoodbye::HelloRequest> {
static const bool value = false;
};
} // namespace carbon
6 changes: 6 additions & 0 deletions mcrouter/lib/carbon/test/A.idl
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,10 @@ reply TestAReply {

service {
enable_shutdown: true;

routing_groups: {
no_group: [
TestARequest,
]
};
};
6 changes: 6 additions & 0 deletions mcrouter/lib/carbon/test/B.idl
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,10 @@ reply TestBReply {

service {
enable_shutdown: true;

routing_groups: {
no_group: [
TestBRequest,
]
};
};
7 changes: 7 additions & 0 deletions mcrouter/lib/carbon/test/CarbonTest.idl
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,11 @@ struct StructWithOptionalEnumInt8 {

service {
enable_shutdown: true;

routing_groups: {
no_group: [
TestRequest,
TestRequestStringKey,
]
};
};
8 changes: 8 additions & 0 deletions mcrouter/lib/carbon/test/CarbonThriftTest.idl
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,12 @@ service {
enable_shutdown: true;
gen_client_tool: false;
thrift_tm_thread: true;

routing_groups: {
no_group: [
ThriftTestRequest,
DummyThriftRequest,
CustomRequest,
]
};
};
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/JsonClientTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "mcrouter/lib/carbon/CmdLineClient.h"
#include "mcrouter/lib/carbon/JsonClient.h"
#include "mcrouter/lib/carbon/test/gen/CarbonTest.h"
#include "mcrouter/lib/carbon/test/gen/CarbonTestRoutingGroups.h"
#include "mcrouter/lib/mc/msg.h"
#include "mcrouter/lib/network/AsyncMcServer.h"
#include "mcrouter/lib/network/AsyncMcServerWorker.h"
Expand Down
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/gen/ARouteHandleIf.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <mcrouter/lib/RouteHandleTraverser.h>

#include "mcrouter/lib/carbon/test/gen/AMessages.h"
#include "mcrouter/lib/carbon/test/gen/BRoutingGroups.h"

namespace carbon {
namespace test {
Expand Down
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/gen/ARouterInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "mcrouter/lib/carbon/test/gen/ARouteHandleIf.h"
#include "mcrouter/lib/carbon/test/gen/ARouterStats.h"
#include "mcrouter/lib/carbon/test/gen/ARoutingGroups.h"

// Forward declarations
namespace folly {
Expand Down
46 changes: 46 additions & 0 deletions mcrouter/lib/carbon/test/gen/ARoutingGroups.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
*/

/*
* THIS FILE IS AUTOGENERATED. DO NOT MODIFY IT; ALL CHANGES WILL BE LOST IN
* VAIN.
*
* @generated
*/
#pragma once

#include <mcrouter/lib/carbon/RoutingGroups.h>

#include "mcrouter/lib/carbon/test/gen/AMessages.h"

namespace carbon {

// ArithmeticLike
template <>
struct ArithmeticLike<carbon::test::A::TestARequest> {
static const bool value = false;
};

// DeleteLike
template <>
struct DeleteLike<carbon::test::A::TestARequest> {
static const bool value = false;
};

// GetLike
template <>
struct GetLike<carbon::test::A::TestARequest> {
static const bool value = false;
};

// UpdateLike
template <>
struct UpdateLike<carbon::test::A::TestARequest> {
static const bool value = false;
};
} // namespace carbon
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/gen/BRouterInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "mcrouter/lib/carbon/test/gen/BRouteHandleIf.h"
#include "mcrouter/lib/carbon/test/gen/BRouterStats.h"
#include "mcrouter/lib/carbon/test/gen/BRoutingGroups.h"

// Forward declarations
namespace folly {
Expand Down
46 changes: 46 additions & 0 deletions mcrouter/lib/carbon/test/gen/BRoutingGroups.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
*/

/*
* THIS FILE IS AUTOGENERATED. DO NOT MODIFY IT; ALL CHANGES WILL BE LOST IN
* VAIN.
*
* @generated
*/
#pragma once

#include <mcrouter/lib/carbon/RoutingGroups.h>

#include "mcrouter/lib/carbon/test/gen/BMessages.h"

namespace carbon {

// ArithmeticLike
template <>
struct ArithmeticLike<carbon::test::B::TestBRequest> {
static const bool value = false;
};

// DeleteLike
template <>
struct DeleteLike<carbon::test::B::TestBRequest> {
static const bool value = false;
};

// GetLike
template <>
struct GetLike<carbon::test::B::TestBRequest> {
static const bool value = false;
};

// UpdateLike
template <>
struct UpdateLike<carbon::test::B::TestBRequest> {
static const bool value = false;
};
} // namespace carbon
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/gen/CarbonTestRouterInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "mcrouter/lib/carbon/test/gen/CarbonTestRouteHandleIf.h"
#include "mcrouter/lib/carbon/test/gen/CarbonTestRouterStats.h"
#include "mcrouter/lib/carbon/test/gen/CarbonTestRoutingGroups.h"

// Forward declarations
namespace folly {
Expand Down
66 changes: 66 additions & 0 deletions mcrouter/lib/carbon/test/gen/CarbonTestRoutingGroups.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
*/

/*
* THIS FILE IS AUTOGENERATED. DO NOT MODIFY IT; ALL CHANGES WILL BE LOST IN
* VAIN.
*
* @generated
*/
#pragma once

#include <mcrouter/lib/carbon/RoutingGroups.h>

#include "mcrouter/lib/carbon/test/gen/CarbonTestMessages.h"

namespace carbon {

// ArithmeticLike
template <>
struct ArithmeticLike<carbon::test::TestRequest> {
static const bool value = false;
};

template <>
struct ArithmeticLike<carbon::test::TestRequestStringKey> {
static const bool value = false;
};

// DeleteLike
template <>
struct DeleteLike<carbon::test::TestRequest> {
static const bool value = false;
};

template <>
struct DeleteLike<carbon::test::TestRequestStringKey> {
static const bool value = false;
};

// GetLike
template <>
struct GetLike<carbon::test::TestRequest> {
static const bool value = false;
};

template <>
struct GetLike<carbon::test::TestRequestStringKey> {
static const bool value = false;
};

// UpdateLike
template <>
struct UpdateLike<carbon::test::TestRequest> {
static const bool value = false;
};

template <>
struct UpdateLike<carbon::test::TestRequestStringKey> {
static const bool value = false;
};
} // namespace carbon
1 change: 1 addition & 0 deletions mcrouter/lib/carbon/test/gen/CarbonThriftTestRouterInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "mcrouter/lib/carbon/test/gen/CarbonThriftTestRouteHandleIf.h"
#include "mcrouter/lib/carbon/test/gen/CarbonThriftTestRouterStats.h"
#include "mcrouter/lib/carbon/test/gen/CarbonThriftTestRoutingGroups.h"

// Forward declarations
namespace folly {
Expand Down
Loading

0 comments on commit 2bf49f0

Please sign in to comment.