ddl: refactor V2 job args of dropping column DDL. #56021

joccau · 2024-09-11T09:38:08Z

What problem does this PR solve?

Issue Number: Ref #53930
Ref #54436

Problem Summary:

What changed and how does it work?

Refactor V2 job args for ActionDropColumn DDL.
Set jobVersion1 at ActionDropDolumn temporarily, set version2 when refactor ActionMultiSchemaChange

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

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.

None

Signed-off-by: joccau <[email protected]>

codecov · 2024-09-11T09:54:46Z

Codecov Report

Attention: Patch coverage is 85.85859% with 14 lines in your changes missing coverage. Please review.

Project coverage is 56.9794%. Comparing base (67bee7c) to head (1b87a3a).
Report is 8 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #56021         +/-   ##
=================================================
- Coverage   73.4436%   56.9794%   -16.4643%     
=================================================
  Files          1600       1758        +158     
  Lines        443578     630677     +187099     
=================================================
+ Hits         325780     359356      +33576     
- Misses        97829     246684     +148855     
- Partials      19969      24637       +4668

Flag	Coverage Δ
integration	`39.5663% <74.7474%> (?)`
unit	`72.6389% <85.8585%> (+0.1280%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.9567% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`63.2810% <ø> (+17.3880%)`	⬆️

Signed-off-by: joccau <[email protected]>

…-args

hawkingrei · 2024-09-14T08:20:16Z

/retest

hawkingrei · 2024-09-14T08:29:18Z

/retest

Signed-off-by: joccau <[email protected]>

joccau · 2024-09-14T12:46:38Z

/retest

Signed-off-by: joccau <[email protected]>

joccau · 2024-09-15T07:32:42Z

/retest

joccau · 2024-09-16T02:30:07Z

/retest

joccau · 2024-09-16T02:50:54Z

/retest

Signed-off-by: joccau <[email protected]>

lance6716

/hold

please unhold at your will

lance6716 · 2024-09-20T02:53:57Z

pkg/meta/model/job.go

+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return rawArgs, nil


Suggested change

if err != nil {

return nil, errors.Trace(err)

}

return rawArgs, nil

return rawArgs, errors.Trace(err)

can it be passed by lint-check? I can try it.

pkg/meta/model/job_args.go

ti-chi-bot · 2024-09-20T02:56:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, lance6716

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/ddl/OWNERS~~ [D3Hunter,lance6716]
~~pkg/meta/OWNERS~~ [D3Hunter,lance6716]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-09-20T02:56:46Z

[LGTM Timeline notifier]

Timeline:

2024-09-20 02:10:00.253216549 +0000 UTC m=+1186269.993640491: ☑️ agreed by D3Hunter.
2024-09-20 02:56:45.824998576 +0000 UTC m=+1189075.565422514: ☑️ agreed by lance6716.

joccau · 2024-09-20T03:21:26Z

/test pull-br-integration-test

tiprow · 2024-09-20T03:21:31Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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 kubernetes-sigs/prow repository.

Signed-off-by: joccau <[email protected]>

joccau · 2024-09-20T04:10:43Z

/test check-dev2

tiprow · 2024-09-20T04:10:48Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test check-dev2

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 kubernetes-sigs/prow repository.

joccau · 2024-09-20T04:10:49Z

/test pull-br-integration-test

ti-chi-bot · 2024-09-20T04:10:52Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test pull-br-integration-test
/test pull-integration-ddl-test
/test pull-lightning-integration-test
/test pull-mysql-client-test
/test unit-test

The following commands are available to trigger optional jobs:

/test canary-notify-when-compatibility-sections-changed
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-nodejs-test
/test pull-sqllogic-test
/test pull-tiflash-test

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test
pingcap/tidb/pull_br_integration_test
pingcap/tidb/pull_integration_ddl_test
pingcap/tidb/pull_lightning_integration_test
pingcap/tidb/pull_mysql_client_test

In response to this:

/test pull-br-integration-test

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 kubernetes/test-infra repository.

tiprow · 2024-09-20T04:10:55Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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 kubernetes-sigs/prow repository.

joccau · 2024-09-20T05:07:08Z

/test unit-test

tiprow · 2024-09-20T05:07:13Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test unit-test

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 kubernetes-sigs/prow repository.

joccau · 2024-09-20T05:07:15Z

/test pull-br-integration-test

tiprow · 2024-09-20T05:07:21Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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 kubernetes-sigs/prow repository.

joccau · 2024-09-20T07:35:46Z

/test pull-lightning-integration-test

tiprow · 2024-09-20T07:35:52Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-lightning-integration-test

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 kubernetes-sigs/prow repository.

ref pingcap#53930

refactor job args of dropping column DDL

e258be2

Signed-off-by: joccau <[email protected]>

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Sep 11, 2024

maintain ut

eb5bad5

Signed-off-by: joccau <[email protected]>

joccau force-pushed the drop-column-args branch from 641f0ae to eb5bad5 Compare September 14, 2024 04:58

add unit test

d89cef9

Signed-off-by: joccau <[email protected]>

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Sep 14, 2024

joccau force-pushed the drop-column-args branch from 4327321 to 5c60112 Compare September 14, 2024 06:31

make bazel

875193f

Signed-off-by: joccau <[email protected]>

joccau force-pushed the drop-column-args branch from 5c60112 to 875193f Compare September 14, 2024 06:53

Merge remote-tracking branch 'remotes/origin/master' into drop-column…

b740774

…-args

joccau force-pushed the drop-column-args branch from 5a0a671 to b740774 Compare September 14, 2024 07:35

make test

22ad81b

Signed-off-by: joccau <[email protected]>

joccau force-pushed the drop-column-args branch from b2cf4a8 to 22ad81b Compare September 14, 2024 09:20

modify code

82abe40

Signed-off-by: joccau <[email protected]>

joccau force-pushed the drop-column-args branch 2 times, most recently from 472a94d to 0f08ae8 Compare September 15, 2024 07:32

joccau force-pushed the drop-column-args branch 2 times, most recently from ddd2474 to 8bad48a Compare September 16, 2024 02:29

joccau force-pushed the drop-column-args branch from 8bad48a to 2effe55 Compare September 16, 2024 02:50

joccau requested a review from D3Hunter September 20, 2024 01:56

D3Hunter approved these changes Sep 20, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 20, 2024

make bazel

314136e

Signed-off-by: joccau <[email protected]>

lance6716 approved these changes Sep 20, 2024

View reviewed changes

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2024

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 20, 2024

deal comments in pr

65d67a8

Signed-off-by: joccau <[email protected]>

Merge remote-tracking branch 'origin/master' into drop-column-args

1b87a3a

joccau removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2024

ti-chi-bot bot merged commit 6bdf685 into pingcap:master Sep 20, 2024
25 checks passed

D3Hunter mentioned this pull request Sep 23, 2024

ddl code refactor/optimize #54436

Open

74 tasks

winoros pushed a commit to winoros/tidb that referenced this pull request Sep 23, 2024

ddl: refactor V2 job args of dropping column DDL. (pingcap#56021)

e9b21b6

ref pingcap#53930

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: refactor V2 job args of dropping column DDL. #56021

ddl: refactor V2 job args of dropping column DDL. #56021

joccau commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

hawkingrei commented Sep 14, 2024

hawkingrei commented Sep 14, 2024

joccau commented Sep 14, 2024

joccau commented Sep 15, 2024

joccau commented Sep 16, 2024

joccau commented Sep 16, 2024

lance6716 left a comment

lance6716 Sep 20, 2024

joccau Sep 20, 2024

joccau Sep 20, 2024

ti-chi-bot bot commented Sep 20, 2024

ti-chi-bot bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

ti-chi-bot bot commented Sep 20, 2024

/test pull-br-integration-test

tiprow bot commented Sep 20, 2024

/test pull-br-integration-test

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

ddl: refactor V2 job args of dropping column DDL. #56021

ddl: refactor V2 job args of dropping column DDL. #56021

Conversation

joccau commented Sep 11, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

hawkingrei commented Sep 14, 2024

hawkingrei commented Sep 14, 2024

joccau commented Sep 14, 2024

joccau commented Sep 15, 2024

joccau commented Sep 16, 2024

joccau commented Sep 16, 2024

lance6716 left a comment

Choose a reason for hiding this comment

lance6716 Sep 20, 2024

Choose a reason for hiding this comment

joccau Sep 20, 2024

Choose a reason for hiding this comment

joccau Sep 20, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 20, 2024

ti-chi-bot bot commented Sep 20, 2024

[LGTM Timeline notifier]

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

/test pull-br-integration-test

ti-chi-bot bot commented Sep 20, 2024

/test pull-br-integration-test

tiprow bot commented Sep 20, 2024

/test pull-br-integration-test

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 20, 2024

tiprow bot commented Sep 20, 2024

joccau commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading