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.

Sign up for GitHub

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

Jump to bottom

planner, executor: support Change and ChangeExec #9789

Merged
merged 46 commits into from
Mar 21, 2019
Merged

planner, executor: support Change and ChangeExec #9789

merged 46 commits into from
Mar 21, 2019

Conversation

aliiohs
Copy link
Contributor

@aliiohs aliiohs commented Mar 19, 2019 •
edited by WangXiangUSTC
Loading

What problem does this PR solve?

support update pump or drainer status.
proposal: #9201

What is changed and how it works?

  1. support sql like "change drainer to node_state='paused' for NodeID '127.0.0.1:8250';"
    already do in Support update pump or drainer status parser#243
  2. update parser's version
  3. handle changestmt, update pump/drainer's status in pd.(tip: can't pause/offline pump/drainer, just modify status in pd)

Check List

Tests

  • Manual test (it is difficult to create fork status information in pd in unit test now, so just have manual test, execute sql change pump/drainer's status, and show pump/drainer's status to check it)

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@codecov
Copy link

codecov bot commented Mar 19, 2019 •
edited
Loading

Codecov Report

Merging #9789 into master will decrease coverage by 0.0185%.
The diff coverage is 0%.

@@               Coverage Diff               @@
##            master      #9789        +/-   ##
===============================================
- Coverage   67.143%   67.1244%   -0.0186%     
===============================================
  Files          381        382         +1     
  Lines        80056      80090        +34     
===============================================
+ Hits         53752      53760         +8     
- Misses       21507      21535        +28     
+ Partials      4797       4795         -2

WangXiangUSTC
WangXiangUSTC reviewed Mar 19, 2019
View reviewed changes
executor/change.go Outdated Show resolved Hide resolved
@WangXiangUSTC WangXiangUSTC added contribution This PR is from a community contributor. component/binlog labels Mar 19, 2019
alivxxx
alivxxx reviewed Mar 19, 2019
View reviewed changes
executor/builder.go Show resolved Hide resolved
executor/change.go Show resolved Hide resolved
executor/change.go Show resolved Hide resolved
executor/change.go Outdated Show resolved Hide resolved
executor/change.go Outdated Show resolved Hide resolved
planner/core/planbuilder.go Show resolved Hide resolved
@aliiohs
Copy link
Contributor Author

aliiohs commented Mar 19, 2019

i don't know how to add a test case for this, but @WangXiangUSTC already Manually tested successfully。

@WangXiangUSTC
Copy link
Contributor

/run-all-tests

alivxxx
alivxxx reviewed Mar 20, 2019
View reviewed changes
executor/change.go Outdated Show resolved Hide resolved
executor/change.go Outdated Show resolved Hide resolved
executor/change.go Outdated Show resolved Hide resolved
executor/change.go Outdated Show resolved Hide resolved
planner/core/common_plans.go Outdated Show resolved Hide resolved
alivxxx
alivxxx reviewed Mar 20, 2019
View reviewed changes
Copy link
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 20, 2019
@WangXiangUSTC
Copy link
Contributor

/run-all-tests

@WangXiangUSTC
Copy link
Contributor

LGTM

@WangXiangUSTC WangXiangUSTC force-pushed the Support_update_pump_or_drainer_status_lastest branch from ae18f5c to 39bd4dc Compare March 21, 2019 07:48
@WangXiangUSTC
Copy link
Contributor

/run-all-tests

XuHuaiyu
XuHuaiyu reviewed Mar 21, 2019
View reviewed changes
executor/change.go Outdated

import (
"context"
"github.com/pingcap/parser/ast"
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this import down.

@WangXiangUSTC WangXiangUSTC force-pushed the Support_update_pump_or_drainer_status_lastest branch from bcdb0a4 to 7fe3ad1 Compare March 21, 2019 09:18
@WangXiangUSTC
Copy link
Contributor

/run-all-tests

1 similar comment
@WangXiangUSTC
Copy link
Contributor

/run-all-tests

XuHuaiyu
XuHuaiyu approved these changes Mar 21, 2019
View reviewed changes
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

@WangXiangUSTC WangXiangUSTC added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 21, 2019
@WangXiangUSTC WangXiangUSTC merged commit dd3ca22 into pingcap:master Mar 21, 2019
@WangXiangUSTC WangXiangUSTC deleted the Support_update_pump_or_drainer_status_lastest branch March 21, 2019 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alivxxx alivxxx alivxxx left review comments

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

@zz-jason zz-jason zz-jason left review comments

@XuHuaiyu XuHuaiyu XuHuaiyu approved these changes

Assignees
No one assigned
Labels
component/binlog contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aliiohs @WangXiangUSTC @zz-jason @XuHuaiyu @alivxxx