redolog: add a precleanup process when s3 enable #3525

ben1009 · 2021-11-18T10:26:42Z

What problem does this PR solve?

add a precleanup prcess when s3 enable
close https://github.com/pingcap/ticdc/issues/3523

What is changed and how it works?

set a delete marker when delete logs in s3, since the process got the remove changefeed job async, may continue write logs after owner remove the logs. if have the changefeed with the same name created later, pre cleanup the log just in case.
when gc if the dir is not exist, just log instead of return err

Check List

Tests

Unit test

Code changes

Has exported function/method change

Side effects

Related changes

Release note

add pre clean up process when s3 enable, fix #3523

ti-chi-bot · 2021-11-18T10:26:43Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

amyangfei
hi-rustin

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.

ben1009 · 2021-11-19T07:39:09Z

/cc @amyangfei

amyangfei · 2021-11-23T02:46:00Z

cdc/redo/writer/writer.go

+func (l *LogWriter) writeDeletedMarkerToS3(ctx context.Context) error {
+	return cerror.WrapError(cerror.ErrS3StorageAPI, l.storage.WriteFile(ctx, l.getDeletedChangefeedMarker(), []byte("D")))
 }


Is the delete marker file written to S3, and the redo log dir in S3 is not deleted after changefeed is removed?

yes, the bucket is still there, no extra cost. but currently we do not delete the bucket even without the marker, just delete the object

codecov-commenter · 2021-12-06T10:07:32Z

Codecov Report

Merging #3525 (7020d71) into master (eed6f9b) will decrease coverage by 0.3674%.
The diff coverage is 59.1598%.

Flag	Coverage Δ
cdc	`57.9604% <72.9306%> (-0.4768%)`	⬇️
dm	`56.1061% <44.1463%> (-0.2692%)`	⬇️

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

@@               Coverage Diff                @@
##             master      #3525        +/-   ##
================================================
- Coverage   57.3452%   56.9777%   -0.3675%     
================================================
  Files           460        473        +13     
  Lines         54927      56243      +1316     
================================================
+ Hits          31498      32046       +548     
- Misses        20180      20957       +777     
+ Partials       3249       3240         -9

cdc/redo/writer/writer_test.go

ben1009 · 2021-12-09T04:38:20Z

/run-leak-test

amyangfei · 2021-12-13T04:07:05Z

/merge

ti-chi-bot · 2021-12-13T04:07:08Z

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

Commit hash: 614b456

ben1009 · 2021-12-13T06:54:04Z

/run-kafka-integration-test
/run-dm-integration-test

ben1009 · 2021-12-13T08:06:14Z

/run-verify

ben1009 · 2021-12-13T08:33:40Z

/run-kafka-integration-test /run-dm-integration-test

+1

ben1009 · 2021-12-13T09:37:43Z

/run-integration-test

ben1009 · 2021-12-14T01:55:54Z

/run-integration-test
/run-dm-integration-test

ben1009 · 2021-12-14T02:26:23Z

/run-integration-test

ti-chi-bot · 2021-12-14T02:44:43Z

@ben1009: 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.

ben1009 · 2021-12-14T02:59:45Z

/run-verify

1

ben1009 · 2021-12-14T03:09:30Z

/run-integration-test

1

ti-chi-bot · 2021-12-14T03:41:11Z

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

…cap#3878)

…ngcap#4323) * fix the txn_batch_size metric inaccuracy bug when the sink target is MQ * address comments * add comments for exported functions * fix the compiling problem * workerpool: limit the rate to output deadlock warning (pingcap#3775) (pingcap#3795) * tests(ticdc): set up the sync diff output directory correctly (pingcap#3725) (pingcap#3741) * relay(dm): use binlog name comparison (pingcap#3710) (pingcap#3712) * dm/load: fix concurrent call Loader.Status (pingcap#3459) (pingcap#3468) * cdc/sorter: make unified sorter cgroup aware (pingcap#3436) (pingcap#3439) * tz (ticdc): fix timezone error (pingcap#3887) (pingcap#3906) * pkg,cdc: do not use log package (pingcap#3902) (pingcap#3940) * *: rename repo from pingcap/ticdc to pingcap/tiflow (pingcap#3959) * http_*: add log for http api and refine the err handle logic (pingcap#2997) (pingcap#3307) * etcd_worker: batch etcd patch (pingcap#3277) (pingcap#3389) * http_api (ticdc): check --cert-allowed-cn before add server common name (pingcap#3628) (pingcap#3882) * kvclient(ticdc): fix kvclient takes too long time to recover (pingcap#3612) (pingcap#3663) * owner: fix owner tick block http request (pingcap#3490) (pingcap#3530) * dm/syncer: use downstream PK/UK to generate DML (pingcap#3168) (pingcap#3256) * dep(dm): update go-mysql (pingcap#3914) (pingcap#3934) * dm/syncer: multiple rows use downstream schema (pingcap#3308) (pingcap#3953) * errorutil,sink,syncer: add errorutil to handle ignorable error (pingcap#3264) (pingcap#3995) * dm/worker: don't exit when failed to read checkpoint in relay (pingcap#3345) (pingcap#4005) * syncer(dm): use an early location to reset binlog and open safemode (pingcap#3860) * ticdc/owner: Fix ddl special comment syntax error (pingcap#3845) (pingcap#3978) * dm/scheduler: fix inconsistent of relay status (pingcap#3474) (pingcap#4009) * owner,scheduler(cdc): fix nil pointer panic in owner scheduler (pingcap#2980) (pingcap#4007) (pingcap#4016) * config(ticdc): Fix old value configuration check for maxwell protocol (pingcap#3747) (pingcap#3783) * sink(ticdc): cherry pick sink bug fix to release 5.3 (pingcap#4083) * master(dm): clean and treat invalid load task (pingcap#4004) (pingcap#4145) * loader: fix wrong progress in query-status for loader (pingcap#4093) (pingcap#4143) close pingcap#3252 * ticdc/processor: Fix backoff base delay misconfiguration (pingcap#3992) (pingcap#4028) * dm: load table structure from dump files (pingcap#3295) (pingcap#4163) * compactor: fix duplicate entry in safemode (pingcap#3432) (pingcap#3434) (pingcap#4088) * kv(ticdc): reduce eventfeed rate limited log (pingcap#4072) (pingcap#4111) close pingcap#4006 * metrics(ticdc): add resolved ts and add changefeed to dataflow (pingcap#4038) (pingcap#4104) * This is an automated cherry-pick of pingcap#4192 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> * retry(dm): align with tidb latest error message (pingcap#4172) (pingcap#4254) close pingcap#4159, close pingcap#4246 * owner(ticdc): Add bootstrap and try to fix the meta information in it (pingcap#3838) (pingcap#3865) * redolog: add a precleanup process when s3 enable (pingcap#3525) (pingcap#3878) * ddl(dm): make skipped ddl pass `SplitDDL()` (pingcap#4176) (pingcap#4227) close pingcap#4173 * cdc/sink: remove Initialize method from the sink interface (pingcap#3682) (pingcap#3765) Co-authored-by: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> * http_api (ticdc): fix http api 'get processor' panic. (pingcap#4117) (pingcap#4123) close pingcap#3840 * sink (ticdc): fix a deadlock due to checkpointTs fall back in sinkNode (pingcap#4084) (pingcap#4099) close pingcap#4055 * cdc/sink: adjust kafka initialization logic (pingcap#3192) (pingcap#4162) * try fix conflicts. * This is an automated cherry-pick of pingcap#4192 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> * fix conflicts. * fix conflicts. Co-authored-by: zhaoxinyu <zhaoxinyu512@gmail.com> Co-authored-by: amyangfei <yangfei@pingcap.com> Co-authored-by: lance6716 <lance6716@gmail.com> Co-authored-by: sdojjy <sdojjy@qq.com> Co-authored-by: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Co-authored-by: 3AceShowHand <jinl1037@hotmail.com>

redolog: add a precleanup prcess when s3 enable

cd0b8f4

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 18, 2021

