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

cdc: use for_update_ts to get old value (#9275) #9282

Merged
merged 3 commits into from
Dec 16, 2020
Merged

cdc: use for_update_ts to get old value (#9275) #9282

merged 3 commits into from
Dec 16, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #9275 to release-4.0

Signed-off-by: 5kbpers tangminghua@pingcap.com

What problem does this PR solve?

Problem Summary:
Use start_ts to get old value maybe return a stale old value.

What is changed and how it works?

What's Changed:

Replace with max(for_update_ts, start_ts)

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Release note

  • cdc: fix stale old value with pessimistic lock.

@ti-srebot
cherry pick tikv#9275 to release-4.0
28f5ebd 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added the component/CDC Component: Change Data Capture label Dec 16, 2020
@ti-srebot ti-srebot mentioned this pull request Dec 16, 2020
Merged
@ti-srebot ti-srebot added type/bugfix This PR fixes a bug. type/cherry-pick Type: PR - Cherry pick labels Dec 16, 2020
@ti-srebot ti-srebot added this to the v4.0.9 milestone Dec 16, 2020
@ti-srebot
Copy link
Contributor Author

@5kbpers you're already a collaborator in bot's repo.

resolve conflicts
544c9a1 
Signed-off-by: 5kbpers <tangminghua@pingcap.com>
@MyonKeminta
Copy link
Contributor

LGTM

@ti-srebot
Copy link
Contributor Author

@MyonKeminta, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: migrate(slack).

@overvenus
Copy link
Member

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 16, 2020
make clippy happy
e9ae564 
Signed-off-by: 5kbpers <tangminghua@pingcap.com>
zhouqiang-cl
zhouqiang-cl approved these changes Dec 16, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl 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
Copy link
Contributor Author

@zhouqiang-cl, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: migrate(slack).

amyangfei
amyangfei reviewed Dec 16, 2020
View reviewed changes
Copy link

@amyangfei amyangfei 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 status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 16, 2020
@5kbpers
Copy link
Member

5kbpers commented Dec 16, 2020

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @5kbpers, this branch's release version is in progress, please contact zhouqiang-cl,shuke987,jebter for more details.

@jebter
Copy link
Collaborator

jebter commented Dec 16, 2020

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @jebter, this branch cannot be merged without an approval of release maintainers.

@ti-srebot ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 16, 2020
@jebter
Copy link
Collaborator

jebter commented Dec 16, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 16, 2020
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 2b4e0bf into tikv:release-4.0 Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amyangfei amyangfei amyangfei left review comments

@zhouqiang-cl zhouqiang-cl zhouqiang-cl approved these changes

@5kbpers 5kbpers 5kbpers approved these changes

@MyonKeminta MyonKeminta Awaiting requested review from MyonKeminta

@overvenus overvenus Awaiting requested review from overvenus

Assignees

@5kbpers 5kbpers

Labels
component/CDC Component: Change Data Capture status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/bugfix This PR fixes a bug. type/cherry-pick Type: PR - Cherry pick
Projects
None yet
Milestone
v4.0.9
Development

Successfully merging this pull request may close these issues.
7 participants
@ti-srebot @MyonKeminta @overvenus @5kbpers @jebter @amyangfei @zhouqiang-cl