processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush #3540

overvenus · 2021-11-19T09:55:07Z

What problem does this PR solve?

Revert #3029 and #2617 temporary for resolving #3503.

Cc https://github.com/pingcap/ticdc/issues/3545

Check List

Tests

Integration test

See #3503

Release note

None

ti-chi-bot · 2021-11-19T09:55:08Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

3AceShowHand
amyangfei

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

liuzix · 2021-11-19T10:41:45Z

/run-integration-tests

Note this is a workaround that reduces the probability of pingcap#3503, sink may still reports a checkpoint that larger than resolved ts, and may cause data lose and changefeed stuck. Signed-off-by: Neil Shen <overvenus@gmail.com>

Signed-off-by: Neil Shen <overvenus@gmail.com>

liuzix · 2021-11-20T06:02:40Z

cdc/owner/changefeed.go

@@ -391,6 +391,10 @@ func (c *changefeed) handleBarrier(ctx cdcContext.Context) (uint64, error) {
 		if !blocked {
 			return barrierTs, nil
 		}
+		log.Info("advance DDL barrier",


The message here can be confusing to users and support staff. The "DDL barrier" does not seem to be actually changed here. Also, for a single DDL, this piece of code can be executed more than once, causing more confusion.

liuzix · 2021-11-20T06:04:29Z

cdc/sink/table_sink.go

@@ -55,6 +57,15 @@ func (t *tableSink) EmitDDLEvent(ctx context.Context, ddl *model.DDLEvent) error
 // is required to be no more than global resolvedTs, table barrierTs and table
 // redo log watermarkTs.
 func (t *tableSink) FlushRowChangedEvents(ctx context.Context, resolvedTs uint64) (uint64, error) {
+	logAbnormalCheckpoint := func(ckpt uint64) {


Can you add some comments here?

amyangfei · 2021-11-20T06:06:20Z

cdc/sink/table_sink.go

+	logAbnormalCheckpoint := func(ckpt uint64) {
+		if ckpt > resolvedTs {
+			log.L().WithOptions(zap.AddCallerSkip(1)).
+				Warn("checkpoint ts > resolved ts, flushed more than emitted",


If this happens, is it better to return an error and pause the changefeed

The probability is high after table rescheduling, return error may cause changefeed stuck easily.
In addition, this situation does not necessarily mean data loss.

Signed-off-by: Neil Shen <overvenus@gmail.com>

overvenus · 2021-11-22T02:19:26Z

/merge

ti-chi-bot · 2021-11-22T02:19:28Z

This pull request has been accepted and is ready to merge.

Commit hash: 4c45290

overvenus · 2021-11-22T03:25:28Z

/run-integration-test

codecov-commenter · 2021-11-22T03:52:17Z

Codecov Report

❗ No coverage uploaded for pull request base (release-5.3@9cec140). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             release-5.3      #3540   +/-   ##
================================================
  Coverage               ?   56.8283%           
================================================
  Files                  ?        211           
  Lines                  ?      22890           
  Branches               ?          0           
================================================
  Hits                   ?      13008           
  Misses                 ?       8545           
  Partials               ?       1337

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-11-22T03:54:26Z

In response to a cherrypick label: new pull request created: #3561.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-11-22T03:55:04Z

In response to a cherrypick label: new pull request created: #3562.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-11-22T03:55:40Z

In response to a cherrypick label: new pull request created: #3563.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2021-11-22T03:56:17Z

In response to a cherrypick label: new pull request created: #3564.

…er sink flush (#3540) (#3561)

…er sink flush (#3540) (#3564)

…er sink flush (#3540) (#3563)

…er sink flush (#3540) (#3562)

…er sink flush (pingcap#3540) (pingcap#3563)

overvenus added type/bugfix This PR fixes a bug. component/sorter Sorter component. labels Nov 19, 2021

overvenus added this to the v5.3.0 milestone Nov 19, 2021

ti-chi-bot added do-not-merge/cherry-pick-not-approved release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 19, 2021

overvenus changed the title ~~processor: let sink report resolved ts~~ processor: let sink report resolved ts and do not skip buffer sink flush Nov 20, 2021

overvenus changed the title ~~processor: let sink report resolved ts and do not skip buffer sink flush~~ processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush Nov 20, 2021

overvenus force-pushed the sink-resolved-ts branch from 783a5fb to f6f9c3c Compare November 20, 2021 05:36

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 20, 2021

overvenus added 2 commits November 20, 2021 13:57

address lints

b7557d0

Signed-off-by: Neil Shen <overvenus@gmail.com>

Merge branch 'release-5.3' into sink-resolved-ts

300f688

Signed-off-by: Neil Shen <overvenus@gmail.com>

liuzix reviewed Nov 20, 2021

View reviewed changes

amyangfei reviewed Nov 20, 2021

View reviewed changes

address comments

4c45290

Signed-off-by: Neil Shen <overvenus@gmail.com>

amyangfei approved these changes Nov 20, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 20, 2021

3AceShowHand approved these changes Nov 20, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 20, 2021

zhouqiang-cl added the cherry-pick-approved Cherry pick PR approved by release team. label Nov 22, 2021

ti-chi-bot removed the do-not-merge/cherry-pick-not-approved label Nov 22, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2021

ti-chi-bot merged commit 20626ba into pingcap:release-5.3 Nov 22, 2021

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Nov 22, 2021

This is an automated cherry-pick of pingcap#3540

a653b6a

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 22, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush (#3540) #3561

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Nov 22, 2021

This is an automated cherry-pick of pingcap#3540

8ee4273

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 22, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush (#3540) #3562

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Nov 22, 2021

This is an automated cherry-pick of pingcap#3540

875a230

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 22, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush (#3540) #3563

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Nov 22, 2021

This is an automated cherry-pick of pingcap#3540

c917d36

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 22, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush (#3540) #3564

Merged

This was referenced Nov 22, 2021

[ticdc 5.3.0] tikv_scale_plan_tidb case fail: 1. cdc oom, 2. data inconsistency, 3. changefeed stuck #3503

Closed

Case "cdc_minor_components_unavaible" failed because changefeed stuck #3521

Closed

ti-chi-bot added a commit that referenced this pull request Nov 23, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buff…

3c514ac

…er sink flush (#3540) (#3561)

ti-chi-bot added a commit that referenced this pull request Nov 30, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buff…

3169b6e

…er sink flush (#3540) (#3564)

ti-chi-bot added a commit that referenced this pull request Nov 30, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buff…

bedd0fb

…er sink flush (#3540) (#3563)

ti-chi-bot added a commit that referenced this pull request Dec 10, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buff…

4837bbd

…er sink flush (#3540) (#3562)

3AceShowHand pushed a commit to ti-chi-bot/tiflow that referenced this pull request Dec 25, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buff…

3136a20

…er sink flush (pingcap#3540) (pingcap#3563)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush #3540

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush #3540

overvenus commented Nov 19, 2021 •

edited

Loading

ti-chi-bot commented Nov 19, 2021 •

edited

Loading

liuzix commented Nov 19, 2021

liuzix Nov 20, 2021

liuzix Nov 20, 2021

amyangfei Nov 20, 2021

overvenus Nov 20, 2021

overvenus commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

overvenus commented Nov 22, 2021

codecov-commenter commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush #3540

processor,sink(cdc): let sink report resolved ts and do not skip buffer sink flush #3540

Conversation

overvenus commented Nov 19, 2021 • edited Loading

What problem does this PR solve?

Check List

Release note

ti-chi-bot commented Nov 19, 2021 • edited Loading

liuzix commented Nov 19, 2021

liuzix Nov 20, 2021

Choose a reason for hiding this comment

liuzix Nov 20, 2021

Choose a reason for hiding this comment

amyangfei Nov 20, 2021

Choose a reason for hiding this comment

overvenus Nov 20, 2021

Choose a reason for hiding this comment

overvenus commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

overvenus commented Nov 22, 2021

codecov-commenter commented Nov 22, 2021

Codecov Report

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

overvenus commented Nov 19, 2021 •

edited

Loading

ti-chi-bot commented Nov 19, 2021 •

edited

Loading