Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

session, executor: add memTracker for DeleteExec #14289

Merged
merged 6 commits into from
Dec 31, 2019

Conversation

XuHuaiyu
Copy link
Contributor

What problem does this PR solve?

Before this PR, the memory usage of DeleteExec is not traced.

What is changed and how it works?

Add a memTracker for DeleteExec, to trace the memory usage of chk created during execution, and the memory usage of StmtBuffer and TxnBuffer.

Check List

Tests

  • Integration test

Code changes

  • Has exported function/method change

Side effects

N/A

Related changes

N/A

Release note

N/A

@XuHuaiyu XuHuaiyu added type/enhancement The issue or PR belongs to an enhancement. sig/execution SIG execution labels Dec 31, 2019
@XuHuaiyu XuHuaiyu force-pushed the memtracker_for_delete branch from 706befa to 9fa2f04 Compare December 31, 2019 02:56
@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu XuHuaiyu requested a review from tiancaiamao December 31, 2019 03:30
executor/delete.go Outdated Show resolved Hide resolved
executor/delete.go Outdated Show resolved Hide resolved
executor/delete.go Show resolved Hide resolved
if err != nil {
}
memUsageOfTxnState := txnState.Size()
err = t.RemoveRecord(ctx, h, data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only one line change, is it easier to estimate the size of data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the existing interface is easier?

session/txn.go Show resolved Hide resolved
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 31, 2019
@XuHuaiyu XuHuaiyu requested a review from lysu December 31, 2019 07:06
lysu
lysu previously approved these changes Dec 31, 2019
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lysu lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 31, 2019
@XuHuaiyu XuHuaiyu added the status/can-merge Indicates a PR has been approved by a committer. label Dec 31, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 31, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Dec 31, 2019

@XuHuaiyu merge failed.

@XuHuaiyu
Copy link
Contributor Author

/run-unit-test

@tiancaiamao tiancaiamao merged commit a7ff591 into pingcap:master Dec 31, 2019
@XuHuaiyu XuHuaiyu deleted the memtracker_for_delete branch December 31, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants