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

util: wrong memory count when use Tracker.ReplaceChild() (#14513) #14533

Merged
merged 3 commits into from
Jan 19, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Jan 19, 2020

cherry-pick #14513 to release-3.0


What problem does this PR solve?

When we use Tracker.ReplaceChild() to replace/remove child, is only updates the result for the parent, but not updates the nodes in the path of the tracker tree recursively.

What is changed and how it works?

Use t.Consume(bytes) to update the result.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch 3.0

Release note

Fix a potential memory count problem.

@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

/run-all-tests

Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added the status/can-merge Indicates a PR has been approved by a committer. label Jan 19, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

Sorry @SunRunAway, you don't have permission to trigger auto merge event on this branch.

@SunRunAway SunRunAway added status/can-merge Indicates a PR has been approved by a committer. and removed status/can-merge Indicates a PR has been approved by a committer. labels Jan 19, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

Sorry @SunRunAway, you don't have permission to trigger auto merge event on this branch.

@SunRunAway SunRunAway added status/can-merge Indicates a PR has been approved by a committer. and removed status/can-merge Indicates a PR has been approved by a committer. labels Jan 19, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

/run-all-tests

@SunRunAway SunRunAway added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. and removed status/can-merge Indicates a PR has been approved by a committer. labels Jan 19, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

/run-all-tests

Copy link
Contributor

@you06 you06 left a comment

Choose a reason for hiding this comment

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

LGTM

@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

@sre-bot merge failed.

@you06
Copy link
Contributor

you06 commented Jan 19, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

Sorry @you06, you don't have permission to trigger auto merge event on this branch.

@you06
Copy link
Contributor

you06 commented Jan 19, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

Your auto merge job has been accepted, waiting for 14536

@sre-bot
Copy link
Contributor Author

sre-bot commented Jan 19, 2020

/run-all-tests

@sre-bot sre-bot merged commit 82b8b7a into pingcap:release-3.0 Jan 19, 2020
@bb7133 bb7133 added this to the v3.0.10 milestone Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/util status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants