Fix incorrect harmonizer behaviour with shared threads #19295
Conversation
|
⚪ |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the harmonizer to correctly account for shared threads by switching to per-thread CPU averages, introducing shared/free CPU quotas, and adjusting pool/thread quota interfaces.
- Replace integer-based diffs with floating-point per-thread averages and compute free/shared CPU quota
- Add
TPoolInfofields (ThreadQuota,FullThreadQuota) and extendAddPoolto accept anignoreFullThreadQuotaflag - Update various structures (
THarmonizerCpuConsumption,TPoolHarmonizerStats) to use floats instead of ints for finer granularity
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/library/actors/core/harmonizer/shared_info.h | Swap CpuConsumptionByPool for CpuConsumptionPerThread; add ThreadCount, FreeCpu, update Pull signature |
| ydb/library/actors/core/harmonizer/shared_info.cpp | Compute per-thread averages, reset/accumulate quotas, remove diff fields |
| ydb/library/actors/core/harmonizer/pool.h | Add FullThreadQuota, ThreadQuota, change PotentialMaxThreadCount to float |
| ydb/library/actors/core/harmonizer/harmonizer.h | Update IHarmonizer::AddPool to include ignoreFullThreadQuota; change PotentialMaxThreadCount to float |
| ydb/library/actors/core/harmonizer/harmonizer.cpp | Integrate shared CPU logic into Harmonize, update AddPool and quota adjustments |
| ydb/library/actors/core/harmonizer/cpu_consumption.h | Introduce BudgetWithoutSharedCpu, LostCpu; remove BudgetInt |
| ydb/library/actors/core/harmonizer/cpu_consumption.cpp | Use ThreadQuota in budget calc, rename param to sharedInfo, update budget/lost CPU logic |
| ydb/library/actors/core/cpu_manager.cpp | Pass ignoreFullThreadQuota flag based on pool type to AddPool |
Comments suppressed due to low confidence (1)
ydb/library/actors/core/cpu_manager.cpp:108
- [nitpick] The name
ignoreThreadsis ambiguous; consider renaming toignoreFullThreadQuotaorisIoPoolfor clarity.
bool ignoreThreads = dynamic_cast<TIOExecutorPool*>(Executors[excIdx].Get());
|
⚪ |
|
🟢 |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
kruall
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
2 participants
Changelog entry
...
Changelog category
Description for reviewers
...