ttl: implement ttl job schedule framework by YangKeao · Pull Request #39472 · pingcap/tidb

YangKeao · 2022-11-29T14:19:25Z

What problem does this PR solve?

Issue Number: close #39471

Problem Summary:

Implement a ttl job schedule framework. It reads information from the info schema and the mysql.tidb_ttl_table_status, gets the job to run and schedule them to a scan worker.

The implementation of variable settings, scanWorker and delWorker are left for future development.

What is changed and how it works?

For most parts, the job manager is split from the original demo: https://github.com/pingcap/tidb/pull/39320/files. It implements the following feature:

The most fundamental one. Let the job run on the worker.
Support graceful shutdown with the context from domain.
Allow collecting the errors from the tasks, and storing them into the table when the job is finished.
Allow cancelling the running jobs.

This pr left a lot of TODO for system variables. They are left in #39727.

Check List

Tests

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

ti-chi-bot · 2022-11-29T14:19:27Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

hawkingrei
lcwangchao

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.

Details

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

hawkingrei · 2022-12-01T02:57:10Z

@YangKeao

ttl/ttlworker/del.go:35:2: should use for range instead of for { select {} } (S1000)

ttl/ttlworker/job_manager.go:423:9: ineffectual assignment to err (ineffassign)

hawkingrei · 2022-12-09T07:10:53Z

/run-mysql-test

ttl/ttlworker/job_manager.go

lcwangchao · 2022-12-09T07:29:17Z

ttl/ttlworker/job_manager.go

seems when a job's hbtime stops to update, a new job with a new ttl expire will be created. I'm not sure whether we should make a little more complex strategy to make the job can be "resume" instead a new job. This maybe useful in some cases, for example, if we have a bug that a job will crash after running for 1hour, it still can guarantee a job will be finished finally and make sure the "cleared watermark" can be pushed forward.

If we want to implement the "resume", another thing we should concern is that if a very old job failed, we should not resume it.

lcwangchao · 2022-12-09T07:33:40Z

ttl/ttlworker/job_manager.go

tableStatus.CurrentJobOwnerID != m.id means this job is take over by other worker, so is it necessary to reset the job's status in this node?

reset the job's status in this node

Remove it from the running job and cancelling all running tasks are the only "status" local . The manager cannot get the job anymore 🤔, so I don't think other states matter.

lcwangchao · 2022-12-09T07:36:05Z

Tough we still have some details to discuss, but I think we can merge it first to do some tests.

YangKeao · 2022-12-12T05:36:14Z

@hawkingrei @lcwangchao Could you help me to merge this PR?

YangKeao · 2022-12-12T06:00:27Z

/hold

I found a bug 🤦

ttl/session/session.go

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

YangKeao · 2022-12-12T06:27:01Z

/unhold

lcwangchao · 2022-12-12T06:28:00Z

/merge

ti-chi-bot · 2022-12-12T06:28:04Z

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

Details

Commit hash: 41326e0

YangKeao · 2022-12-12T07:26:12Z

/run-unit-test.

wuhuizuo · 2022-12-12T07:32:35Z

/run-unit-test

wuhuizuo · 2022-12-12T07:45:04Z

/run-unit-test

ti-chi-bot · 2022-12-12T07:47:49Z

hello from wuhuizuo

ti-chi-bot · 2022-12-12T07:53:02Z

In response to a cherrypick label: new pull request created to branch release-6.5: #39830.

sre-bot · 2022-12-12T08:21:59Z

TiDB MergeCI notify

✅ Well Done! New fixed [1] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/common-test	🔴 failed 1, success 10, total 11	11 min	Existing failure
idc-jenkins-ci-tidb/integration-compatibility-test	✅ all 1 tests passed	3 min 22 sec	Fixed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	22 min	Existing passed
idc-jenkins-ci/integration-cdc-test	🟢 all 40 tests passed	19 min	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 20 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	6 min 15 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	4 min 51 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	4 min 20 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 49 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

