util: improve TopSQL reporter memory efficiency #25195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ti-chi-bot
added
the
size/M
|
/cc @breeswish @crazycs520 |
crazycs520
reviewed
Jun 7, 2021
ti-chi-bot
added
size/L
crazycs520
reviewed
Jun 7, 2021
Co-authored-by: crazycs <chen.two.cs@gmail.com>
breezewish
reviewed
Jun 7, 2021
util/topsql/reporter/client.go
Outdated
| @@ -28,7 +28,7 @@ import ( | |||
|
|
|||
| // ReportClient send data to the target server. | |||
| type ReportClient interface { | |||
| Send(ctx context.Context, addr string, sqlMetas []*tipb.SQLMeta, planMetas []*tipb.PlanMeta, records []*tipb.CPUTimeRecord) error | |||
| Send(ctx context.Context, addr string, data reportData, decodePlan planBinaryDecodeFunc) error | |||
There was a problem hiding this comment.
decodePlan should be a ReportClient constructor parameter.
breezewish
added
the
do-not-merge/hold
Signed-off-by: crazycs <crazycs520@gmail.com>
…s is instance Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
|
/run-unit-test |
github-actions
bot
added
sig/execution
Signed-off-by: crazycs <crazycs520@gmail.com>
|
/run-all-tests |
2 similar comments
|
/run-all-tests |
|
/run-all-tests |
breezewish
approved these changes
Jun 10, 2021
breezewish
added
needs-cherry-pick-release-5.1
and removed
do-not-merge/hold
crazycs520
force-pushed
the
improve-topsql-mem
branch
from
ae23b9d
to
1d5bede
Compare
crazycs520
force-pushed
the
improve-topsql-mem
branch
from
5cc65cc
to
7dbd857
Compare
Signed-off-by: crazycs <crazycs520@gmail.com>
|
/run-all-tests |
Signed-off-by: crazycs <crazycs520@gmail.com>
…into improve-topsql-mem
|
/run-all-tests |
|
/run-all-tests |
breezewish
approved these changes
Jun 11, 2021
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 1ab86ae
|
ti-chi-bot
added
the
status/can-merge
|
/merge |
|
/run-all-tests |
ti-srebot
pushed a commit
to ti-srebot/tidb
that referenced
this pull request
Jun 11, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-5.1 in PR #25363 |
ti-chi-bot
pushed a commit
that referenced
this pull request
Jun 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Problem Summary:
The current TopSQL reporter copies data before sending to gRPC endpoint.
What is changed and how it works?
What's Changed:
Iterate data at send time to reduce a memory copy.
How it Works:
Because the SQL/plan meta could be > 1M for each record, this could potentially save some GB of memory.
Related changes
Check List
Tests
Side effects
None.
Release note