Skip to content

Commit 469e9b2

Browse files
vj-menonmeta-codesync[bot]
authored andcommitted
Show every group w.r.t. an object when an object belongs to multiple groups in the same partition
Summary: The object table currently shows only one group when an object belongs to multiple groups in an overlapping partition. [[https://www.internalfb.com/code/fbsource/[573d17b48f]/fbcode/rebalancer/explorer/cpp_server/lib/LoadModel.cpp?lines=138-160 | The partition column overwrites the object's group name]] as it visits each group, so only the last group remains. In this diff, [[https://www.internalfb.com/code/fbsource/[00b5e442e4cc]/fbcode/rebalancer/explorer/cpp_server/lib/LoadModel.cpp?lines=138-166 | disjoint partitions keep their existing construction, while overlapping partitions collect, sort, and join every group name]]. Objects in one group and objects outside the partition keep their existing output. Reviewed By: sud03r Differential Revision: D116634650 fbshipit-source-id: 62be80852f27ab38fb9e9314e15640c06d271577
1 parent 4b0c51d commit 469e9b2

2 files changed

Lines changed: 34 additions & 5 deletions

File tree

algopt/rebalancer/explorer/cpp_server/lib/LoadModel.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <fmt/core.h>
2222
#include <folly/FileUtil.h>
2323
#include <folly/futures/SharedPromise.h>
24+
#include <folly/String.h>
2425
#include <folly/Synchronized.h>
2526
#include <folly/system/HardwareConcurrency.h>
2627

@@ -138,11 +139,19 @@ static std::vector<std::shared_ptr<const Column>> buildPartitionCols(
138139
auto& partition = universe.getPartition(partitionId);
139140
auto& partitionName = universe.getEntityName(partitionId);
140141
Map<EntityId, DataCell> objectToCell;
141-
for (auto groupId : partition.getGroupIds()) {
142-
auto& groupName = universe.getEntityName(groupId);
143-
for (auto objectId : partition.getObjectIds(groupId)) {
144-
const DataCell name(groupName);
145-
objectToCell[toEntityId(objectId)] = name;
142+
for (const auto& [objectId, groupIds] : partition.getObjectIdToGroupIds()) {
143+
if (groupIds.size() == 1) {
144+
objectToCell[toEntityId(objectId)] =
145+
DataCell(universe.getEntityName(groupIds.front()));
146+
} else {
147+
std::vector<std::string> groupNames;
148+
groupNames.reserve(groupIds.size());
149+
for (const auto groupId : groupIds) {
150+
groupNames.push_back(universe.getEntityName(groupId));
151+
}
152+
std::sort(groupNames.begin(), groupNames.end());
153+
objectToCell[toEntityId(objectId)] =
154+
DataCell(folly::join(", ", groupNames));
146155
}
147156
}
148157
DataCell defaultPartition("");

algopt/rebalancer/explorer/cpp_server/tests/LoadModelTest.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,26 @@ TEST(LoadModelTest, Basic) {
311311
}
312312
}
313313

314+
TEST(LoadModelTest, OverlappingPartitionJoinsGroups) {
315+
auto bundle = TestUtils::buildBundle({.includeOverlappedPartition = true});
316+
auto explorerModel = LoadModel::buildData(std::move(bundle));
317+
addObjectTable(explorerModel);
318+
319+
const auto& universe = *explorerModel.universe;
320+
const auto& table = explorerModel.tableData.at("host");
321+
const auto partitionColumn =
322+
Utils::fetchColumn(table.getColumnData(), "overlapped");
323+
EXPECT_EQ(
324+
"group1, group2",
325+
partitionColumn->getStrView(toEntityId(universe.getObjectId("host0"))));
326+
EXPECT_EQ(
327+
"group1",
328+
partitionColumn->getStrView(toEntityId(universe.getObjectId("host3"))));
329+
EXPECT_EQ(
330+
"",
331+
partitionColumn->getStrView(toEntityId(universe.getObjectId("host4"))));
332+
}
333+
314334
TEST(LoadModelTest, DynamicDimensionTableUsesGroupRowsForCompactStorage) {
315335
UniverseProblemBuilder builder(
316336
std::make_unique<AsyncConfig>(getTestExecutor(/*numThreads=*/true)));

0 commit comments

Comments
 (0)