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

executor, util: fix spilling disk when oom (#16895) #18288

Merged
merged 2 commits into from
Jul 2, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #16895 to release-4.0


What problem does this PR solve?

Issue Number: close #16696

Problem Summary: RowContainer can't spill disk sometimes.

What is changed and how it works?

What's Changed:

  1. Add a lock in rowContainer, for Get/Spill mutually exclusive.
  2. Add an atomic variable in Tracker.actionMu, to avoid deadlock.

How it Works:

Related changes

  • Need to cherry-pick to the release branch 4.0

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Performance regression
    • Consumes more CPU

Release note

  • [Bug-fix] Make spilling disk work well.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@wshwsh12 please accept the invitation then you can push to the cherry-pick pull requests.
https://github.com/ti-srebot/tidb/invitations

@wshwsh12
Copy link
Contributor

/run-all-tests

@SunRunAway SunRunAway added status/all-tests-passed and removed priority/release-blocker This PR blocks a release. Please review it ASAP. labels Jun 30, 2020
@SunRunAway SunRunAway modified the milestones: v4.0.2, v4.0.3 Jun 30, 2020
@DanielZhangQD
Copy link

/run-build-image

@XuHuaiyu XuHuaiyu changed the title executor, util: fix spilling disk when oom. (#16895) executor, util: fix spilling disk when oom (#16895) Jul 1, 2020
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor Author

@XuHuaiyu,Thanks for you review.

@XuHuaiyu XuHuaiyu added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 1, 2020
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

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jul 1, 2020
@ti-srebot
Copy link
Contributor Author

@SunRunAway,Thanks for you review.

@SunRunAway
Copy link
Contributor

@ti-srebot /merge

@SunRunAway
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 2, 2020
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@SunRunAway SunRunAway merged commit 6669399 into pingcap:release-4.0 Jul 2, 2020
@SunRunAway SunRunAway deleted the release-4.0-9d9f330a4b9d branch July 2, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/util priority/P2 sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. type/bug-fix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants