Adding a delay in getTeamByServers to prevent CPU monopolization#12865
Adding a delay in getTeamByServers to prevent CPU monopolization#12865neethuhaneesha wants to merge 1 commit intoapple:release-7.3from
Conversation
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
| state Optional<Reference<IDataDistributionTeam>> res; | ||
| state std::vector<Reference<TCTeamInfo>>::iterator teamIt; | ||
|
|
||
| TraceEvent("GetTeamByServersStart"); |
There was a problem hiding this comment.
How frequently is this being called today? I have the impression it's being called from within an outer loop and possibly hundreds to thousands of times per second. Changing that to once every 30s seems pretty radical.
If we want a lightweight metric to count this, something like this would work:
static SimpleCounter<int64_t>* counter =
SimpleCounter<int64_t>::makeCounter("/DD/getTeamByServers");
counter->increment(1);
(gets logged under SimpleCounters every few minutes and is way less expensive than a TraceEvent per invocation)
There was a problem hiding this comment.
I changed the code to print under suppress.. so it will print once every second
| teamIt = self->teams.begin(); | ||
| for (; teamIt != self->teams.end(); ++teamIt) { | ||
| if ((*teamIt)->getServerIDsStr() == servers) { | ||
| res = *teamIt; |
There was a problem hiding this comment.
Sorry if this is just adding noise, but should there be a yield somewhere in this loop? I'm asking because it seems like if the problem is that this task can result in CPU monopolization, it would make sense to let other tasks interrupt execution so that they can make progress
There was a problem hiding this comment.
There is a possibility of teams getting changed/updated by other code, if we yield in between the loop. Exactly not sure if that behavior is accepted or not.
Replace this text with your description here...
Code-Reviewer Section
The general pull request guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branchormainif this is the youngest branch)