executor: run idxlookup workers in a pool

[zyguan]

What problem does this PR solve?

Issue Number: ref #56649

Problem Summary: IndexLookupExecutor always starts 1+indexLookupConcurrency goroutines which is expensive for common queries.

What changed and how does it work?

Use a goroutine pool for workers and spawn new goroutine on demand.

Let prepare sysbench dataset with --secondary and run point select workload.

Previously, tidb spawns 6 goroutines for each query and only two of them are used (non-concurrently).

With this PR, only 1 goroutine are created for both index task and table task.

Here are benchmark results.

workload	threads	pool	base	diff
idxlookup_1	15	16560.51	16320.65	1.47%
idxlookup_1	30	26729.51	26146.49	2.23%
idxlookup_1	60	29680.83	28253.65	5.05%

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

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

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

None

[ti-chi-bot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[tiprow]

Hi @zyguan. 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.

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.

[zyguan]

/ok-to-test

[codecov]

Codecov Report

Attention: Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.7279%. Comparing base (8c88823) to head (954be6b).
Report is 35 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58033        +/-   ##
================================================
+ Coverage   73.1477%   73.7279%   +0.5802%     
================================================
  Files          1674       1676         +2     
  Lines        461685     467809      +6124     
================================================
+ Hits         337712     344906      +7194     
+ Misses       103209     101995      -1214     
- Partials      20764      20908       +144     

Flag	Coverage Δ
integration	43.1896% <85.2631%> (?)
unit	72.3378% <96.8421%> (+0.0587%)	⬆️

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

Components	Coverage Δ
dumpling	52.6910% <ø> (ø)
parser	∅ <ø> (∅)
br	45.4431% <ø> (-0.5522%)	⬇️

[zyguan]

/retest

[zyguan]

/retest

[cfzjywxk]

How about using a lock free queue to store the pending task pointers, so the Mutex could be avoided?

[zyguan]

Here gopool is mainly for concurrency control, each IndexLookupExecutor has its own pool, that is, there are only about indexLookupConcurrency goroutines access the pool at most 2xNumOfTableTasks times. I think it's acceptable, no need to introduce additional complexity.

[cfzjywxk]

There is some GoroutinePool like structures in other packages, how about renaming it to IndexLookupWorkerPool or CopWorkerPool it we do not want it to be a generic used type?

[zyguan]

Actually I prefer to keep it simple and private currently. gp and pool have already been used in tidb, but neither of them meet my requirement. So I just implement a tiny one here and maybe we can extract uniform GoroutinePool interface in the future.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, crazycs520

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:

• OWNERS [cfzjywxk,crazycs520]

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

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2024-12-10 07:47:18.947687577 +0000 UTC m=+338229.036490114: ☑️ agreed by crazycs520.
• 2024-12-13 01:11:51.631861177 +0000 UTC m=+573701.720663705: ☑️ agreed by cfzjywxk.