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

cmd: check changefeed start-ts when creating or resuming #1497

Merged
merged 2 commits into from
Mar 12, 2021

Conversation

amyangfei
Copy link
Contributor

What problem does this PR solve?

Add check described in #1150, TODO list item 1

What is changed and how it works?

  • The check is enabled when
    • create a new changefeed, comparing start-ts with current tso.
    • or resume a changefeed, comparing changefeed checkpoint-ts with current tso.
  • The threshold is 1 day

Check List

Tests

  • Unit test
  • Integration test

Release note

  • Add double confirm when creating or resuming changefeed with start-ts or checkpoint-ts 1 day before current ts.

@amyangfei amyangfei added release-blocker This issue blocks a release. Please solve it ASAP. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. labels Mar 11, 2021
@amyangfei amyangfei added this to the v5.0.0 milestone Mar 11, 2021
@amyangfei
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

codecov-io commented Mar 11, 2021

Codecov Report

Merging #1497 (a30673f) into master (4f0e07a) will increase coverage by 0.1716%.
The diff coverage is 50.0000%.

@@               Coverage Diff                @@
##             master      #1497        +/-   ##
================================================
+ Coverage   49.2311%   49.4028%   +0.1716%     
================================================
  Files           150        150                
  Lines         15543      15574        +31     
================================================
+ Hits           7652       7694        +42     
+ Misses         7118       7106        -12     
- Partials        773        774         +1     

@amyangfei amyangfei added the status/ptal Could you please take a look? label Mar 11, 2021
Copy link
Contributor

@zier-one zier-one 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 added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 12, 2021
@liuzix
Copy link
Contributor

liuzix commented Mar 12, 2021

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Mar 12, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Mar 12, 2021
@zier-one
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 12, 2021
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@amyangfei merge failed.

@amyangfei
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit fb8cc9c into pingcap:master Mar 12, 2021
ti-srebot pushed a commit to ti-srebot/ticdc that referenced this pull request Mar 12, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1499

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. release-blocker This issue blocks a release. Please solve it ASAP. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. status/ptal Could you please take a look?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants