ttl: Implement scan and delete task for TTL #39481

lcwangchao · 2022-11-30T05:57:19Z

What problem does this PR solve?

Issue Number: close #39480 close #39554

Problem Summary:

implement ttlScanTask for ttl
implement ttlDeleteTask for ttl

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Add new sysvars: `tidb_ttl_job_enable`, `tidb_ttl_scan_batch_size`, `tidb_ttl_delete_batch_size`, `tidb_ttl_delete_rate_limit`

ti-chi-bot · 2022-11-30T05:57:20Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

YangKeao
bb7133

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.

lcwangchao · 2022-12-02T06:16:48Z

/run-check_dev_2

YangKeao · 2022-12-02T06:43:37Z

ttl/ttlworker/scan.go

+			continue
+		}
+
+		delTask := &ttlDeleteTask{


Should delTask also have some way to send back the execution status? (like the notifyStateCh above).

The problem I considered is that the job and manager has no idea how much del tasks will be created, so they don't know how much state they will need to wait. But they actually need to know when all tasks have finished 🤔 (to update state, and migrate all fields from current_job_* to last_job_*).

A way I could propose is to use a counter to count the number of spawned task of the job, when a task (no matter it's scan or del, and no matter it's error or not) finished, decrease the counter and if the counter arrives 0, update the state and migrate the fields.

See https://github.com/pingcap/tidb/pull/39472/files#diff-ce94329d9478a76725965679f8e00f713c604a630bad1220443582a4a50b8d88R103.

When a scan task want to spawn some del task, it could firstly add the counter (by task.tracker.Add()), and when a task is finished, task.tracker.Done(xxx) is called (by scan and del worker). 🤔

(For this approach, using function call, or a channel with extra goroutine for each job are both fine. I'm not sure which one is better.)

I have also considered to track del task by scan worker, and track scan task by job (manager). It seems more complicated 🤦 .

Each task have a ttlStatistics object, I think you can use it to determine whether a all deletions from a scan task is end (by evaluating totalCount == errorCount + successCount).

lcwangchao · 2022-12-02T06:56:59Z

/run-unit-test

ttl/ttlworker/worker.go

bb7133

LGTM

YangKeao

LGTM

lcwangchao · 2022-12-05T03:31:54Z

/merge

ti-chi-bot · 2022-12-05T03:31:58Z

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

Commit hash: 03b8b5f

ti-chi-bot · 2022-12-05T05:08:44Z

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

close pingcap#39480, close pingcap#39554

sre-bot · 2022-12-05T05:38:43Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-common-test	🟥 failed 1, success 16, total 17	17 min	New failing
idc-jenkins-ci/integration-cdc-test	🔴 failed 1, success 39, total 40	24 min	Existing failure
idc-jenkins-ci-tidb/common-test	🔴 failed 1, success 10, total 11	16 min	Existing failure
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	9 min 14 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	6 min 44 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	5 min 45 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	4 min 55 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 39 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 54 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

close #39480, close #39554

lcwangchao added 5 commits November 29, 2022 20:38

ttl: implement scan task

4f04c73

update

c646f59

update

784eb2e

update

b2abd9d

update

4de83eb

lcwangchao requested a review from a team as a code owner November 30, 2022 05:57

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 30, 2022

lcwangchao changed the title *: Implement scan task for TTL ttl: Implement scan task for TTL Nov 30, 2022

Merge branch 'master' into ttl_scan

4141cff

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

lcwangchao added 4 commits December 1, 2022 13:32

update

f31dbd5

update

d264384

update

5cffd05

ttl: implement deleteWorker

32d669b

lcwangchao changed the title ~~ttl: Implement scan task for TTL~~ ttl: Implement scan and delete task for TTL Dec 1, 2022

Merge branch 'master' into ttl_scan

8c70348

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 1, 2022

lcwangchao added 5 commits December 1, 2022 20:37

update

ce17b01

update

dcbb731

update

c22e38a

remove blank line

8866d9b

update

a92cad3

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

Merge branch 'master' into ttl_scan

8e3a9ad

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

update bazel

ea62729

lcwangchao added 3 commits December 2, 2022 13:48

update

4ec1626

Merge branch 'ttl_scan' of github.com:lcwangchao/tidb into ttl_scan

a762ceb

Merge branch 'master' into ttl_scan

56027a6

update

1cfff23

YangKeao reviewed Dec 2, 2022

View reviewed changes

update

d5199e6

ti-chi-bot added the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Dec 2, 2022

bb7133 reviewed Dec 2, 2022

View reviewed changes

ttl/ttlworker/worker.go Outdated Show resolved Hide resolved

lcwangchao added 2 commits December 5, 2022 10:08

update

132cc14

Merge branch 'master' into ttl_scan

03b8b5f

bb7133 approved these changes Dec 5, 2022

View reviewed changes

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

YangKeao approved these changes Dec 5, 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 Dec 5, 2022

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

ti-chi-bot added 3 commits December 5, 2022 11:32

Merge branch 'master' into ttl_scan

5a37bb9

Merge branch 'master' into ttl_scan

d8b77d5

Merge branch 'master' into ttl_scan

bd795fb

ti-chi-bot merged commit 2d30149 into pingcap:master Dec 5, 2022

ti-chi-bot mentioned this pull request Dec 5, 2022

ttl: Implement scan and delete task for TTL (#39481) #39615

Merged

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 5, 2022

ttl: Implement scan and delete task for TTL (pingcap#39481)

9288bd7

close pingcap#39480, close pingcap#39554

lcwangchao deleted the ttl_scan branch December 5, 2022 06:07

ti-chi-bot added a commit that referenced this pull request Dec 19, 2022

ttl: Implement scan and delete task for TTL (#39481) (#39615)

d211731

close #39480, close #39554

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ttl: Implement scan and delete task for TTL #39481

ttl: Implement scan and delete task for TTL #39481

lcwangchao commented Nov 30, 2022 •

edited

Loading

ti-chi-bot commented Nov 30, 2022 •

edited

Loading

lcwangchao commented Dec 2, 2022

YangKeao Dec 2, 2022

YangKeao Dec 2, 2022

lcwangchao Dec 2, 2022

lcwangchao commented Dec 2, 2022

bb7133 left a comment

YangKeao left a comment

lcwangchao commented Dec 5, 2022

ti-chi-bot commented Dec 5, 2022

ti-chi-bot commented Dec 5, 2022

sre-bot commented Dec 5, 2022

ttl: Implement scan and delete task for TTL #39481

ttl: Implement scan and delete task for TTL #39481

Conversation

lcwangchao commented Nov 30, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 30, 2022 • edited Loading

lcwangchao commented Dec 2, 2022

YangKeao Dec 2, 2022

Choose a reason for hiding this comment

YangKeao Dec 2, 2022

Choose a reason for hiding this comment

lcwangchao Dec 2, 2022

Choose a reason for hiding this comment

lcwangchao commented Dec 2, 2022

bb7133 left a comment

Choose a reason for hiding this comment

YangKeao left a comment

Choose a reason for hiding this comment

lcwangchao commented Dec 5, 2022

ti-chi-bot commented Dec 5, 2022

ti-chi-bot commented Dec 5, 2022

sre-bot commented Dec 5, 2022

TiDB MergeCI notify

lcwangchao commented Nov 30, 2022 •

edited

Loading

ti-chi-bot commented Nov 30, 2022 •

edited

Loading