Skip to content

Efficient get team by servers v3 #12881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spraza wants to merge 4 commits into from
Closed

Efficient get team by servers v3 #12881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2cca1a
Add hash index for getTeamByServers O(1) lookup
spraza Mar 26, 2026
f0db5f1
Suppress PingLatency events (#12316)
jzhou77 Aug 20, 2025
6475fb7
Update version range for Snap Restart test (#12283)
xy-54321 Aug 20, 2025
94ea9b0
Fix restarting test failures (#12324)
jzhou77 Aug 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fdbrpc/FlowTransport.actor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ ACTOR Future<Void> pingLatencyLogger(TransportData* self) {
if (peer && (peer->pingLatencies.getPopulationSize() >= 10 || peer->connectFailedCount > 0 ||
peer->timeoutCount > 0)) {
TraceEvent("PingLatency")
.suppressFor(30.0)
.detail("Elapsed", now() - peer->lastLoggedTime)
.detail("PeerAddr", lastAddress)
.detail("PeerAddress", lastAddress)
Expand Down
11 changes: 5 additions & 6 deletions fdbserver/DDTeamCollection.actor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,10 @@ class DDTeamCollectionImpl {

// Find the team with the exact storage servers as req.src.
static void getTeamByServers(DDTeamCollection* self, GetTeamRequest req) {
const std::string servers = TCTeamInfo::serversToString(req.src);
Optional<Reference<IDataDistributionTeam>> res;
for (const auto& team : self->teams) {
if (team->getServerIDsStr() == servers) {
res = team;
break;
}
auto it = self->teamsByServerIDs.find(TCTeamInfo::serversToString(req.src));
if (it != self->teamsByServerIDs.end()) {
res = it->second;
}
req.reply.send(std::make_pair(res, false));
}
Expand Down Expand Up @@ -4763,6 +4760,7 @@ void DDTeamCollection::addTeam(const std::vector<Reference<TCServerInfo>>& newTe

// For a good team, we add it to teams and create machine team for it when necessary
teams.push_back(teamInfo);
teamsByServerIDs[teamInfo->getServerIDsStr()] = teamInfo;
for (auto& server : newTeamServers) {
server->addTeam(teamInfo);
}
Expand Down Expand Up @@ -5688,6 +5686,7 @@ void DDTeamCollection::addServer(StorageServerInterface newServer,

bool DDTeamCollection::removeTeam(Reference<TCTeamInfo> team) {
TraceEvent("RemovedServerTeam", distributorId).detail("Team", team->getDesc());
teamsByServerIDs.erase(team->getServerIDsStr());
Copy link
Copy Markdown
Contributor

@alecgrieser alecgrieser Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: It may make sense to make this call to erase conditional on actually finding the team. The call to erase if the team isn't found should be a no-op, but it shouldn't hurt

bool found = false;
for (int t = 0; t < teams.size(); t++) {
if (teams[t] == team) {
Expand Down
5 changes: 5 additions & 0 deletions fdbserver/include/fdbserver/DDTeamCollection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,12 @@ class DDTeamCollection : public ReferenceCounted<DDTeamCollection> {
std::map<Standalone<StringRef>, Reference<TCMachineInfo>> machine_info;
std::vector<Reference<TCMachineTeamInfo>> machineTeams; // all machine teams

// IMPORTANT: teams and teamsByServerIDs MUST be consistent, so any time we
// mutate teams, we must also mutate teamsByServerIDs
std::vector<Reference<TCTeamInfo>> teams;
// O(1) hash map from server ID string to team information
// Currently used by getTeamByServers
std::unordered_map<std::string, Reference<TCTeamInfo>> teamsByServerIDs;

std::vector<DDTeamCollection*> teamCollections;
AsyncTrigger printDetailedTeamsInfo;
Expand Down
24 changes: 12 additions & 12 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,26 +414,26 @@ if(WITH_PYTHON)
TEST_FILES restarting/from_7.3.50/BlobGranuleRestartLarge-1.toml
restarting/from_7.3.50/BlobGranuleRestartLarge-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/SnapCycleRestart-1.toml
restarting/from_7.3.50/SnapCycleRestart-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/SnapCycleRestart-1.toml
restarting/from_7.3.50_until_7.4.0/SnapCycleRestart-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/SnapTestAttrition-1.toml
restarting/from_7.3.50/SnapTestAttrition-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/SnapTestAttrition-1.toml
restarting/from_7.3.50_until_7.4.0/SnapTestAttrition-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/SnapTestSimpleRestart-1.toml
restarting/from_7.3.50/SnapTestSimpleRestart-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/SnapTestSimpleRestart-1.toml
restarting/from_7.3.50_until_7.4.0/SnapTestSimpleRestart-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/SnapTestRestart-1.toml
restarting/from_7.3.50/SnapTestRestart-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/SnapTestRestart-1.toml
restarting/from_7.3.50_until_7.4.0/SnapTestRestart-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/ClientTransactionProfilingCorrectness-1.toml
restarting/from_7.3.50/ClientTransactionProfilingCorrectness-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/ClientTransactionProfilingCorrectness-1.toml
restarting/from_7.3.50_until_7.4.0/ClientTransactionProfilingCorrectness-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/CycleTestRestart-1.toml
restarting/from_7.3.50/CycleTestRestart-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/StorefrontTestRestart-1.toml
restarting/from_7.3.50/StorefrontTestRestart-2.toml)
TEST_FILES restarting/from_7.3.50_until_7.4.0/StorefrontTestRestart-1.toml
restarting/from_7.3.50_until_7.4.0/StorefrontTestRestart-2.toml)
add_fdb_test(
TEST_FILES restarting/from_7.3.50/ClientMetricRestart-1.toml
restarting/from_7.3.50/ClientMetricRestart-2.toml)
Expand Down
0 ...entTransactionProfilingCorrectness-1.toml → ...entTransactionProfilingCorrectness-1.toml
View file
Open in desktop
File renamed without changes.
0 ...entTransactionProfilingCorrectness-2.toml → ...entTransactionProfilingCorrectness-2.toml
View file
Open in desktop
File renamed without changes.
0 ...rting/from_7.3.50/SnapCycleRestart-1.toml → ....3.50_until_7.4.0/SnapCycleRestart-1.toml
View file
Open in desktop
File renamed without changes.
0 ...rting/from_7.3.50/SnapCycleRestart-2.toml → ....3.50_until_7.4.0/SnapCycleRestart-2.toml
View file
Open in desktop
File renamed without changes.
0 ...ting/from_7.3.50/SnapTestAttrition-1.toml → ...3.50_until_7.4.0/SnapTestAttrition-1.toml
View file
Open in desktop
File renamed without changes.
0 ...ting/from_7.3.50/SnapTestAttrition-2.toml → ...3.50_until_7.4.0/SnapTestAttrition-2.toml
View file
Open in desktop
File renamed without changes.
0 ...arting/from_7.3.50/SnapTestRestart-1.toml → ...7.3.50_until_7.4.0/SnapTestRestart-1.toml
View file
Open in desktop
File renamed without changes.
0 ...arting/from_7.3.50/SnapTestRestart-2.toml → ...7.3.50_until_7.4.0/SnapTestRestart-2.toml
View file
Open in desktop
File renamed without changes.
0 .../from_7.3.50/SnapTestSimpleRestart-1.toml → ..._until_7.4.0/SnapTestSimpleRestart-1.toml
View file
Open in desktop
File renamed without changes.
0 .../from_7.3.50/SnapTestSimpleRestart-2.toml → ..._until_7.4.0/SnapTestSimpleRestart-2.toml
View file
Open in desktop
File renamed without changes.
0 .../from_7.3.50/StorefrontTestRestart-1.toml → ..._until_7.4.0/StorefrontTestRestart-1.toml
View file
Open in desktop
File renamed without changes.
0 .../from_7.3.50/StorefrontTestRestart-2.toml → ..._until_7.4.0/StorefrontTestRestart-2.toml
View file
Open in desktop
File renamed without changes.