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

validation(dm): manage validator life cycle #4479

Merged
merged 42 commits into from
Feb 14, 2022
Merged

validation(dm): manage validator life cycle #4479

merged 42 commits into from
Feb 14, 2022

Conversation

D3Hunter
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #4459

What is changed and how it works?

see issue for validator life cycle description

validator is empty in this pr. dmctl command to start/stop validator will be add in later pr

Check List

Tests

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

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to update key monitor metrics in both TiCDC document and official document

Release note 

`None`

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 26, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Ehco1996
  • lance6716

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-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 26, 2022
@lance6716 lance6716 added the area/dm Issues or PRs related to DM. label Jan 26, 2022
@D3Hunter
Copy link
Contributor Author

/run-all-tests

@lance6716 lance6716 changed the title dm,validation: manage validator life cycle validation(dm): manage validator life cycle Jan 26, 2022
@D3Hunter
Copy link
Contributor Author

/run-all-tests

@D3Hunter
Copy link
Contributor Author

/run-all-tests

@D3Hunter
Copy link
Contributor Author

/run-all-tests

@D3Hunter
Copy link
Contributor Author

@lance6716 @Ehco1996 ptal

@D3Hunter
Copy link
Contributor Author

/run-dm-integration-test

Ehco1996
Ehco1996 reviewed Feb 10, 2022
View reviewed changes
Copy link
Contributor

@Ehco1996 Ehco1996 left a comment

Choose a reason for hiding this comment

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

rest LGTM

dm/syncer/data_validator.go Outdated Show resolved Hide resolved
dm/dm/proto/dmworker.proto Show resolved Hide resolved
Ehco1996
Ehco1996 approved these changes Feb 11, 2022
View reviewed changes
Copy link
Contributor

@Ehco1996 Ehco1996 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 added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 11, 2022
@D3Hunter
Copy link
Contributor Author

/run-all-tests

lance6716
lance6716 reviewed Feb 11, 2022
View reviewed changes
Copy link
Contributor

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

rest lgtm

dm/dm/worker/source_worker.go Show resolved Hide resolved
dm/pkg/ha/stage.go
)
if revision > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we always use the latest revision? in future we might decouple from etcd, and revision may not be supported in new metadata storage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in operateSubTaskStage, we need to get validator stage using revision of the subtask.

although is't right to use latest revision(same result), but it's more clear this way.

dm/syncer/data_validator.go Show resolved Hide resolved
@D3Hunter
Copy link
Contributor Author

/run-all-tests

@D3Hunter
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 14, 2022
@D3Hunter
Copy link
Contributor Author

/run-all-tests

@D3Hunter
Copy link
Contributor Author

/run-verify

@D3Hunter
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

@D3Hunter: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

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 ti-community-infra/tichi repository.

@D3Hunter
Copy link
Contributor Author

needs merge @lance6716 @Ehco1996

@lance6716
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 2e4bb60

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 14, 2022
@ti-chi-bot ti-chi-bot merged commit 52f0ade into pingcap:master Feb 14, 2022
@D3Hunter D3Hunter deleted the fast-validator branch February 14, 2022 07:34
zhaoxinyu pushed a commit to zhaoxinyu/ticdc that referenced this pull request Feb 16, 2022
@D3Hunter @zhaoxinyu
validation(dm): manage validator life cycle (pingcap#4479)
182a138 
close pingcap#4459
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@buchuitoudegou buchuitoudegou buchuitoudegou left review comments

@Ehco1996 Ehco1996 Ehco1996 approved these changes

@lance6716 lance6716 lance6716 approved these changes

@niubell niubell Awaiting requested review from niubell

Assignees
No one assigned
Labels
area/dm Issues or PRs related to DM. release-note-none Denotes a PR that doesn't merit a release note. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

manage validation life cycle
6 participants
@D3Hunter @ti-chi-bot @codecov-commenter @lance6716 @Ehco1996 @buchuitoudegou