redo (ticdc): only delete changefeed related file when changefeed was removed #7279

asddongmen · 2022-09-30T08:58:41Z

What problem does this PR solve?

Issue Number: close #6413

What is changed and how it works?

Only delete the file belong to a changefeed when it's release, rather than the whole directory that used by it.

Check List

Tests

Unit test
Manual test (add detailed scripts or steps below)

create changefeed test-1, and other two changefeed. Enable redo log for all of them, using the same storage uri:

s3://guagua/test1?access-key=xxxx&secret-access-key=xxxx&endpoint=http://xxxx/

Replicate some data, and remove one changefeed.
Inspect the s3://guagua/test1 dir, the files of removed changefeed has been deleted but other two changefeeds' files remain.

Questions

Will it cause performance regression or break compatibility?

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

Release note

Fix a bug that may cause changefeed's redo log files be deleted wrongly.

ti-chi-bot · 2022-09-30T08:58:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nongfushanquan
zhaoxinyu

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.

asddongmen · 2022-09-30T09:38:48Z

/run-integration-tests

…dc into dongmen/fix_redo_clean_up

codecov-commenter · 2022-10-01T00:56:16Z

Codecov Report

Merging #7279 (aeac209) into master (8f81dbb) will increase coverage by 0.0024%.
The diff coverage is 59.3442%.

Additional details and impacted files

Flag	Coverage Δ
cdc	`67.2806% <62.6609%> (-0.0335%)`	⬇️
dm	`51.9866% <36.6666%> (+0.0325%)`	⬆️
engine	`61.0419% <57.1428%> (-0.0310%)`	⬇️

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

@@               Coverage Diff                @@
##             master      #7279        +/-   ##
================================================
+ Coverage   59.7265%   59.7289%   +0.0023%     
================================================
  Files           790        789         -1     
  Lines         91204      91274        +70     
================================================
+ Hits          54473      54517        +44     
+ Misses        31987      31976        -11     
- Partials       4744       4781        +37

nongfushanquan

please correct the title "Redo"

cdc/redo/common/util.go

cdc/redo/writer/file.go

cdc/redo/writer/writer.go

zhaoxinyu · 2022-10-09T06:53:19Z

Rest LGTM

zhaoxinyu · 2022-10-10T02:36:33Z

pkg/uuid/uuid.go

@@ -15,7 +15,7 @@ package uuid

 import guuid "github.com/google/uuid"

-// Generator defines an interface that can generate a uuid
+// Generator defines an interface that can generate an uuid


Suggested change

// Generator defines an interface that can generate an uuid

// Generator defines an interface that can generate a uuid

The original code comment is correct.

cdc/redo/common/util.go

cdc/redo/writer/writer.go

asddongmen · 2022-10-10T03:00:04Z

/merge

ti-chi-bot · 2022-10-10T03:00:07Z

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

Commit hash: 8c03288

ti-chi-bot · 2022-10-10T03:25:59Z

@asddongmen: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-10-10T03:58:27Z

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

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2022-10-10T03:59:04Z

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

