Limit v2 compute load by CPU #861
Conversation
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 4be8e9f.
🔴 linux-x86_64-release-asan: some tests FAILED for commit 4be8e9f.
|
Hor911
force-pushed
the
YQ-2702
branch
2 times, most recently
from
fb31e1e to
4be8e9f
Compare
| PrepareAstAndPlan(pingTaskRequest, QueryStats.query_plan(), QueryStats.query_ast()); | ||
| try { | ||
| pingTaskRequest.set_statistics(GetV1StatFromV2Plan(QueryStats.query_plan())); | ||
| TDuration duration = TDuration::MicroSeconds(QueryStats.total_duration_us()); |
There was a problem hiding this comment.
Can we move this code to the separate function PrepareStatistics(...)?
There was a problem hiding this comment.
I've made this refactoring already with next "--stat_full" feature, will merge in next PR
| TString Scope; | ||
| TDuration Duration; | ||
| double CpuSecondsConsumed; | ||
| double Quota; // if zero, default quota is used |
There was a problem hiding this comment.
Where is this parameter filled in or is it for the future usage?
There was a problem hiding this comment.
It was idea that client my override default expected load in relation to compiled query size (task count). Currently is not used
ydb/core/fq/libs/compute/ydb/control_plane/compute_database_control_plane_service.cpp
Outdated
Show resolved
Hide resolved
|
|
||
| void Handle(TEvYdbCompute::TEvCpuLoadRequest::TPtr& ev) { | ||
| auto actorId = GetMonitoringActorIdByScope(ev.Get()->Get()->Scope); | ||
| if (actorId != TActorId{}) { |
There was a problem hiding this comment.
if (actorId != TActorId{}) {
Send(ev->Forward(actorId));
return;
}
Send(ev->Sender, new TEvYdbCompute::TEvCpuLoadResponse(NYql::TIssues{NYql::TIssue{TStringBuilder{} << "Cluster load monitoring disabled"}}), 0, ev->Cookie);
to get rid of else
There was a problem hiding this comment.
does not matter in this case imho
but yes, no "else" - simplier code
| uint32 DefaultQueryLoadPercentage = 5; // default 10 | ||
| uint32 PendingQueueSize = 6; // default 0 == instant decline if overloaded | ||
| bool Inherit = 7; // whether to apply global config to databases (override, NOT merge) | ||
| uint32 CpuNumber = 8; |
There was a problem hiding this comment.
Do we need this option? What does this option mean (per house, per cluster)?
There was a problem hiding this comment.
It is number if CPU per cluster. It is needed to align two different "systems". Why I ask for cluster load with Monitoring Service, I've got ratio between 0.0 and 1.0 for complete cluster. Query reports its usage in number of CPUs. To convert one to another we need to know number of CPU per cluster.
There was a problem hiding this comment.
Do we have another way to get this information? It's difficult to keep in consistent state two configs (ydb sls and fq proxy config). If we don't have another way than this solution is ok for me :)
There was a problem hiding this comment.
No other sources is available. This param is not mandatory, it is required for "cashback" feature - decrease CPU usage estimation after query completion. Allows better CPU utilization for short light queries
|
⚪
|
|
⚪
|
|
⚪
|
* Limit v2 compute load by CPU * Mon fix * Per db load config + cpu load ajustment (cashback) * Review fixes * Clear config etc
3 participants
No description provided.