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

TiCDC supports a snapshot-map in replication #932

Merged
merged 38 commits into from
Oct 13, 2020
Merged

Conversation

Colins110
Copy link
Contributor

@Colins110 Colins110 commented Sep 7, 2020

What problem does this PR solve?

TiDB Cluster supports snapshot level consistency replication in time

What is changed and how it works?

  1. Make MySQL sink supports to replicate to a global consistent state dynamically.
    set a syncpoint at regular intervals for get globally consistent state;
    by stopping updating the global resolvedTS util the global checkpointTS == global resolvedTS, then set the sync point TS
    =checkpointTS;

  2. Store a safepoint mapping in downstream, recording the TSO mapping between upstream and downstream.
    the map in the downstream like this:

cf primary_ts secondary_ts
sample 419288824469258241 419288824566251551
sample 419288981663907844 419288981957509121

Check List

Tests

  • Manual test
  • Integration Test
    a integration test named syncpoint test the syncpoint's correctness by sync_diff_inspector tool. you can use make integration_test CASE=syncpoint to run the test.

Code changes

in cdc/changeFeed.go:
add function:
startSyncPeriod()
createSynctable()
sinkSyncpoint(ctx context.Context)

add some property in changeFeed struct

some change in function calcResolvedTs(ctx context.Context):
to judge the syncpoint and sink the map to downstream

in cdc/owner.go:
add function:
configureSinkURI(ctx context.Context, dsnCfg *dmysql.Config, tz *time.Location)
createSyncpointSink(ctx context.Context, info *model.ChangeFeedInfo, id string)

some change in function newChangeFeed( ctx context.Context, id model.ChangeFeedID, processorsInfos model.ProcessorsInfos, taskPositions map[string]*model.TaskPosition, info *model.ChangeFeedInfo, checkpointTs uint64):
for every changefeed,we need create a standalone mysql link to record the syncpoint map in the future.

the new flags for cdc cli changefeed:
you can open the feature to record the syncpoint like this when you create a new changefeed or update a changefeed:

  1. set on create
    cdc changefeed create --sink-uri="$SINK_URI" --sync-point --sync-interval=10s
  2. set on update
cdc cli changefeed pause -c test-cf
cdc cli changefeed update -c test-cf --sink-uri="mysql://127.0.0.1:3306/" --sync-point --sync-interval=10m
cdc cli changefeed resume -c test-cf

the --sync-point mean open the syncpoint feature, the --sync-interval can set the interval for recording the sync point(the default value is 10min).

Release note

  • Support snapshot level consistency replication in time

@CLAassistant
Copy link

CLAassistant commented Sep 7, 2020

CLA assistant check
All committers have signed the CLA.

cdc/changefeed.go Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
@amyangfei amyangfei added this to the v5.0.0-alpha milestone Sep 8, 2020
@amyangfei amyangfei added component/replica-model Replication model component. component/sink Sink component. labels Sep 8, 2020
@liuzix
Copy link
Contributor

liuzix commented Sep 8, 2020

@Colins110 Could you explain more about how a user would make use of this feature? What is the potential user scenario and how would the user make use of the "safepoint mapping"?

@Colins110 Colins110 closed this Sep 13, 2020
@Colins110 Colins110 reopened this Sep 13, 2020
@Colins110
Copy link
Contributor Author

@Colins110 Could you explain more about how a user would make use of this feature? What is the potential user scenario and how would the user make use of the "safepoint mapping"?

you can refer to this issue: TiDB Cluster supports snapshot level consistency replication in time

cdc/owner.go Outdated Show resolved Hide resolved
cdc/changefeed.go Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cmd/client_changefeed.go Outdated Show resolved Hide resolved
tests/syncpoint/run.sh Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
@amyangfei
Copy link
Contributor

/run-integration-tests tidb=release-4.0 tikv=release-4.0 pd=release-4.0

cdc/changefeed.go Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
cdc/changefeed.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #932 into master will decrease coverage by 0.6336%.
The diff coverage is 1.9607%.

@@               Coverage Diff                @@
##             master       #932        +/-   ##
================================================
- Coverage   32.1982%   31.5646%   -0.6337%     
================================================
  Files           100        101         +1     
  Lines         10836      11025       +189     
================================================
- Hits           3489       3480         -9     
- Misses         6956       7152       +196     
- Partials        391        393         +2     

cdc/owner.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
cdc/sink/mysql.go Outdated Show resolved Hide resolved
cdc/sink/mysql.go Outdated Show resolved Hide resolved
cdc/sink/mysql.go Outdated Show resolved Hide resolved
@ti-srebot
Copy link
Contributor

@Colins110,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).

@@ -333,13 +333,13 @@ func (o *Owner) newChangeFeed(
}
errCh := make(chan error, 1)

sink, err := sink.NewSink(ctx, id, info.SinkURI, filter, info.Config, info.Opts, errCh)
primarySink, err := sink.NewSink(ctx, id, info.SinkURI, filter, info.Config, info.Opts, errCh)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
primarySink, err := sink.NewSink(ctx, id, info.SinkURI, filter, info.Config, info.Opts, errCh)
sink, err := sink.NewSink(ctx, id, info.SinkURI, filter, info.Config, info.Opts, errCh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should use package sink in L360 (var syncpointStore sink.SyncpointStore) to create a syncpointStore,if the var named sink,the package sink will not be used in the follow code,there is a conflict.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move the sink.SyncpointStore to another package is better, but it reuses some code in the sink package.
We can refine this in another PR.

cdc/sink/mysql.go Outdated Show resolved Hide resolved
@amyangfei
Copy link
Contributor

/run-integration-tests

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 Oct 12, 2020
@zier-one
Copy link
Contributor

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 12, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 12, 2020
@amyangfei
Copy link
Contributor

/merge

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

/run-all-tests

@ti-srebot
Copy link
Contributor

@Colins110 merge failed.

@amyangfei
Copy link
Contributor

/run-integration-tests

2 similar comments
@amyangfei
Copy link
Contributor

/run-integration-tests

@amyangfei
Copy link
Contributor

/run-integration-tests

@amyangfei amyangfei merged commit 3c08cc7 into pingcap:master Oct 13, 2020
@IANTHEREAL IANTHEREAL changed the title TiDB Cluster supports snapshot level consistency replication in time TiCDC supports a snapshot-map in replication Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/replica-model Replication model component. component/sink Sink component. 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?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants