refine the implementation of IndexMergeReader · pingcap/tidb#14332

(2 comments) (5 reactions) (0 assignees)Go (40,090 stars) (6,186 forks)batch import

help wantedsig/executiontype/enhancement

Description

Feature Request

Is your feature request related to a problem? Please describe:

The implementation of IndexMergeReader is rough now, we can refine it further to make the code more readable and correct.

Describe the feature you'd like:

Remove the useless attribute KeepOrder for IndexMergeReaderExecutor. IndexMerge does not promise the output order and does not need the order of its children now, thus we do not need the KeepOrder attribute now. issue: https://github.com/pingcap/tidb/issues/14339
Remove the attribute indexMergeProcessWorker.maps, we can define this variable as a local variable in indexMergeProcessWorker.fetchLoop. issue: https://github.com/pingcap/tidb/issues/14339
Refine the IPC between partialWorker and indexMergeProcessWorker. indexMergeProcessWorker.fetchLoop use partialWorkerCnt-- to check whether all the partialWorkers finishing their work. We can use a WaitGroup to synchronize the work between all the partial workers, and start a goroutine which waits for the WaitGroup, and close the fetchCh when all the partialWorkers exit. When indexMergeProcessWorker fails to fetch data from fetchCh, it knows all the partialWorkers exit. Refer this issue: https://github.com/pingcap/tidb/issues/14333
Runtime stats and feedback are not handled correctly during indexMergeReaderExecutor, we need to make them work. https://github.com/pingcap/tidb/issues/14334
We can use WithRecovery instead of the way used to recover to make the code more readable. Refer this. issue: https://github.com/pingcap/tidb/issues/14335
Start all the workers for the first time Next is called instead Open. issue: https://github.com/pingcap/tidb/issues/14336
Send the result chk back to indexMergeProcessWorker to save the memory usage. Refer this issue: https://github.com/pingcap/tidb/issues/14337
Send a batch of handles to the indexMergeTableScanWorkers instead of the handles of a task which is filtered by indexMergeProcessWorker.maps. issue: https://github.com/pingcap/tidb/issues/14338

Contributor guide

Tech stack: go
Domain: backenddatabase
Issue type: refactor
Difficulty: 4
Estimated time: 3-5 days
Activity status: stale
Clarity: clear
Prerequisites: Goconcurrencydatabase internals
Newbie friendliness: 20
Research direction: Review the existing implementation in executor/index merge reader.go and related files. Address the unchecked tasks: implement proper runtime stats and feedback handling (see issue #14334), reduce memory usage by sending result chunks back (see #14337), and batch handle sending (see #14338). Use the patterns from executor/join.go as references. Consult the linked issues for details.