Skip to content

Commit be182c2

Browse files
vj-menonmeta-codesync[bot]
authored andcommitted
Support dimension-ranked destination exploration in ScopeItemList
Summary: `MoveToScopeItemsSpec` restricts an object's destinations to a set of scope items, but always explores every scope item in that set. In this diff we add an optional `priorityOrdering` to `ScopeItemList`, a `ScopeItemPriorityOrdering` union whose only arm today is `DimensionBasedOrdering`. `DimensionBasedOrdering` let us specify an dynamic object dimension name. Given this, when evaluating a move w.r.t .an object, we only explore scope items that are ranked better than the object's current scope item as per the dimension. This is useful in scenarios like the following: an object which represents traffic from region `R` is currently in `S` and we have a goal to reduce the latency w.r.t. to this object. IN such scenario we only want to explore destinations that are strictly better than `S` for that object. The dimension must be dynamic (scope-dependent) on the list's scope; a static one is rejected, since its value doesn't vary by scope item. `ProblemChecker` catches a bad dimension early. `SwapMoveType` rejects `priorityOrdering`, since a swap has no single moving object to rank against. If the object isn't in the scope, all candidates are explored. **The field is optional; leaving it unset preserves today's behavior exactly---no behavior change unless a client opts in. We plan to use this for GSP latency optimization stage** Reviewed By: polmauri Differential Revision: D113100882 fbshipit-source-id: 104f835b16888480370e2c14913358a94ca3a25e
1 parent 67cf5ff commit be182c2

10 files changed

Lines changed: 428 additions & 39 deletions

algopt/rebalancer/interface/ProblemChecker.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1979,6 +1979,29 @@ void ProblemChecker::check(
19791979
checkScopeItemExists(*scopeItemList.scopeName(), scopeItem);
19801980
}
19811981
}
1982+
if (scopeItemList.filter().has_value()) {
1983+
const auto& filter = *scopeItemList.filter();
1984+
switch (filter.getType()) {
1985+
case interface::ScopeItemFilter::Type::objectDimensionBased: {
1986+
const auto& dimensionName =
1987+
*filter.get_objectDimensionBased().dimensionName();
1988+
checkDimensionExists(dimensionName);
1989+
const auto& scopeName = *scopeItemList.scopeName();
1990+
const auto* dimensionScope =
1991+
folly::get_ptr(dynamicObjectDimensionToScope_, dimensionName);
1992+
if (dimensionScope == nullptr || *dimensionScope != scopeName) {
1993+
throw std::runtime_error(
1994+
fmt::format(
1995+
"ObjectDimensionBasedFilter dimension '{}' is expected to be dynamic on scope '{}'",
1996+
dimensionName,
1997+
scopeName));
1998+
}
1999+
break;
2000+
}
2001+
case interface::ScopeItemFilter::Type::__EMPTY__:
2002+
throw std::runtime_error("ScopeItemFilter is not set");
2003+
}
2004+
}
19822005
}
19832006

19842007
void ProblemChecker::check(

algopt/rebalancer/interface/ProblemChecker.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ class ProblemChecker {
375375
algopt::SetImpl<std::string> constraintNames_;
376376
algopt::MapImpl<std::string, algopt::SetImpl<std::string>>
377377
dimensionToEntityTypes;
378+
algopt::MapImpl<std::string, std::string> dynamicObjectDimensionToScope_;
378379
algopt::MapImpl<std::string, EntityType> globalNameToType;
379380
algopt::SetImpl<std::string> routingConfigNames_;
380381
algopt::SetImpl<std::string> destinationsToExploreOptionNames_;
@@ -400,6 +401,7 @@ void ProblemChecker::addDynamicObjectDimension(
400401
}
401402
}
402403
addObjectDimension(dimensionName, ScopeItemToObjectToValue());
404+
dynamicObjectDimensionToScope_.emplace(dimensionName, scope);
403405
}
404406

405407
template <typename ScopeItemToGroupToValue>
@@ -419,6 +421,7 @@ void ProblemChecker::addDynamicObjectDimension(
419421
}
420422
}
421423
addObjectDimension(dimensionName, ScopeItemToGroupToValue());
424+
dynamicObjectDimensionToScope_.emplace(dimensionName, scope);
422425
}
423426

