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

config: support temporary storage usage limitation #15700

Merged
merged 50 commits into from
Apr 8, 2020

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Mar 25, 2020

What problem does this PR solve?

Issue Number: close #13983
UCP: #13983

Problem Summary:

What is changed and how it works?

How it Works:
Add TempStorageQuota(bytes) in config which describe the temporary storage quota for the tidb-server.
If the quota exceed the capacity of the target directory which defined by TempStoragePath, the server would exit with the fatal error.

And we use a GlobalDiskTracker to track the disk usage for the executor, if the executor exceeds its disk quota, the query would be killed with Out of Global Storage.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Release note

Support temporary storage usage limitation for tidb-server

@sre-bot
Copy link
Contributor

sre-bot commented Mar 25, 2020

@Yisaer, you already got 250 points from easy level tasks when all pull requests merged. And you will not get score from this PR.

@sre-bot
Copy link
Contributor

sre-bot commented Mar 25, 2020

Thanks for your contribution. If your PR get merged, you will be rewarded 50 points.

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.

Hi,
we need to control the temporary storage usage of total queries to prevent tidb-server consuming all the disk space.

config/config.go Outdated Show resolved Hide resolved
config/config.toml.example Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.toml.example Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #15700 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15700   +/-   ##
===========================================
  Coverage   80.4050%   80.4050%           
===========================================
  Files           505        505           
  Lines        135729     135729           
===========================================
  Hits         109133     109133           
  Misses        18100      18100           
  Partials       8496       8496           

@Yisaer Yisaer requested a review from a team as a code owner March 26, 2020 13:43
@ghost ghost requested review from qw4990 and removed request for a team March 26, 2020 13:43
@github-actions github-actions bot added the sig/execution SIG execution label Mar 26, 2020
@Yisaer

This comment has been minimized.

@Yisaer
Copy link
Contributor Author

Yisaer commented Mar 31, 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

@qw4990 qw4990 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/WIP labels Apr 8, 2020
qw4990
qw4990 previously approved these changes Apr 8, 2020
@qw4990 qw4990 added the status/can-merge Indicates a PR has been approved by a committer. label Apr 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

@Yisaer merge failed.

@Yisaer
Copy link
Contributor Author

Yisaer commented Apr 8, 2020

/run-all-tests

1 similar comment
@Yisaer
Copy link
Contributor Author

Yisaer commented Apr 8, 2020

/run-all-tests

@Yisaer
Copy link
Contributor Author

Yisaer commented Apr 8, 2020

/run-integration-copr-test

@SunRunAway
Copy link
Contributor

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

Your auto merge job has been accepted, waiting for 16130

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

/run-all-tests

@sre-bot sre-bot merged commit d8978cb into pingcap:master Apr 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

Team Yisaer complete task #13983 and get 300 score, currerent score 300

@SunRunAway SunRunAway added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-cherry-pick-4.0 labels Apr 8, 2020
@SunRunAway
Copy link
Contributor

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-4.0 in PR #16211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config release-note Denotes a PR that will be considered when it comes time to generate release notes. 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/new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

temporary storage usage limitation of total queries.
4 participants