executor: tiny optimize import into from select performance by crazycs520 · Pull Request #60378 · pingcap/tidb

crazycs520 · 2025-04-02T10:33:04Z

What problem does this PR solve?

Issue Number: ref #60377

Problem Summary: tiny optimize import into from select performance

What changed and how does it work?

Tiny fix for better pipeline.

Manual test:

prepare:

sysbench --config-file=sysbench.conf oltp_point_select --tables=2 --table-size=10000 --threads=2 prepare

CREATE TABLE `t` (
  `id` int(11) NOT NULL,
  `k` int(11) NOT NULL DEFAULT '0',
  `c` char(120) NOT NULL DEFAULT '',
  `pad` char(60) NOT NULL DEFAULT '',
  `id2` int(11) NOT NULL,
  `k2` int(11) NOT NULL DEFAULT '0',
  `c2` char(120) NOT NULL DEFAULT '',
  `pad2` char(60) NOT NULL DEFAULT '',
  KEY `k_1` (`k`)
);

Before:

> import into t from select * from sbtest1 join sbtest2 where sbtest2.id <=2000 with thread=16,DISABLE_PRECHECK;
Query OK, 20000000 rows affected
Time: 94.425s

related tidb log:

[2025/04/03 01:24:12.759 +08:00] [INFO] [chunk_process.go:334] ["process chunk completed"] [table=t] [import-id=431840f5-f697-4b3b-8093-e8dcb329d77a] [key=import-from-select] [readDur=43.395559463s] [encodeDur=9.013400878s] [checksum="{cksum=1539544195137435216,size=563438023,kvs=2499458}"] [deliverDur=1.608033096s] [type=query] [takeTime=53.673599639s] []

[readDur=43.395559463s] [encodeDur=9.013400878s] [checksum="{cksum=1539544195137435216,size=563438023,kvs=2499458}"] [deliverDur=1.608033096s] [type=query] [takeTime=53.673599639s]

This PR:

> import into t from select * from sbtest1 join sbtest2 where sbtest2.id <=2000 with thread=16,DISABLE_PRECHECK;
Query OK, 20000000 rows affected
Time: 71.866s

related tidb log:

[2025/04/03 01:21:09.799 +08:00] [INFO] [chunk_process.go:334] ["process chunk completed"] [table=t] [import-id=9db22ac4-c6d3-40cd-99d0-1f1676b4fd20] [key=import-from-select] [readDur=23.43142496s] [encodeDur=7.273896193s] [checksum="{cksum=3503798787028937410,size=565142813,kvs=2507026}"] [deliverDur=1.667509888s] [type=query] [takeTime=31.962920615s] []

[readDur=23.43142496s] [encodeDur=7.273896193s] [checksum="{cksum=3503798787028937410,size=565142813,kvs=2507026}"] [deliverDur=1.667509888s] [type=query] [takeTime=31.962920615s]

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

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

Signed-off-by: crazycs520 <chenshuang@pingcap.cn>

ti-chi-bot · 2025-04-02T10:33:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from crazycs520, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sre-bot · 2025-04-02T10:33:11Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

crazycs520 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

tiprow · 2025-04-02T10:33:22Z

Hi @crazycs520. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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 kubernetes-sigs/prow repository.

codecov · 2025-04-02T10:52:29Z

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.5424%. Comparing base (7e32850) to head (610b588).
Report is 2 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #60378        +/-   ##
================================================
+ Coverage   73.1433%   73.5424%   +0.3991%     
================================================
  Files          1710       1710                
  Lines        472848     472894        +46     
================================================
+ Hits         345857     347778      +1921     
+ Misses       105725     103850      -1875     
  Partials      21266      21266

Flag	Coverage Δ
integration	`42.9334% <0.0000%> (?)`
unit	`72.3387% <0.0000%> (-0.0182%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`47.2367% <ø> (+0.0332%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ti-chi-bot · 2025-04-02T10:57:01Z

@crazycs520: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
idc-jenkins-ci-tidb/check_dev_2	`610b588`	link	true	`/test check-dev2`

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

D3Hunter · 2025-04-02T11:18:11Z

 		fields := exec.RetTypes(e.selectExec)
 		var idAllocator int64
+		chkSize := e.selectExec.InitCap()
+		maxChkSize := e.selectExec.MaxChunkSize()


what's the value of MaxChunkSize?

It is the value of tidb_max_chunk_size, by default the value is 1024.

D3Hunter

rest lgtm

can you also consider this opt direction #60377 (comment)

Benjamin2037 · 2025-04-02T11:40:17Z

Please add the necessary test cases.

Benjamin2037 · 2025-04-02T11:44:21Z

/hold

D3Hunter · 2025-04-02T11:50:06Z

 		}
 	}()
-	selectedRowCh := make(chan importer.QueryRow)
+	selectedRowCh := make(chan importer.QueryRow, e.MaxChunkSize())


does the optimization comes from enlarge the channel, or enlarge the below chunk?

in most cases, unbuffered channel is enough

I prefer to set the channel capacity to worker count * worker batch size. So if the average speed of producer (SELECT side) is larger than consumer (encode worker side) the worker will never be blocked

Only enlarge chunk size

> import into t from select * from sbtest1 join sbtest2 where sbtest2.id <=2000 with thread=16,DISABLE_PRECHECK; Query OK, 20000000 rows affected Time: 94.532s

[2025/04/02 21:36:16.306 +08:00] [INFO] [chunk_process.go:334] ["process chunk completed"] [table=t] [import-id=97bba291-a9f1-407d-a695-d7bf395664bf] [key=import-from-select] [readDur=44.168811199s] [encodeDur=9.422960425s] [checksum="{cksum=12542963264396444897,size=561438104,kvs=2490588}"] [deliverDur=1.536865319s] [type=query] [takeTime=54.887474621s] []

master

> import into t from select * from sbtest1 join sbtest2 where sbtest2.id <=2000 with thread=16,DISABLE_PRECHECK; Query OK, 20000000 rows affected Time: 96.610s

[2025/04/02 21:38:56.446 +08:00] [INFO] [chunk_process.go:334] ["process chunk completed"] [table=t] [import-id=a5ac398c-ebaa-4711-8887-52138fe490c2] [key=import-from-select] [readDur=46.095255199s] [encodeDur=9.143337612s] [checksum="{cksum=4793880330072286733,size=563626556,kvs=2500296}"] [deliverDur=1.546770509s] [type=query] [takeTime=56.509829908s] []

This PR, enlarge the channel and enlarge the chunk size?:

> import into t from select * from sbtest1 join sbtest2 where sbtest2.id <=2000 with thread=16,DISABLE_PRECHECK; Query OK, 20000000 rows affected Time: 65.959s

[2025/04/02 21:42:58.107 +08:00] [INFO] [chunk_process.go:334] ["process chunk completed"] [table=t] [import-id=8cc33018-c79a-4b42-a133-d3f86bb45201] [key=import-from-select] [readDur=16.859098986s] [encodeDur=7.161220248s] [checksum="{cksum=4348892783063240861,size=564394376,kvs=2503700}"] [deliverDur=1.613289223s] [type=query] [takeTime=25.343078567s] []

crazycs520 · 2025-04-02T13:49:56Z

rest lgtm

can you also consider this opt direction #60377 (comment)

see #60377 (comment)

crazycs520 · 2025-04-02T17:31:49Z

#60384 is much better than this.

refine

610b588

Signed-off-by: crazycs520 <chenshuang@pingcap.cn>

ti-chi-bot Bot added the release-note-none Denotes a PR that doesn't merit a release note. label Apr 2, 2025

ti-chi-bot Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 2, 2025

crazycs520 added the type/performance label Apr 2, 2025

D3Hunter reviewed Apr 2, 2025

View reviewed changes

ti-chi-bot Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 2, 2025

D3Hunter reviewed Apr 2, 2025

View reviewed changes

crazycs520 closed this Apr 2, 2025

Conversation

crazycs520 commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot Bot commented Apr 2, 2025

Uh oh!

sre-bot commented Apr 2, 2025

Uh oh!

tiprow Bot commented Apr 2, 2025

Uh oh!

codecov Bot commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ti-chi-bot Bot commented Apr 2, 2025

Uh oh!

D3Hunter Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crazycs520 Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Benjamin2037 commented Apr 2, 2025

Uh oh!

Benjamin2037 commented Apr 2, 2025

Uh oh!

D3Hunter Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

lance6716 Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

crazycs520 Apr 2, 2025

Choose a reason for hiding this comment

Only enlarge chunk size

master

This PR, enlarge the channel and enlarge the chunk size?:

Uh oh!

crazycs520 commented Apr 2, 2025

Uh oh!

crazycs520 commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

crazycs520 commented Apr 2, 2025 •

edited

Loading

codecov Bot commented Apr 2, 2025 •

edited

Loading

D3Hunter Apr 2, 2025 •

edited

Loading