424427
template <typename ObjectToGroup>

algopt/rebalancer/interface/tests/ProblemSolverChecksTest.cpp

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,19 @@ getDefaultGroupCapacitySpec() {
7373
return spec;
7474
}
7575

76+
static ScopeItemList makeDimensionFilteredScopeItemList(
77+
const std::string& scopeName,
78+
const std::string& dimensionName) {
79+
ObjectDimensionBasedFilter dimensionFilter;
80+
dimensionFilter.dimensionName() = dimensionName;
81+
ScopeItemFilter scopeItemFilter;
82+
scopeItemFilter.objectDimensionBased() = dimensionFilter;
83+
ScopeItemList scopeItemList;
84+
scopeItemList.scopeName() = scopeName;
85+
scopeItemList.filter() = scopeItemFilter;
86+
return scopeItemList;
87+
}
88+
7689
TEST_P(ProblemSolverChecksTest, SetObjectNameTwice) {
7790
auto solver =
7891
initializeTestProblemSolver({.executorThreadCount = GetParam()});
@@ -2753,6 +2766,39 @@ TEST_P(ProblemSolverChecksTest, MoveToScopeItemSpecsTest) {
27532766
"name2", destinationsToExploreOption),
27542767
"unknown group j3 in partition job");
27552768
}
2769+
2770+
{
2771+
// filter must reference an existing dimension
2772+
MoveToScopeItemsSpec moveToScopeItemsSpec;
2773+
moveToScopeItemsSpec.defaultScopeItems() =
2774+
makeDimensionFilteredScopeItemList("rack", "network");
2775+
2776+
DestinationsToExploreOptions destinationsToExploreOption;
2777+
destinationsToExploreOption.set_moveToScopeItems(moveToScopeItemsSpec);
2778+
2779+
REBALANCER_EXPECT_RUNTIME_ERROR(
2780+
solver->addDestinationsToExploreOptions(
2781+
"filterBadDimension", destinationsToExploreOption),
2782+
"unknown dimension network");
2783+
}
2784+
2785+
{
2786+
// filter requires a dynamic dimension; a static one is rejected.
2787+
solver->addObjectDimension(
2788+
"size", std::map<std::string, double>{{"s1", 1.0}, {"s2", 2.0}});
2789+
2790+
MoveToScopeItemsSpec moveToScopeItemsSpec;
2791+
moveToScopeItemsSpec.defaultScopeItems() =
2792+
makeDimensionFilteredScopeItemList("rack", "size");
2793+
2794+
DestinationsToExploreOptions destinationsToExploreOption;
2795+
destinationsToExploreOption.set_moveToScopeItems(moveToScopeItemsSpec);
2796+
2797+
REBALANCER_EXPECT_RUNTIME_ERROR(
2798+
solver->addDestinationsToExploreOptions(
2799+
"filterStaticDimension", destinationsToExploreOption),
2800+
"ObjectDimensionBasedFilter dimension 'size' is expected to be dynamic on scope 'rack'");
2801+
}
27562802
}
27572803