YangKeao added the release-note-none Denotes a PR that doesn't merit a release note. label Nov 29, 2022

YangKeao requested a review from a team as a code owner November 29, 2022 14:19

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 29, 2022

YangKeao force-pushed the add-ttl-job-schedule branch 2 times, most recently from 15885a8 to 8b275b1 Compare November 30, 2022 04:57

YangKeao requested a review from lcwangchao November 30, 2022 05:06

YangKeao force-pushed the add-ttl-job-schedule branch 4 times, most recently from a359fc7 to 1d0d7c2 Compare November 30, 2022 05:31

YangKeao requested a review from hawkingrei November 30, 2022 05:35

YangKeao force-pushed the add-ttl-job-schedule branch from 1d0d7c2 to 8eb9302 Compare November 30, 2022 05:38

YangKeao force-pushed the add-ttl-job-schedule branch from 8eb9302 to 1d5d30d Compare December 1, 2022 03:50

YangKeao marked this pull request as draft December 1, 2022 08:39

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2022

YangKeao mentioned this pull request Dec 1, 2022

ttl: add info schema cache and ttl table status table cache for ttl scheduler #39467

Merged

4 tasks

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

YangKeao added the affects-6.5 This bug affects the 6.5.x(LTS) versions. label Dec 2, 2022

YangKeao force-pushed the add-ttl-job-schedule branch from 1d5d30d to c9fc7f9 Compare December 5, 2022 13:22

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

YangKeao removed the request for review from a team December 6, 2022 04:30

YangKeao force-pushed the add-ttl-job-schedule branch 4 times, most recently from 4b46c4a to c1effd3 Compare December 6, 2022 10:46

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

YangKeao marked this pull request as ready for review December 6, 2022 10:49

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2022

YangKeao requested a review from lcwangchao December 9, 2022 06:29

YangKeao force-pushed the add-ttl-job-schedule branch 3 times, most recently from e0be974 to c091e49 Compare December 9, 2022 07:31

lcwangchao approved these changes Dec 9, 2022

View reviewed changes

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

YangKeao force-pushed the add-ttl-job-schedule branch 3 times, most recently from 6c68a80 to ab70f7f Compare December 9, 2022 07:56

hawkingrei approved these changes Dec 12, 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 12, 2022

YangKeao commented Dec 12, 2022

View reviewed changes

ttl/session/session.go Outdated Show resolved Hide resolved

add ttl job manager framework

41326e0

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

Merge branch 'master' into add-ttl-job-schedule

f191022

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

ttl: implement ttl job schedule framework (#39472) #39830

Merged

4 tasks

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

ttl, variables: add system variables for ttl job schedule (#39727) #39869

Merged

4 tasks

Conversation

YangKeao commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

ti-chi-bot commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkingrei commented Dec 1, 2022

Uh oh!

hawkingrei commented Dec 9, 2022

Uh oh!

Uh oh!

Uh oh!

lcwangchao Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

lcwangchao Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

lcwangchao Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

YangKeao Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

lcwangchao commented Dec 9, 2022

Uh oh!

YangKeao commented Dec 12, 2022

Uh oh!

YangKeao commented Dec 12, 2022

Uh oh!

Uh oh!

YangKeao commented Dec 12, 2022

Uh oh!

lcwangchao commented Dec 12, 2022

Uh oh!

ti-chi-bot commented Dec 12, 2022

Uh oh!

YangKeao commented Dec 12, 2022

Uh oh!

wuhuizuo commented Dec 12, 2022

Uh oh!

wuhuizuo commented Dec 12, 2022

Uh oh!

ti-chi-bot commented Dec 12, 2022

Uh oh!

ti-chi-bot commented Dec 12, 2022

Uh oh!

sre-bot commented Dec 12, 2022

TiDB MergeCI notify

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

YangKeao commented Nov 29, 2022 •

edited

Loading

ti-chi-bot commented Nov 29, 2022 •

edited

Loading