Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: add ttl related jobs / execution #39298

Merged
merged 5 commits into from
Nov 28, 2022
Merged

ddl: add ttl related jobs / execution #39298

merged 5 commits into from
Nov 28, 2022

Conversation

YangKeao
Copy link
Member

@YangKeao YangKeao commented Nov 21, 2022 •
edited
Loading

What problem does this PR solve?

Define and execute ttl related ddl jobs.

Issue Number: close #39269, close #39270, close #39271

This PR is based on #39341, so please merge #39341 before this one.

Problem Summary:

What is changed and how it works?

Add the execution of TTl related ddl statements, some validations and the ddl worker support.

The DDL worker simply records the ttlInfo and ttlEnable. We'll need to check and modify the running status of ttl job in the future.

Check List

Tests

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

Release note

Support TTL define, modification and print in create table, alter table and show create table statement.

@ti-chi-bot
Copy link
Member

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

[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.

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

@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 21, 2022
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch 2 times, most recently from a32b7b4 to 482faad Compare November 21, 2022 13:19
hawkingrei
hawkingrei reviewed Nov 21, 2022
View reviewed changes
ddl/ttl.go
@@ -0,0 +1,110 @@
// Copyright 2022 PingCAP, Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1、remember to update bazel config when you create a new file.
2、ddl has been put too many files. so can you put them into ddl/ttl if it is easy to do ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Done
  2. They depend on some internal structs / functions of ddl, for example findColumnByName, ddlCtx... So they cannot be moved to ddl/ttl directly 🤔 .
    If it's an issue, I suggest to try to extract some utilities functions first, so that other functions would be easier to be extracted.

@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch 2 times, most recently from f5b8ff3 to d3db3cf Compare November 21, 2022 13:46
@YangKeao YangKeao marked this pull request as draft November 22, 2022 02:17
@ti-chi-bot 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 Nov 22, 2022
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch 12 times, most recently from 26669bc to 25d2441 Compare November 22, 2022 05:20
@YangKeao YangKeao marked this pull request as ready for review November 22, 2022 05:21
@ti-chi-bot 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 Nov 22, 2022
@YangKeao
Copy link
Member Author

/run-all-tests

@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch from 25d2441 to 3824668 Compare November 22, 2022 07:38
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 22, 2022
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch from 3824668 to 61348ed Compare November 22, 2022 08:10
@YangKeao YangKeao requested a review from lcwangchao November 22, 2022 08:54
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch 5 times, most recently from 15f6580 to 37d8d29 Compare November 24, 2022 02:30
lcwangchao
lcwangchao approved these changes Nov 24, 2022
View reviewed changes
ddl/column.go Show resolved Hide resolved
ddl/ddl_api.go Outdated Show resolved Hide resolved
ddl/ddl_api.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 24, 2022
@lcwangchao lcwangchao self-requested a review November 24, 2022 06:42
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch 2 times, most recently from 8baa376 to 28b41b6 Compare November 24, 2022 07:40
lcwangchao
lcwangchao reviewed Nov 24, 2022
View reviewed changes
ddl/ttl.go Show resolved Hide resolved
@YangKeao
add ddl job for ttlInfo
85b0264 
Signed-off-by: YangKeao <[email protected]>
@YangKeao YangKeao force-pushed the ttl-option-in-table-info branch from 28b41b6 to 85b0264 Compare November 24, 2022 08:09
@hawkingrei
update nogo linter
e3fbb89 
Signed-off-by: Weizhen Wang <[email protected]>
@hawkingrei
Copy link
Member

/run-build

@YangKeao
Copy link
Member Author

/run-check_dev_2

@YangKeao
Copy link
Member Author

CI passed. PTAL @hawkingrei

@ti-chi-bot 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 Nov 27, 2022
@hawkingrei
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: e3fbb89

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 28, 2022
@ti-chi-bot ti-chi-bot merged commit 7f632be into pingcap:master Nov 28, 2022
@sre-bot
Copy link
Contributor

sre-bot commented Nov 28, 2022

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-ddl-test 🟥 failed 1, success 5, total 6 6 min 3 sec New failing
idc-jenkins-ci/integration-cdc-test 🟢 all 39 tests passed 20 min Existing passed
idc-jenkins-ci-tidb/integration-common-test 🟢 all 17 tests passed 16 min Existing passed
idc-jenkins-ci-tidb/common-test 🟢 all 11 tests passed 10 min Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2 🟢 all 28 tests passed 6 min 50 sec Existing passed
idc-jenkins-ci-tidb/mybatis-test 🟢 all 1 tests passed 6 min 12 sec Existing passed
idc-jenkins-ci-tidb/tics-test 🟢 all 1 tests passed 5 min 50 sec Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1 🟢 all 26 tests passed 5 min 2 sec Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test 🟢 all 1 tests passed 2 min 36 sec Existing passed
idc-jenkins-ci-tidb/plugin-test 🟢 build success, plugin test success 4min Existing passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkingrei hawkingrei hawkingrei approved these changes

@lcwangchao lcwangchao lcwangchao approved these changes

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
5 participants
@YangKeao @ti-chi-bot @hawkingrei @sre-bot @lcwangchao