27582804
TEST_P(ProblemSolverChecksTest, SampleSizeTest) {
@@ -3233,7 +3279,7 @@ TEST_P(ProblemSolverChecksTest, GroupMoveWithHintStrategiesMoveType) {
32333279
}
32343280
{
32353281
// no hint map
3236-
MoveStrategies groupToMoveStrategy2;
3282+
const MoveStrategies groupToMoveStrategy2;
32373283
auto spec = makeGroupMoveWithStrategies(
32383284
"primaryPartition", "secondaryPartition", groupToMoveStrategy2);
32393285

algopt/rebalancer/interface/thrift/SolverSpecs.thrift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,34 @@ struct MoveToCurrentScopeItemSpec {
414414
1: string scopeNameForExploringMovesToCurrentScopeItem;
415415
}
416416

417+
// Filters candidate scope items by a dimension of the moving object, so a move
418+
// type explores only scope items that are better than the current one, plus
419+
// ties when includeEqual is set. See ScopeItemList.filter.
420+
struct ObjectDimensionBasedFilter {
421+
// The object dimension whose value is used to compare candidate scope items. It
422+
// must be a dynamic object dimension defined on the enclosing ScopeItemList's scope (scopeName)
423+
1: string dimensionName;
424+
// Whether a lower value should be considered better (e.g. latency). If false, higher is better.
425+
2: bool preferLower = true;
426+
// Also keep scope items that tie the current one, not just better ones.
427+
3: bool includeEqual = false;
428+
}
429+
430+
union ScopeItemFilter {
431+
1: ObjectDimensionBasedFilter objectDimensionBased;
432+
}
433+
417434
@thrift.ReserveIds{ids = [3]}
418435
struct ScopeItemList {
419436
1: string scopeName;
420437
// if scopeItems are not explicitly listed, all scopeItems in the specified scopeName are taken
421438
2: optional list<string> scopeItems;
439+
// If set, restrict exploration to candidate scope items that the selected
440+
// ScopeItemFilter considers better than the moving object's current scope
441+
// item, plus ties when the filter includes equal values. If unset, or the
442+
// object has no scope item in this scope, all candidates are explored. See
443+
// ScopeItemFilter for the strategies.
444+
4: optional ScopeItemFilter filter;
422445
}
423446

424447
struct GroupToScopeItemList {

algopt/rebalancer/solver/moves/DestinationsToExploreGenerator.cpp

Lines changed: 110 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,55 @@
1616

1717
namespace facebook::rebalancer {
1818

19+
namespace {
20+
// Keeps only the scope items that compare better than the moving object's
21+
// current one on a chosen dimension. See ScopeItemList.filter.
22+
class ObjectDimensionBasedFilter {
23+
public:
24+
ObjectDimensionBasedFilter(
25+
const interface::ObjectDimensionBasedFilter& filter,
26+
const entities::ObjectScalarDimension& dimension,
27+
entities::ScopeId scopeId,
28+
entities::ObjectId object,
29+
std::optional<entities::ScopeItemId> currentScopeItem)
30+
: filter_{filter},
31+
dimension_{dimension},
32+
scopeId_{scopeId},
33+
object_{object} {
34+
if (currentScopeItem.has_value()) {
35+
currentValue_ = valueAt(*currentScopeItem);
36+
}
37+
}
38+
39+
bool includes(entities::ScopeItemId scopeItemId) const {
40+
// An object outside the scope has no current value to beat, so keep all.
41+
if (!currentValue_.has_value()) {
42+
return true;
43+
}
44+
const auto candidate = valueAt(scopeItemId);
45+
if (candidate == *currentValue_) {
46+
return *filter_.includeEqual();
47+
}
48+
return *filter_.preferLower() ? candidate < *currentValue_
49+
: candidate > *currentValue_;
50+
}
51+
52+
private:
53+
double valueAt(entities::ScopeItemId scopeItemId) const {
54+
return dimension_.getValueSafe(
55+
object_,
56+
entities::ScopeScopeItemPair{
57+
.scopeId = scopeId_, .scopeItemId = scopeItemId});
58+
}
59+
60+
const interface::ObjectDimensionBasedFilter& filter_;
61+
const entities::ObjectScalarDimension& dimension_;
62+
entities::ScopeId scopeId_;
63+
entities::ObjectId object_;
64+
std::optional<double> currentValue_;
65+
};
66+
} // namespace
67+
1968
DestinationsToExploreGenerator::DestinationsToExploreGenerator(
2069
const PackerSet<entities::ContainerId>& nonAcceptingContainers,
2170
const entities::Universe& universe)
@@ -36,7 +85,8 @@ DestinationsToExploreGenerator::getAcceptingDestinations(
3685
// in the given scope
3786
interface::ScopeItemList scopeItemList;
3887
scopeItemList.scopeName() = scopeName;
39-
return getAcceptingContainersList(scopeItemList);
88+
return getAcceptingContainersList(
89+
scopeItemList, hotContainerId, /*hotObject=*/std::nullopt);
4090
}
4191

4292
return ReferenceList<const std::vector<entities::ContainerId>>{
@@ -46,14 +96,16 @@ DestinationsToExploreGenerator::getAcceptingDestinations(
4696
ReferenceList<const std::vector<entities::ContainerId>>
4797
DestinationsToExploreGenerator::getAcceptingDestinations(
4898
const interface::MoveToScopeItemsSpec& moveToScopeItems,
99+
const entities::ContainerId hotContainer,
49100
const entities::ObjectId hotObject) {
50101
auto& objectToScopeItems = *moveToScopeItems.objectToScopeItems();
51102
auto& hotObjectName = universe_.getEntityName(hotObject);
52103

53104
// if an object is specified in objectToScopeItems, use that.
54105
auto scopeItemListPtr = folly::get_ptr(objectToScopeItems, hotObjectName);
55106
if (scopeItemListPtr) {
56-
return getAcceptingContainersList(*scopeItemListPtr);
107+
return getAcceptingContainersList(
108+
*scopeItemListPtr, hotContainer, hotObject);
57109
}
58110

59111
// if a group is specified in groupToScopeItems and hotObject belongs to that
@@ -81,43 +133,88 @@ DestinationsToExploreGenerator::getAcceptingDestinations(
81133
groupToScopeItem,
82134
universe_.getEntityName(onlyGroupId),
83135
defaultScopeItems);
84-
return getAcceptingContainersList(scopeItemList);
136+
return getAcceptingContainersList(scopeItemList, hotContainer, hotObject);
85137
}
86138
}
87139

88140
// use defaultScopeItems if there is no specialization for hotObject
89-
return getAcceptingContainersList(defaultScopeItems);
141+
return getAcceptingContainersList(defaultScopeItems, hotContainer, hotObject);
90142
}
91143

92144
ReferenceList<const std::vector<entities::ContainerId>>
93145
DestinationsToExploreGenerator::getAcceptingDestinations(
94-
const interface::MoveToScopeItemsSpec& moveToScopeItems) {
146+
const interface::MoveToScopeItemsSpec& moveToScopeItems,
147+
const entities::ContainerId hotContainer) {
95148
if (!moveToScopeItems.objectToScopeItems()->empty()) {
96149
throw std::runtime_error(
97150
"this function requires that objectToScopeItems is empty");
98151
}
99-
return getAcceptingContainersList(*moveToScopeItems.defaultScopeItems());
152+
return getAcceptingContainersList(
153+
*moveToScopeItems.defaultScopeItems(),
154+
hotContainer,
155+
/*hotObject=*/std::nullopt);
100156
}
101157

102158
ReferenceList<const std::vector<entities::ContainerId>>
103159
DestinationsToExploreGenerator::getAcceptingContainersList(
104-
const interface::ScopeItemList& scopeItemList) {
105-
ReferenceList<const std::vector<entities::ContainerId>> destinations;
160+
const interface::ScopeItemList& scopeItemList,
161+
const entities::ContainerId hotContainer,
162+
std::optional<entities::ObjectId> hotObject) {
106163
auto& scopeName = *scopeItemList.scopeName();
107164
auto scopeId = universe_.getScopeId(scopeName);
108165
auto& scope = universe_.getScope(scopeId);
109-
if (scopeItemList.scopeItems().has_value()) {
110-
auto& scopeItemNames = scopeItemList.scopeItems().value();
111-
for (auto& scopeItemName : scopeItemNames) {
112-
auto scopeItemId = universe_.getScopeItemId(scopeId, scopeItemName);
166+
167+
std::optional<ObjectDimensionBasedFilter> objectDimensionFilter;
168+
if (scopeItemList.filter().has_value()) {
169+
if (!hotObject.has_value()) {
170+
throw std::runtime_error(
171+
"ScopeItemList.filter requires the moving object, but this move type does not provide one");
172+
}
173+
const auto& filter = *scopeItemList.filter();
174+
switch (filter.getType()) {
175+
case interface::ScopeItemFilter::Type::objectDimensionBased: {
176+
const auto& filterSpec = filter.get_objectDimensionBased();
177+
const auto& dimension = universe_.getObjects()
178+
.getDimension(universe_.getDimensionId(
179+
*filterSpec.dimensionName()))
180+
.only();
181+
if (!dimension.isDynamic() || dimension.getScopeId() != scopeId)
182+
[[unlikely]] {
183+
throw std::runtime_error(
184+
fmt::format(
185+
"ObjectDimensionBasedFilter dimension '{}' is expected to be dynamic on scope '{}'",
186+
*filterSpec.dimensionName(),
187+
scopeName));
188+
}
189+
const auto hotContainerScopeItem = scope.getScopeItemId(hotContainer);
190+
objectDimensionFilter.emplace(
191+
filterSpec, dimension, scopeId, *hotObject, hotContainerScopeItem);
192+
break;
193+
}
194+
case interface::ScopeItemFilter::Type::__EMPTY__:
195+
throw std::runtime_error("ScopeItemFilter is not set");
196+
}
197+
}
198+
199+
ReferenceList<const std::vector<entities::ContainerId>> destinations;
200+
const auto addToDestinations = [&](entities::ScopeItemId scopeItemId) {
201+
if (!objectDimensionFilter.has_value() ||
202+
objectDimensionFilter->includes(scopeItemId)) {
113203
destinations.emplace_back(getAcceptingContainers(scopeItemId, scope));
114204
}
205+
};
206+
207+
if (scopeItemList.scopeItems().has_value()) {
208+
const auto& scopeItemNames = scopeItemList.scopeItems().value();
209+
for (const auto& scopeItemName : scopeItemNames) {
210+
addToDestinations(universe_.getScopeItemId(scopeId, scopeItemName));
211+
}
115212
} else {
116213
// if scopeItems are not explicitly listed, all scopeItems in the specified
117214
// scopeName are taken
118215
auto& scopeItemIds = scope.getScopeItemIds();
119216
for (auto scopeItemId : scopeItemIds) {
120-
destinations.emplace_back(getAcceptingContainers(scopeItemId, scope));
217+
addToDestinations(scopeItemId);
121218
}
122219
}
123220
return destinations;

algopt/rebalancer/solver/moves/DestinationsToExploreGenerator.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,20 @@ class DestinationsToExploreGenerator {
4040
ReferenceList<const std::vector<entities::ContainerId>>
4141
getAcceptingDestinations(
4242
const interface::MoveToScopeItemsSpec& moveToScopeItems,
43+
const entities::ContainerId hotContainer,
4344
const entities::ObjectId hotObject);
4445

4546
ReferenceList<const std::vector<entities::ContainerId>>
4647
getAcceptingDestinations(
47-
const interface::MoveToScopeItemsSpec& moveToScopeItems);
48+
const interface::MoveToScopeItemsSpec& moveToScopeItems,
49+
const entities::ContainerId hotContainer);
4850

4951
private:
5052
ReferenceList<const std::vector<entities::ContainerId>>
51-
getAcceptingContainersList(const interface::ScopeItemList& scopeItemList);
53+
getAcceptingContainersList(
54+
const interface::ScopeItemList& scopeItemList,
55+
const entities::ContainerId hotContainer,
56+
std::optional<entities::ObjectId> hotObject);
5257

5358
const std::vector<entities::ContainerId>& getAcceptingContainers(
5459
entities::ScopeItemId scopeItemId,

algopt/rebalancer/solver/moves/GroupMoveWithHintStrategiesMoveType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ std::vector<MoveSet> GroupMoveWithHintStrategiesMoveType::generateAllMoveSets(
472472

473473
auto acceptingContainersPerScopeItem =
474474
problem.getDestinationsGenerator().getAcceptingDestinations(
475-
*hintOptions.moveToScopeItems(), hotObjectId);
475+
*hintOptions.moveToScopeItems(), hotContainer, hotObjectId);
476476

477477
std::vector<MoveSet> newMoveSets;
478478
if (hintOptions.tertiaryPartition().has_value() &&

algopt/rebalancer/solver/moves/MoveType.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ MoveType::getDestinationsToExplore(
118118

119119
case interface::DestinationsToExploreOptions::Type::moveToScopeItems: {
120120
return problem.getDestinationsGenerator().getAcceptingDestinations(
121-
destinationsToExplore.get_moveToScopeItems(), hotObjectId);
121+
destinationsToExplore.get_moveToScopeItems(),
122+
hotContainerId,
123+
hotObjectId);
122124
}
123125

124126
case interface::DestinationsToExploreOptions::Type::

0 commit comments

Comments
 (0)