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 flushed too many before table is initialized (#1190) #1191

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Dec 10, 2020

cherry-pick #1190 to release-4.0


What problem does this PR solve?

Fix https://github.com/pingcap/ticdc/issues/1188. This is a hotfix for current codebase, we should process with the Dirty field of task status carefully.

What is changed and how it works?

change flush check condition

Check List

Tests

  • Unit test

  • Integration test

  • Manual test (add detailed scripts or steps below)

    • hack puller p.tsTracker.Frontier() to force no forward to simulate long time incremental scan.
    • watch the task status key version in etcd doesn't change.

Release note

  • Fix task status flushed to etcd too many before table is initialized

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

Codecov Report

Merging #1191 (cfd19ae) into release-4.0 (ba68bc1) will decrease coverage by 0.0798%.
The diff coverage is 0.0000%.

@@                 Coverage Diff                 @@
##           release-4.0      #1191        +/-   ##
===================================================
- Coverage      39.8383%   39.7584%   -0.0799%     
===================================================
  Files              112        112                
  Lines            11750      11756         +6     
===================================================
- Hits              4681       4674         -7     
- Misses            6597       6610        +13     
  Partials           472        472                

Copy link
Contributor

@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 the status/LGT1 Indicates that a PR has LGTM 1. label Dec 10, 2020
@amyangfei amyangfei modified the milestones: v4.0.10, v4.0.9 Dec 10, 2020
@liuzix
Copy link
Contributor

liuzix commented Dec 10, 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 Dec 10, 2020
@amyangfei amyangfei merged commit 402379a into pingcap:release-4.0 Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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