*: adaptively control analyze distsql concurrency #53261
Conversation
…b node Signed-off-by: Weizhen Wang <[email protected]>
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #53261 +/- ##
================================================
- Coverage 74.3906% 74.0185% -0.3722%
================================================
Files 1493 1524 +31
Lines 356315 444505 +88190
================================================
+ Hits 265065 329016 +63951
- Misses 71913 95362 +23449
- Partials 19337 20127 +790
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pkg/executor/builder.go
Outdated
| if concurrency <= 0 { | ||
| concurrency = adaptiveAnlayzeDistSQLConcurrency(context.Background(), b.ctx) | ||
| } |
There was a problem hiding this comment.
Could we merge this logic with AnalyzeDistSQLScanConcurrency or create a new function, since they are always called together?
There was a problem hiding this comment.
Done, it has been merged.
| ) | ||
|
|
||
| func adaptiveAnlayzeDistSQLConcurrency(ctx context.Context, sctx sessionctx.Context) int { |
There was a problem hiding this comment.
How often is this function called? Could it be invoked so frequently in some cases that it overwhelms the PD API server?
There was a problem hiding this comment.
Every time, analyze jobs need to run.
There was a problem hiding this comment.
How often is this function called? Could it be invoked so frequently in some cases that it overwhelms the PD API server?
Auto analyze. But I think it's QPS <= 10 qps
Signed-off-by: Weizhen Wang <[email protected]>
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/approve |
|
/hold @hi-rustin PTAL |
ti-chi-bot
bot
added
the
do-not-merge/hold
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: easonn7, hi-rustin, JmPotato, winoros The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
|
/retest |
1 similar comment
|
/retest |
What problem does this PR solve?
Issue Number: close #53262
Problem Summary:
What changed and how does it work?
When tidb cluster is larger and larger, the concurrency of the analyze scan should be larger to have a better performance. so we should make analyze_distsql_scan_concurrency adaptive.
BTW, Even a fourfold increase in concurrency seems conservative; for some customers, the actual concurrency is a thousand times the default value.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.