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

owner: check update safepoint error (#1282) #1367

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Jan 28, 2021

cherry-pick #1282 to release-4.0
You can switch your code base to this Pull Request by using git-extras:

# In ticdc repo:
git pr https://github.com/pingcap/ticdc/pull/1367

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/ticdc.git pr/1367:release-4.0-9817ce8f1099

What problem does this PR solve?

  • CDC does not fail in time after service safepoint is lost.
  • CDC sometimes returns TiKV errors to the user, which is confusing.

What is changed and how it works?

  • Owner now checks whether updating service safepoint is successful, and, if not, throws the appropriate error.
  • Wrapping TiKV errors with appropriate CDC errors.

Check List

Tests

  • Integration test

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release note

  • Check update safepoint error.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added type/bugfix This PR fixes a bug. component/replica-model Replication model component. status/ptal Could you please take a look? type/4.0-cherry-pick labels Jan 28, 2021
@ti-srebot ti-srebot added this to the v4.0.11 milestone Jan 28, 2021
@liuzix
Copy link
Contributor

liuzix commented Jan 28, 2021

LGTM

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

LGTM

@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 Jan 28, 2021
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 1366

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (release-4.0@26748b3). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             release-4.0      #1367   +/-   ##
================================================
  Coverage               ?   47.9520%           
================================================
  Files                  ?        130           
  Lines                  ?      13184           
  Branches               ?          0           
================================================
  Hits                   ?       6322           
  Misses                 ?       6181           
  Partials               ?        681           

@ti-srebot ti-srebot merged commit e3b93d6 into pingcap:release-4.0 Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/replica-model Replication model component. 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? type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants