global sort: reduce the memory usage of merge sort concurrent reader#62921
global sort: reduce the memory usage of merge sort concurrent reader#62921ti-chi-bot[bot] merged 8 commits intopingcap:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
Hi @fzzf678. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #62921 +/- ##
================================================
+ Coverage 72.8138% 73.4498% +0.6359%
================================================
Files 1823 1853 +30
Lines 494575 502708 +8133
================================================
+ Hits 360119 369238 +9119
+ Misses 112573 110903 -1670
- Partials 21883 22567 +684
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
tidb/pkg/ddl/backfilling_merge_sort.go Line 92 in 16ec21b memSizePerConto control the concurrency of concurrent reader?The memSizePerCon can't control, need another value to control the total memory usage of concurrent reader
|
There was a problem hiding this comment.
before this PR, each subtask tasks about 44-46 minutes. After this PR, each subtask tasks about 33-34 minutes to finish merge sort.
should before this PR OOM? where the 44-46 minutes came from?
i think this PR only avoid OOM, why the performance is getting better?
The OOM occurs when |
[LGTM Timeline notifier]Timeline:
|
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: D3Hunter, tangenta, wjhuang2016 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
In response to a cherrypick label: new pull request created to branch |
What problem does this PR solve?
Issue Number: close #62853
Problem Summary:
What changed and how does it work?
Reduce the concurrency of concurrent readers, pass the
outerConcurrencycorrectly.In the previous PR, we increased the concurrency to get better performance and if there are multiple readers concurrently read, the
outerConcurrencywill be used to control the whole concurrency. https://github.com/pingcap/tidb/pull/48871/files#diff-1ea5d87a202f7a3a089796fa8ac98c520879a2bdfdc739ed98a2261a9a7e8ad1R377-R381But the
outerConcurrencywas forgotten to be passed(see https://github.com/pingcap/tidb/pull/48871/files#diff-b573df9af3de0ee789f4ec5114d897cc2f9b5ac57f7cc19757e2353127680edfR91), the reader concurrency is always 256 no matter how many reader we use, this leads to the OOM.Check List
Tests
outerConcurrencycorrectly in this PR, the add index can be success without OOM. The profile as shown below, we can see the memory occupied byswitchToConcurrentReaderis 2GiB, this is expected because there are 8 readers and each has 32 concurrency with 256MiB memory, totally 8*256MiB = 2GiB.tidb_ddl_reorg_worker_cnt = 4. Before this PR, each subtask tasks about 44-46 minutes. After this PR, each subtask tasks about 33-34 minutes to finish merge sort.We detect the reason is, previously the concurrency was too high, which slowed down the pull of some goroutines, causing tail requests to drag down the overall situation. After reducing the concurrency, resource contention and congestion were alleviated, slow requests were reduced, and the overall tail delay converged. Resource contention is reduced or congestion handling capabilities are improved, making data transmission more stable and efficient. This PR is mainly to fix the OOM problem, maybe we can continue investigating performance improvements later.
Before(repeat 3 times):
After(repeat 3 times):
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.