executor: tiny optimize index-lookup query performance by reuse lite-cop-worker. #58586
Conversation
Signed-off-by: crazycs520 <[email protected]>
crazycs520
added
the
do-not-merge/work-in-progress
ti-chi-bot
bot
added
release-note-none
|
Hi @crazycs520. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with 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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #58586 +/- ##
================================================
+ Coverage 73.5297% 74.0188% +0.4890%
================================================
Files 1680 1680
Lines 464567 469898 +5331
================================================
+ Hits 341595 347813 +6218
+ Misses 102120 101239 -881
+ Partials 20852 20846 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
crazycs520
changed the title
Signed-off-by: crazycs520 <[email protected]>
ti-chi-bot
bot
added
size/S
Signed-off-by: crazycs520 <[email protected]>
| // Only one cop-reader can use lite worker. Using lite-worker in multiple readers will affect the concurrent execution of readers. | ||
| TryCopLiteWorker uint32 | ||
| // Only one cop-reader can use lite worker at the same time. Using lite-worker in multiple readers will affect the concurrent execution of readers. | ||
| TryCopLiteWorker atomic.Uint32 |
There was a problem hiding this comment.
Can't use sync/atomic here, since
nogo: errors found by nogo during build-time code analysis:
pkg/distsql/context/context.go:105:12: assignment copies lock value to newCtx: github.com/pingcap/tidb/pkg/distsql/context.DistSQLContext contains sync/atomic.Uint32 contains sync/atomic.noCopy (copylocks)Signed-off-by: crazycs520 <[email protected]>
Signed-off-by: crazycs520 <[email protected]>
| @@ -1195,6 +1198,10 @@ func (w *liteCopIteratorWorker) liteSendReq(ctx context.Context, it *copIterator | |||
| } else { | |||
| it.tasks = it.tasks[1:] | |||
| } | |||
| if len(it.tasks) == 0 { | |||
| // if all tasks are finished, reset tryCopLiteWorker to 0 to make future request can reuse copLiteWorker. | |||
| w.tryCopLiteWorker.Store(0) | |||
There was a problem hiding this comment.
Should CAS operation be used here too?
There was a problem hiding this comment.
There's no need, just reset here.
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, hawkingrei, zyguan 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:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: ref #56649
Problem Summary:
What changed and how does it work?
Try to reuse lite-cop-worker.
Before this PR
This PR, the
tableWorkerofIndexLookUpExecutorcan use lite-cop-worker to reduce consumption.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.