*: merge the runtime stats in time to avoid using too many memory by wshwsh12 · Pull Request #39394 · pingcap/tidb

wshwsh12 · 2022-11-25T06:40:02Z

What problem does this PR solve?

Issue Number: close #39413

Now tidb need to record some runtime stats for each executor/cop_task and so on, and merge them after the sql finfished.
If there are many cop tasks, the memory can't gc and hold util the sql finfished. Maybe it cause OOM.

Problem Summary:

When the stats will not updated, merge it to the summary stats.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2022-11-25T06:40:04Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

XuHuaiyu
crazycs520

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

wshwsh12 · 2022-11-25T06:40:10Z

/hold

wshwsh12 · 2022-11-25T07:41:59Z

/run-unit-test

wshwsh12 · 2022-11-28T04:45:27Z

/unhold

crazycs520 · 2022-11-28T04:57:19Z

 		r.memConsume(-respSize)
 	}
+	if r.stats != nil {
+		defer r.ctx.GetSessionVars().StmtCtx.RuntimeStatsColl.RegisterStats(r.rootPlanID, r.stats)


why use defer here?

Use defer to ensure that the Close()(make sure all goroutines have been exit in some parallel executer) is finished before registering stats.

hawkingrei · 2022-11-29T06:22:55Z

@wshwsh12

    execdetails_test.go:228: 
        	Error Trace:	/home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/3543/execroot/__main__/bazel-out/k8-fastbuild/bin/util/execdetails/execdetails_test_/execdetails_test.runfiles/__main__/util/execdetails/execdetails_test.go:228
        	Error:      	Not equal: 
        	            	expected: "tiflash_task:{proc max:2ns, min:1ns, avg: 1ns, p80:2ns, p95:2ns, iters:3, tasks:2, threads:2}, tiflash_scan:{dmfile:{total_scanned_packs:1, total_skipped_packs:0, total_scanned_rows:8192, total_skipped_rows:0, total_rough_set_index_load_time: 15ms, total_read_time: 202ms}, total_create_snapshot_time: 40ms}"
        	            	actual  : "tiflash_task:{proc max:2ns, min:1ns, avg: 1ns, p80:2ns, p95:2ns, iters:3, tasks:2, threads:2}"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-tiflash_task:{proc max:2ns, min:1ns, avg: 1ns, p80:2ns, p95:2ns, iters:3, tasks:2, threads:2}, tiflash_scan:{dmfile:{total_scanned_packs:1, total_skipped_packs:0, total_scanned_rows:8192, total_skipped_rows:0, total_rough_set_index_load_time: 15ms, total_read_time: 202ms}, total_create_snapshot_time: 40ms}
        	            	+tiflash_task:{proc max:2ns, min:1ns, avg: 1ns, p80:2ns, p95:2ns, iters:3, tasks:2, threads:2}
        	Test:       	TestCopRuntimeStatsForTiFlash

wshwsh12 · 2022-11-29T07:15:46Z

/merge

ti-chi-bot · 2022-11-29T07:15:50Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 35bbb14

wshwsh12 · 2022-11-29T08:24:46Z

/run-check_dev_2

wshwsh12 · 2022-11-29T10:33:05Z

/run-all-tests

ti-chi-bot · 2022-11-29T13:00:46Z

In response to a cherrypick label: new pull request created: #39462.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2022-11-29T13:01:28Z

In response to a cherrypick label: new pull request created: #39463.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2022-11-29T13:02:11Z

In response to a cherrypick label: new pull request created: #39464.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

sre-bot · 2022-11-29T13:24:18Z

TiDB MergeCI notify

✅ Well Done! New fixed [2] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/sqllogic-test-2	✅ all 28 tests passed	6 min 13 sec	Fixed
idc-jenkins-ci-tidb/integration-ddl-test	✅ all 6 tests passed	5 min 43 sec	Fixed
idc-jenkins-ci/integration-cdc-test	🟢 all 40 tests passed	19 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	17 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	10 min	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	6 min 13 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 4 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	4 min 33 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	3 min 11 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

wshwsh12 added 4 commits November 22, 2022 15:41

fix

bb86101

try fix

a61d579

fix

2ec13f4

fix test

fb05c46

wshwsh12 requested a review from a team as a code owner November 25, 2022 06:40

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Nov 25, 2022

Merge branch 'master' into runtime-info

44689c9

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 25, 2022

wshwsh12 and others added 4 commits November 25, 2022 15:45

fix nil

e040e9b

Merge branch 'master' into runtime-info

ae7a339

fix race

030cf94

Merge branch 'master' into runtime-info

a215d23

wshwsh12 requested review from XuHuaiyu and crazycs520 November 28, 2022 04:45

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2022

crazycs520 reviewed Nov 28, 2022

View reviewed changes

Comment thread executor/distsql.go

XuHuaiyu changed the title *: merge the runtime stats in time to avoid using too many memroy *: merge the runtime stats in time to avoid using too many memory Nov 28, 2022

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2022

Merge remote-tracking branch 'upstream/master' into runtime-info

851f355

wshwsh12 and others added 2 commits November 29, 2022 11:59

Merge branch 'master' into runtime-info

fb886b2

Merge branch 'master' into runtime-info

f010542

crazycs520 approved these changes Nov 29, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 29, 2022

XuHuaiyu approved these changes Nov 29, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 29, 2022

fix-test

35bbb14

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 29, 2022

ti-chi-bot added 2 commits November 29, 2022 15:15

Merge branch 'master' into runtime-info

c3953e1

Merge branch 'master' into runtime-info

54cf69a

Merge branch 'master' into runtime-info

0d4a530

hawkingrei and others added 2 commits November 29, 2022 19:53

Merge branch 'master' into runtime-info

5722937

Merge branch 'master' into runtime-info

6b3e7dc

ti-chi-bot merged commit 23543a4 into pingcap:master Nov 29, 2022

ti-chi-bot mentioned this pull request Nov 29, 2022

*: merge the runtime stats in time to avoid using too many memory (#39394) #39462

Closed

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 29, 2022

This is an automated cherry-pick of pingcap#39394

9954b45

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 29, 2022

*: merge the runtime stats in time to avoid using too many memory (#39394) #39463

Closed

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 29, 2022

This is an automated cherry-pick of pingcap#39394

34f9db4

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 29, 2022

*: merge the runtime stats in time to avoid using too many memory (#39394) #39464

Closed

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 29, 2022

This is an automated cherry-pick of pingcap#39394

bf74e2b

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

Conversation

wshwsh12 commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wshwsh12 commented Nov 25, 2022

Uh oh!

wshwsh12 commented Nov 25, 2022

Uh oh!

wshwsh12 commented Nov 28, 2022

Uh oh!

crazycs520 Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

wshwsh12 Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hawkingrei commented Nov 29, 2022

Uh oh!

wshwsh12 commented Nov 29, 2022

Uh oh!

ti-chi-bot commented Nov 29, 2022

Uh oh!

wshwsh12 commented Nov 29, 2022

Uh oh!

wshwsh12 commented Nov 29, 2022

Uh oh!

ti-chi-bot commented Nov 29, 2022

Uh oh!

ti-chi-bot commented Nov 29, 2022

Uh oh!

ti-chi-bot commented Nov 29, 2022

Uh oh!

sre-bot commented Nov 29, 2022

TiDB MergeCI notify

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wshwsh12 commented Nov 25, 2022 •

edited

Loading

ti-chi-bot commented Nov 25, 2022 •

edited

Loading