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

ddl, parser, planner: admin Pause/Resume on DDL jobs #43081

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

dhysum
Copy link
Contributor

@dhysum dhysum commented Apr 15, 2023

Add Pause/Resume admin command in parser and planner

What problem does this PR solve?

Issue Number: close #18015 #40041

Related issue number: #39751 #43061

Problem Summary:

What is changed and how it works?

  1. Add admin pause ddl jobs 3,5;
    a. the job should be in Running or Queueing state
    b. change the state to be Pausing on command issued, in another PR
    c. background work will pause the job once scheduled, in another PR

  2. Add admin resume ddl jobs 3,5;
    a. the job should be paused
    b. change the state to be Queueing on command issued, in another PR
    c. background worker will continue the job, no need to change

Check List

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

- Add two commands `admin pause ddl jobs 3,5;` and `admin resume ddl jobs 3,5;`
- Add two state of the job, Pausing and Paused
- The job in Running / Queueing could be transferred to be Pausing by `admin pause ddl jobs 3,5;`, then be Paused by background worker
- Only Paused job can be resumed to be Queueing by `admin pause ddl jobs 3,5;
- New error code added: ErrCannotPauseDDLJob  = 9014, ErrCannotResumeDDLJob = 9015, ErrPausedDDLJob       = 9016

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Apr 15, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Benjamin2037
  • zimulala

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/invalid-title labels Apr 15, 2023
@ti-chi-bot
Copy link
Member

Welcome @dhysum!

It looks like this is your first PR to pingcap/tidb 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/tidb. 😃

@ti-chi-bot ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Apr 15, 2023
@ti-chi-bot
Copy link
Member

Hi @dhysum. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sre-bot
Copy link
Contributor

sre-bot commented Apr 15, 2023

CLA assistant check
All committers have signed the CLA.

@Benjamin2037
Copy link
Collaborator

/ok-to-test

@ti-chi-bot ti-chi-bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Apr 15, 2023
@dhysum dhysum changed the title Parser/Executor change for DDL Pause/Resume #18015 #40041 ddl, planner, parser: Parser/Executor change for DDL Pause/Resume Apr 15, 2023
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch 6 times, most recently from 4625224 to d2fd020 Compare April 17, 2023 11:17
@dhysum
Copy link
Contributor Author

dhysum commented Apr 17, 2023

/ok-to-test

@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch 3 times, most recently from 319f718 to 098831a Compare April 17, 2023 11:42
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2023
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch 2 times, most recently from 85e2051 to 554fa18 Compare April 18, 2023 02:10
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2023
@dhysum
Copy link
Contributor Author

dhysum commented Apr 18, 2023

/retest

@dhysum dhysum changed the title ddl, planner, parser: Parser/Executor change for DDL Pause/Resume parser, planner, executor: admin Pause/Resume on DDL jobs Apr 18, 2023
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch from 554fa18 to fabb2f3 Compare April 18, 2023 03:21
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch 3 times, most recently from 846878e to 38ed93b Compare April 26, 2023 06:22
planner/core/common_plans.go Outdated Show resolved Hide resolved
planner/core/common_plans.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
parser/model/ddl.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 26, 2023
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch from 38ed93b to e8fd4f9 Compare April 27, 2023 01:55
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2023
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch from e8fd4f9 to a76f1c0 Compare April 27, 2023 02:39
@dhysum dhysum force-pushed the dev/job_pause_resume_parser branch from a76f1c0 to 40dee77 Compare April 27, 2023 02:42
Copy link
Contributor

@zimulala zimulala 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-chi-bot ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 28, 2023
@zimulala
Copy link
Contributor

/merge

@ti-chi-bot
Copy link

ti-chi-bot bot commented Apr 28, 2023

This pull request has been accepted and is ready to merge.

Commit hash: 40dee77

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 28, 2023
@ti-chi-bot ti-chi-bot bot merged commit d95ce84 into pingcap:master Apr 28, 2023
@Benjamin2037 Benjamin2037 added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label May 4, 2023
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.1: #43511.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. ok-to-test Indicates a PR is ready to be tested. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pause add an index(write reorganization) and set a time window to resume
5 participants