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: fix global checkpointts stopping when it 's not exist changefees #496

Merged
merged 4 commits into from
Apr 26, 2020

Conversation

zier-one
Copy link
Contributor

@zier-one zier-one commented Apr 26, 2020

What problem does this PR solve?

If the number of changefeed is zero, checkpoint will not be pushing

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test

@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one zier-one changed the title Fix checkpointts block owner: fix global checkpointts stopping when it 's not exist changefees Apr 26, 2020
@zier-one zier-one marked this pull request as ready for review April 26, 2020 08:29
@zier-one zier-one added the status/ptal Could you please take a look? label Apr 26, 2020
Comment on lines 483 to 486
if len(c.taskPositions) == 0 {
minCheckpointTs = c.status.CheckpointTs
minCheckpointTs = c.status.ResolvedTs
} else if len(c.taskPositions) < len(c.taskStatus) {
return nil
Copy link
Contributor

@amyangfei amyangfei Apr 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this better:

  • check if len(c.taskPositions) < len(c.taskStatus), do nothing
  • then check if len(c.taskPositions) == 0, update minCheckpointTs to c.status.ResolvedTs

If owner has dispatched a table but processor doesn't start replicating it, owner should wait until the processor is ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If owner has dispatched a table but processor doesn't start replicating it, owner should wait until the processor is ready?

Yes, the owner should, and we using table lock to make the owner wait until the processor is ready now

@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one
Copy link
Contributor Author

/run-integration-tests

@codecov-io
Copy link

codecov-io commented Apr 26, 2020

Codecov Report

Merging #496 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master       #496   +/-   ##
===========================================
  Coverage   26.2199%   26.2199%           
===========================================
  Files            61         61           
  Lines          6373       6373           
===========================================
  Hits           1671       1671           
  Misses         4589       4589           
  Partials        113        113           

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

@amyangfei amyangfei added LGT1 and removed status/ptal Could you please take a look? labels Apr 26, 2020
@amyangfei amyangfei merged commit 614bbc2 into pingcap:master Apr 26, 2020
@zier-one zier-one deleted the fix_checkpointts_block branch May 11, 2020 02:29
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants