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

Temporary fix for ErrSnapshotSchemaNotFound and ErrSchemaStorageGCed #1069

Merged
merged 31 commits into from
Nov 24, 2020
Merged

Temporary fix for ErrSnapshotSchemaNotFound and ErrSchemaStorageGCed #1069

merged 31 commits into from
Nov 24, 2020

Conversation

liuzix
Copy link
Contributor

@liuzix liuzix commented Nov 11, 2020 •
edited
Loading

What problem does this PR solve?

  • ErrSnapshotSchemaNotFound or ErrSchemaStorageGCed after table migrations.
  • Some of other potential problems caused by table puller starting from a startTs that is too early.

What is changed and how it works?

  • Fixed bug in the handling of table moves.
  • Mitigated other some situations by resetting startTs of a puller to the latest valid global checkpointTS.
  • Mitigated other some situations by delaying SchemaStorage GC by 20 minutes (in case checkpointTS advanced unexpectedly when puller is starting).
    Note: The last two are ad-hoc temporary fixes, and the problems should be identified exhaustively before making other plans for fixing.

Check List

Tests

  • Integration test
  • Manual test

Side effects

  • Possible performance regression
  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

Release note

  • No release note

@liuzix
Copy link
Contributor Author

liuzix commented Nov 11, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 11, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 11, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 12, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 12, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 12, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 12, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 13, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 13, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 13, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 13, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 13, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 20, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 20, 2020

/run-all-tests

@liuzix
Copy link
Contributor Author

liuzix commented Nov 23, 2020

/run-all-tests

@codecov-io
Copy link

codecov-io commented Nov 23, 2020 •
edited
Loading

Codecov Report

Merging #1069 (15c16d7) into master (0138edc) will increase coverage by 0.0504%.
The diff coverage is 42.0560%.

@@               Coverage Diff                @@
##             master      #1069        +/-   ##
================================================
+ Coverage   37.8006%   37.8510%   +0.0504%     
================================================
  Files           110        110                
  Lines         11394      11466        +72     
================================================
+ Hits           4307       4340        +33     
- Misses         6662       6698        +36     
- Partials        425        428         +3

@liuzix
Copy link
Contributor Author

liuzix commented Nov 23, 2020

/run-all-tests

@zier-one zier-one added this to the v4.0.9 milestone Nov 24, 2020
@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 24, 2020
@zier-one zier-one added the needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. label Nov 24, 2020
@liuzix
Copy link
Contributor Author

liuzix commented Nov 24, 2020

/run-all-tests

overvenus
overvenus approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@overvenus overvenus 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 24, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 24, 2020
@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 Nov 24, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@liuzix merge failed.

@amyangfei
Copy link
Contributor

/run-integration-tests

@amyangfei amyangfei merged commit 6b6b97d into pingcap:master Nov 24, 2020
ti-srebot pushed a commit to ti-srebot/ticdc that referenced this pull request Nov 24, 2020
@liuzix @ti-srebot
cherry pick pingcap#1069 to release-4.0
7125054 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 24, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1114

ti-srebot added a commit that referenced this pull request Nov 24, 2020
@ti-srebot
Temporary fix for ErrSnapshotSchemaNotFound and ErrSchemaStorageGCed (#…
b181f9d 
…1069) (#1114)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@amyangfei amyangfei mentioned this pull request Nov 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@overvenus overvenus overvenus approved these changes

@zier-one zier-one zier-one approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@amyangfei amyangfei Awaiting requested review from amyangfei

Assignees
No one assigned
Labels
component/replica-model Replication model component. component/scheduler TiCDC inner scheduler component. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. 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
Milestone
v4.0.9
Development

Successfully merging this pull request may close these issues.

ErrSnapshotSchemaNotFound or ErrSchemaStorageGCed after migrating tables
6 participants
@liuzix @codecov-io @zier-one @ti-srebot @amyangfei @overvenus