statistics: support global singleflight for sync load #52796
Conversation
ti-chi-bot
bot
added
do-not-merge/invalid-title
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
hawkingrei
changed the title
ti-chi-bot
bot
removed
do-not-merge/invalid-title
do-not-merge/needs-tests-checked
labels
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
from
6cad5a3
to
ba4654c
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #52796 +/- ##
================================================
+ Coverage 72.4170% 74.2834% +1.8664%
================================================
Files 1477 1480 +3
Lines 427989 430723 +2734
================================================
+ Hits 309937 319956 +10019
+ Misses 98854 90824 -8030
- Partials 19198 19943 +745
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/retest |
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
2 times, most recently
from
a5ca967
to
36637fc
Compare
ti-chi-bot
bot
added
size/L
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
from
fe5cf09
to
145a589
Compare
ti-chi-bot
bot
added
the
needs-rebase
| @@ -211,17 +211,18 @@ func TestConcurrentLoadHistWithPanicAndFail(t *testing.T) { | |||
| task3, err3 := h.HandleOneTask(testKit.Session().(sessionctx.Context), task1, exitCh) | |||
| require.NoError(t, err3) | |||
| require.Nil(t, task3) | |||
|
|
|||
| task, err3 := h.HandleOneTask(testKit.Session().(sessionctx.Context), nil, exitCh) | |||
There was a problem hiding this comment.
Now, we merge the same request on the top. it has nothing in the task channel. so it will block.
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
from
85816a1
to
ea67022
Compare
ti-chi-bot
bot
removed
the
needs-rebase
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
2 times, most recently
from
c2ebf60
to
2d1376c
Compare
|
the old singleflight still exists, but it is to support timeout for tasks in the worker. |
hawkingrei
force-pushed
the
global_singleflight_for_sync_load_v2
branch
2 times, most recently
from
76077b1
to
9b609ab
Compare
| @@ -312,11 +323,11 @@ func TestRetry(t *testing.T) { | |||
| result, err1 := h.HandleOneTask(testKit.Session().(sessionctx.Context), task1, exitCh) | |||
| require.NoError(t, err1) | |||
| require.Nil(t, result) | |||
| select { | |||
There was a problem hiding this comment.
We cannot use the old way to check. the item in the channel will be fetched by globalSingleFlight.
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
|
/cherrypick release-8.1 |
|
@hawkingrei: new pull request created to branch In response to this:
Instructions 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 ti-community-infra/tichi repository. |
|
/cherrypick release-7.5 |
|
@hawkingrei: new pull request created to branch In response to this:
Instructions 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 ti-community-infra/tichi repository. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
/cherrypick release-7.1 |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
@hawkingrei: new pull request created to branch In response to this:
Instructions 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 ti-community-infra/tichi repository. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
@hawkingrei: new pull request created to branch In response to this:
Instructions 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 ti-community-infra/tichi repository. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
What problem does this PR solve?
Issue Number: close #52797
Problem Summary:
What changed and how does it work?
before
All items the query needs will be put into the global task channel( the default capacity is 1000). And workers will merge the same task with the singleflight. So performance depends on the number of tasks that can be concurrently executed.
Under poor performance conditions, he will not be able to handle a sufficient number of tasks, resulting in slow processing. But there are still many same tasks in the channel. So they still need to check whether to sync load. And They will fill up this channel. Query can not insert the sync load task into it and load necessary stats.
after
So we need to refactor this code. We need first to let the query pass through singleflight to merge identical tasks. Then insert the task into the channel to make the worker syncload it.
So why is it that during the startup of Wetech, the queries are very unstable and there are severe timeouts with sync load? The problem lies right here.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.