cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed #6020

zhaoxinyu · 2022-06-24T03:05:22Z

What problem does this PR solve?

Issue Number: close #5684 #6021

What is changed and how it works?

Adding a new cli cmd parameter --overwrite-checkpoint-ts in changefeed resume, which allows users to specify a new changefeed checkpoint ts in the range of (GCSafeTime, CurrentTSO]. By this way, a changefeed in failed state can be resumed with an overwritten checkpoint ts instead of removing and then recreating a changefeed.
In owner(ticdc): Add backoff mechanism into changefeed restart logic (#4262) #4337, we add a defaultBackoffMaxElapsedTime to control the total retry time of changefeed. If that time elapses, the changefeed will enter into failed state. However, some users think that it's not a good idea because gc-ttl is still in valid range. So in this PR, we remove defaultBackoffMaxElapsedTime and allow changefeed retry continuously if gc-ttl is in that valid range.

Check List

Tests

Unit test
Integration test

Questions

Will it cause performance regression or break compatibility?

No

Do you need to update user documentation, design documentation or monitoring documentation?

Yes

Release note

None

ti-chi-bot · 2022-06-24T03:05:24Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

asddongmen
sdojjy

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.

zhaoxinyu · 2022-06-24T03:16:46Z

/cc @asddongmen
/cc @sdojjy
/cc @overvenus

nongfushanquan · 2022-06-24T03:30:26Z

cdc/owner/feed_state_manager.go

@@ -120,7 +116,7 @@ func (m *feedStateManager) shiftStateWindow(state model.FeedState) {
 	m.stateHistory[defaultStateWindowSize-1] = state
 }

-func (m *feedStateManager) Tick(state *orchestrator.ChangefeedReactorState) {
+func (m *feedStateManager) Tick(state *orchestrator.ChangefeedReactorState) (adminJobPending bool) {


is a good idea to return a value in the tick method ?

sdojjy · 2022-06-24T03:30:32Z

/run-integration-tests

cdc/api/v2/changefeed.go

pkg/cmd/cli/cli_changefeed_resume.go

zhaoxinyu · 2022-06-27T02:00:45Z

A failed unit test is under fixing.

zhaoxinyu · 2022-06-27T07:25:57Z

/cc @liuzix

cdc/owner/changefeed.go

cdc/scheduler/internal/base/owner_scheduler.go

pkg/cmd/cli/cli_changefeed_resume.go

zhaoxinyu · 2022-06-27T15:14:04Z

/run-integration-tests
/run-unit-tests

zhaoxinyu · 2022-06-27T15:45:48Z

/run-unit-tests

…ience

zhaoxinyu · 2022-06-28T11:03:53Z

/hold
There remains some code conflicts to be resolved.

zhaoxinyu · 2022-06-28T15:00:30Z

/unhold

zhaoxinyu · 2022-06-29T02:30:58Z

/run-integration-tests

zhaoxinyu · 2022-06-29T02:31:12Z

/cc @crelax

zhaoxinyu · 2022-06-29T04:01:16Z

/run-verify

codecov-commenter · 2022-06-29T04:16:29Z

Codecov Report

Merging #6020 (4c1714f) into cli-use-open-api (6a92960) will decrease coverage by 6.4267%.
The diff coverage is n/a.

Flag	Coverage Δ
cdc	`?`
dm	`51.9889% <ø> (+0.0685%)`	⬆️
engine	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

@@                   Coverage Diff                    @@
##           cli-use-open-api      #6020        +/-   ##
========================================================
- Coverage           58.4156%   51.9889%   -6.4267%     
========================================================
  Files                   708        240       -468     
  Lines                 83743      39393     -44350     
========================================================
- Hits                  48919      20480     -28439     
+ Misses                30373      16810     -13563     
+ Partials               4451       2103      -2348

zhaoxinyu · 2022-06-29T06:09:49Z

/run-integration-tests

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 24, 2022

zhaoxinyu mentioned this pull request Jun 24, 2022

Support configurable BackoffMaxElapsedTime #5684

Closed

ti-chi-bot requested review from asddongmen, overvenus and sdojjy June 24, 2022 03:16

nongfushanquan reviewed Jun 24, 2022

View reviewed changes

sdojjy reviewed Jun 24, 2022

View reviewed changes

cdc/api/v2/changefeed.go Outdated Show resolved Hide resolved

sdojjy reviewed Jun 24, 2022

View reviewed changes

pkg/cmd/cli/cli_changefeed_resume.go Show resolved Hide resolved

asddongmen mentioned this pull request Jun 24, 2022

command gc-ttl does not work as expected #5647

Closed

ti-chi-bot requested a review from liuzix June 27, 2022 07:25

liuzix reviewed Jun 27, 2022

View reviewed changes

cdc/owner/changefeed.go Show resolved Hide resolved

cdc/scheduler/internal/base/owner_scheduler.go Outdated Show resolved Hide resolved

pkg/cmd/cli/cli_changefeed_resume.go Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2022

zhaoxinyu changed the title ~~cli(ticdc): Allow overwrite checkpoint ts when resuming a changefeed~~ cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed Jun 27, 2022

zhaoxinyu added 9 commits June 27, 2022 17:47

allow overwrite checkpoint ts when resuming a changfeed

bdc0958

remove redundant logs

5c8017c

fix a failed ut

3bb644b

tune some code format

62c3bfb

modify the integration tset

5e177f4

remove the useless code comment

06afca3

address comments

7265ccf

fix some lint problems

91b8807

remove redundant logs

9b51968

zhaoxinyu force-pushed the optimize_changefeed_resume_experience branch from 7b49cf7 to 9b51968 Compare June 27, 2022 09:48

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2022

fix a lint problem

dd0ae0a

zhaoxinyu requested review from sdojjy and liuzix June 28, 2022 02:07

Merge branch 'cli-use-open-api' into optimize_changefeed_resume_exper…

8648a02

…ience

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2022

resolve the code conflicts

4c59939

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2022

zhaoxinyu added 2 commits June 28, 2022 23:15

modify some code related to pd config

be02561

fix some lint problems

ea4b95d

ti-chi-bot requested a review from crelax June 29, 2022 02:31

unify function name

4c1714f

sdojjy approved these changes Jun 29, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 29, 2022

asddongmen approved these changes Jun 29, 2022

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 Jun 29, 2022

asddongmen merged commit 702f151 into pingcap:cli-use-open-api Jun 29, 2022

This was referenced Jul 1, 2022

config(ticdc): Support configurable BackoffMaxElapsedTime #6009

Closed

Add a new cli cmd flag to overwrite checkpoint when resuming a changefeed #6021

Closed

zhaoxinyu mentioned this pull request Jul 20, 2022

owner(ticdc): Fix an unexpected stop problem for changefeed backoff #6351

Merged

zhaoxinyu mentioned this pull request Feb 27, 2023

owner(ticdc): do not stop backoff when encountering an error #8382

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed #6020

cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed #6020

zhaoxinyu commented Jun 24, 2022 •

edited

Loading

ti-chi-bot commented Jun 24, 2022 •

edited

Loading

zhaoxinyu commented Jun 24, 2022

nongfushanquan Jun 24, 2022

sdojjy commented Jun 24, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 28, 2022

zhaoxinyu commented Jun 28, 2022

zhaoxinyu commented Jun 29, 2022

zhaoxinyu commented Jun 29, 2022

zhaoxinyu commented Jun 29, 2022

codecov-commenter commented Jun 29, 2022

zhaoxinyu commented Jun 29, 2022

cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed #6020

cli(ticdc): Allow overwriting checkpoint ts when resuming a changefeed #6020

Conversation

zhaoxinyu commented Jun 24, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

ti-chi-bot commented Jun 24, 2022 • edited Loading

zhaoxinyu commented Jun 24, 2022

nongfushanquan Jun 24, 2022

Choose a reason for hiding this comment

sdojjy commented Jun 24, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 27, 2022

zhaoxinyu commented Jun 28, 2022

zhaoxinyu commented Jun 28, 2022

zhaoxinyu commented Jun 29, 2022

zhaoxinyu commented Jun 29, 2022

zhaoxinyu commented Jun 29, 2022

codecov-commenter commented Jun 29, 2022

Codecov Report

zhaoxinyu commented Jun 29, 2022

zhaoxinyu commented Jun 24, 2022 •

edited

Loading

ti-chi-bot commented Jun 24, 2022 •

edited

Loading