… removed (#7279) (#7305) close #6413

reod: only delete changefeed related file

6d089d0

ti-chi-bot added needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.3 Should cherry pick this PR to release-6.3 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Sep 30, 2022

Merge branch 'master' into dongmen/fix_redo_clean_up

795560e

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 30, 2022

asddongmen requested review from zhaoxinyu and hicqu September 30, 2022 08:59

amyangfei added the area/ticdc Issues or PRs related to TiCDC. label Sep 30, 2022

asddongmen added 2 commits October 1, 2022 08:38

fix lint check

3146f88

Merge branch 'dongmen/fix_redo_clean_up' of github.com:asddongmen/tic…

7d7498d

…dc into dongmen/fix_redo_clean_up

asddongmen added the component/redolog label Oct 1, 2022

asddongmen changed the title ~~reod: only delete changefeed related file when changefeed was remove~~ reod: only delete changefeed related file when changefeed was removed Oct 1, 2022

nongfushanquan reviewed Oct 8, 2022

View reviewed changes

cdc/redo/common/util.go Outdated Show resolved Hide resolved

asddongmen changed the title ~~reod: only delete changefeed related file when changefeed was removed~~ redo (ticdc): only delete changefeed related file when changefeed was removed Oct 8, 2022

zhaoxinyu reviewed Oct 9, 2022

View reviewed changes

cdc/redo/writer/file.go Outdated Show resolved Hide resolved

zhaoxinyu reviewed Oct 9, 2022

View reviewed changes

cdc/redo/writer/writer.go Outdated Show resolved Hide resolved

zhaoxinyu reviewed Oct 9, 2022

View reviewed changes

cdc/redo/writer/writer.go Outdated Show resolved Hide resolved

address comments

c4fea73

zhaoxinyu reviewed Oct 10, 2022

View reviewed changes

zhaoxinyu approved these changes Oct 10, 2022

View reviewed changes

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

hicqu reviewed Oct 10, 2022

View reviewed changes

cdc/redo/common/util.go Outdated Show resolved Hide resolved

hicqu reviewed Oct 10, 2022

View reviewed changes

cdc/redo/writer/writer.go Show resolved Hide resolved

nongfushanquan approved these changes Oct 10, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 10, 2022

ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 10, 2022

address comments

8c03288

hicqu approved these changes Oct 10, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 10, 2022

ti-chi-bot added 2 commits October 10, 2022 11:00

Merge branch 'master' into dongmen/fix_redo_clean_up

6efbcd7

Merge branch 'master' into dongmen/fix_redo_clean_up

aeac209

ti-chi-bot merged commit 3ede032 into pingcap:master Oct 10, 2022

ti-chi-bot mentioned this pull request Oct 10, 2022

redo (ticdc): only delete changefeed related file when changefeed was removed (#7279) #7305

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Oct 10, 2022

This is an automated cherry-pick of pingcap#7279

6b92f23

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Oct 10, 2022

redo (ticdc): only delete changefeed related file when changefeed was removed (#7279) #7306

Closed

ti-chi-bot added a commit that referenced this pull request Oct 10, 2022

redo (ticdc): only delete changefeed related file when changefeed was…

4af5e75

… removed (#7279) (#7305) close #6413

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

redo (ticdc): only delete changefeed related file when changefeed was removed #7279

redo (ticdc): only delete changefeed related file when changefeed was removed #7279

asddongmen commented Sep 30, 2022 •

edited

Loading

ti-chi-bot commented Sep 30, 2022 •

edited

Loading

asddongmen commented Sep 30, 2022

codecov-commenter commented Oct 1, 2022 •

edited

Loading

nongfushanquan left a comment

zhaoxinyu commented Oct 9, 2022

zhaoxinyu Oct 10, 2022

zhaoxinyu Oct 10, 2022

asddongmen commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

	// Generator defines an interface that can generate an uuid
	// Generator defines an interface that can generate a uuid

redo (ticdc): only delete changefeed related file when changefeed was removed #7279

redo (ticdc): only delete changefeed related file when changefeed was removed #7279

Conversation

asddongmen commented Sep 30, 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 Sep 30, 2022 • edited Loading

asddongmen commented Sep 30, 2022

codecov-commenter commented Oct 1, 2022 • edited Loading

Codecov Report

nongfushanquan left a comment

Choose a reason for hiding this comment

zhaoxinyu commented Oct 9, 2022

zhaoxinyu Oct 10, 2022

Choose a reason for hiding this comment

zhaoxinyu Oct 10, 2022

Choose a reason for hiding this comment

asddongmen commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

ti-chi-bot commented Oct 10, 2022

asddongmen commented Sep 30, 2022 •

edited

Loading

ti-chi-bot commented Sep 30, 2022 •

edited

Loading

codecov-commenter commented Oct 1, 2022 •

edited

Loading