ben1009 added area/ticdc Issues or PRs related to TiCDC. component/redolog and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 18, 2021

Merge branch 'master' into cdc-redo-refine-s3

7e00391

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 18, 2021

ben1009 requested a review from Rustin170506 November 19, 2021 07:36

ti-chi-bot requested a review from amyangfei November 19, 2021 07:39

amyangfei reviewed Nov 23, 2021

View reviewed changes

amyangfei added the needs-cherry-pick-release-5.3 Should cherry pick this PR to release-5.3 branch. label Nov 23, 2021

Merge branch 'master' into cdc-redo-refine-s3

7f7c648

Rustin170506 changed the title ~~redolog: add a precleanup prcess when s3 enable~~ redolog: add a precleanup process when s3 enable Dec 7, 2021

Rustin170506 reviewed Dec 7, 2021

View reviewed changes

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

Rustin170506 reviewed Dec 7, 2021

View reviewed changes

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

ben1009 added 2 commits December 9, 2021 11:45

Merge branch 'master' into cdc-redo-refine-s3

db68641

fix comments

614b456

Rustin170506 approved these changes Dec 9, 2021

View reviewed changes

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

Rustin170506 requested a review from amyangfei December 9, 2021 06:40

amyangfei approved these changes Dec 13, 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 Dec 13, 2021

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

Merge branch 'master' into cdc-redo-refine-s3

d90dee1

ti-chi-bot added 6 commits December 13, 2021 20:48

Merge branch 'master' into cdc-redo-refine-s3

7020d71

Merge branch 'master' into cdc-redo-refine-s3

84214ac

Merge branch 'master' into cdc-redo-refine-s3

9e47e50

Merge branch 'master' into cdc-redo-refine-s3

a07c424

Merge branch 'master' into cdc-redo-refine-s3

81163cc

Merge branch 'master' into cdc-redo-refine-s3

db3a0a6

Merge branch 'master' into cdc-redo-refine-s3

8580ebd

Merge branch 'master' into cdc-redo-refine-s3

eebc80f

ti-chi-bot merged commit fbf5f4e into pingcap:master Dec 14, 2021

ben1009 deleted the cdc-redo-refine-s3 branch December 14, 2021 03:40

ti-chi-bot mentioned this pull request Dec 14, 2021

redolog: add a precleanup process when s3 enable (#3525) #3878

Merged

zhaoxinyu pushed a commit to zhaoxinyu/ticdc that referenced this pull request Dec 29, 2021

redolog: add a precleanup process when s3 enable (pingcap#3525)

2074830

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

redolog: add a precleanup process when s3 enable (#3525) (#3878)

7231096

3AceShowHand pushed a commit to ti-chi-bot/tiflow that referenced this pull request Jan 13, 2022

redolog: add a precleanup process when s3 enable (pingcap#3525) (ping…

72ce433

…cap#3878)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

redolog: add a precleanup process when s3 enable #3525

redolog: add a precleanup process when s3 enable #3525

ben1009 commented Nov 18, 2021

ti-chi-bot commented Nov 18, 2021 •

edited

Loading

ben1009 commented Nov 19, 2021

amyangfei Nov 23, 2021

ben1009 Nov 23, 2021

codecov-commenter commented Dec 6, 2021 •

edited

Loading

ben1009 commented Dec 9, 2021

amyangfei commented Dec 13, 2021

ti-chi-bot commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

redolog: add a precleanup process when s3 enable #3525

redolog: add a precleanup process when s3 enable #3525

Conversation

ben1009 commented Nov 18, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

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

ben1009 commented Nov 19, 2021

amyangfei Nov 23, 2021

Choose a reason for hiding this comment

ben1009 Nov 23, 2021

Choose a reason for hiding this comment

codecov-commenter commented Dec 6, 2021 • edited Loading

Codecov Report

ben1009 commented Dec 9, 2021

amyangfei commented Dec 13, 2021

ti-chi-bot commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 13, 2021

ben1009 commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ben1009 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

ti-chi-bot commented Nov 18, 2021 •

edited

Loading

codecov-commenter commented Dec 6, 2021 •

edited

Loading