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

processor: fix task status is always flushed because of incorrect mod revision cache #1017

Merged

Conversation

amyangfei
Copy link
Contributor

@amyangfei amyangfei commented Oct 22, 2020

What problem does this PR solve?

After investigating the #993, found key /tidb/cdc/task/status/<capture-id>/<changefeed-id> is always updated when processor calls flushTaskStatusAndPosition

This bug happens since v4.0.5

What is changed and how it works?

The fix is straightforward, check newModRevision returned by AtomicPutTaskStatus, newModRevision=0 means status is not updated, we don't update the mod revision cache.

Check List

Tests

  • Unit test
  • Integration test

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Fix a bug that processor always flushes task status to the etcd because of incorrect mod revision cache.

@amyangfei amyangfei added type/bugfix This PR fixes a bug. 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 Oct 22, 2020
@amyangfei amyangfei added this to the v4.0.8 milestone Oct 22, 2020
@amyangfei
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

Codecov Report

Merging #1017 into master will decrease coverage by 0.4533%.
The diff coverage is 0.0000%.

@@               Coverage Diff                @@
##             master      #1017        +/-   ##
================================================
- Coverage   32.1359%   31.6826%   -0.4534%     
================================================
  Files           101        101                
  Lines         11358      11066       -292     
================================================
- Hits           3650       3506       -144     
+ Misses         7297       7167       -130     
+ Partials        411        393        -18     

@amyangfei amyangfei added the status/ptal Could you please take a look? label Oct 22, 2020
@zier-one
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 22, 2020
@liuzix
Copy link
Contributor

liuzix commented Oct 22, 2020

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 Oct 22, 2020
@amyangfei
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 22, 2020
@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
Copy link
Contributor

@amyangfei merge failed.

@amyangfei
Copy link
Contributor Author

/merge

3 similar comments
@zier-one
Copy link
Contributor

/merge

@zier-one
Copy link
Contributor

/merge

@zier-one
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@zier-one
Copy link
Contributor

/merge

@amyangfei amyangfei merged commit f46e812 into pingcap:master Oct 22, 2020
@amyangfei amyangfei deleted the fix-task-status-update-too-frequent branch October 22, 2020 12:04
ti-srebot pushed a commit to ti-srebot/ticdc that referenced this pull request Oct 22, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1019

ti-srebot added a commit that referenced this pull request Oct 22, 2020
… revision cache (#1017) (#1019)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@amyangfei amyangfei added the priority/p0 Issue with the highest priority, should be resolved as soon as possible. label Mar 22, 2021
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. priority/p0 Issue with the highest priority, should be resolved as soon as possible. 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? type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants