ddl: Increase ddl unit test speed #41034

blacktear23 · 2023-02-03T07:00:14Z

What problem does this PR solve?

Issue Number: close #41216

Problem Summary:
make test or make ut is too slow, sometimes timeout after 10min

What is changed and how it works?

Update some ticker and wait time to shorten the wait time for unit test.

domain.Reload should notify mdlCheckLoop
DDL's job dispatch loop should decrease sleep time when owner or waiting check. (In test case, isOwner will return false sometimes after create the testkit.)

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.

None

ti-chi-bot · 2023-02-03T07:00:16Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

hawkingrei
tangenta

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 · 2023-02-03T07:00:24Z

Hi @blacktear23. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

bb7133 · 2023-02-07T02:49:25Z

PTAL @wjhuang2016 @zimulala

hawkingrei · 2023-02-13T05:54:00Z

/check-issue-triage-complete

ddl/ddl_worker.go

hawkingrei · 2023-02-18T12:57:26Z

/ok-to-test

blacktear23 · 2023-02-19T10:00:44Z

/ok-to-test

hawkingrei · 2023-02-19T10:09:32Z

/ok-to-test

It is unnecessary for you. It is for pingcap's staff members to allow contributors to run CI tasks for safety factors. Now you can run tasks by yourself.

hawkingrei · 2023-02-20T03:14:11Z

/retest

hawkingrei · 2023-02-24T01:15:17Z

/retest

blacktear23 · 2023-02-24T03:06:53Z

domain/domain.go

@@ -711,78 +712,86 @@ func (do *Domain) refreshMDLCheckTableInfo() {
 }

 func (do *Domain) mdlCheckLoop() {
-	ticker := time.Tick(time.Millisecond * 50)
+	var ticker <-chan time.Time
+	if intest.InTest {


If we provide a notify after MDL updated is change the tick duration in test required? @hawkingrei

I think it is possible to implement it.

hawkingrei · 2023-02-27T05:58:48Z

/retest

hawkingrei · 2023-02-27T09:32:09Z

PTAL @wjhuang2016 @zimulala

zimulala · 2023-02-27T09:37:13Z

ddl/job_table.go

@@ -174,15 +175,24 @@ func (d *ddl) startDispatchLoop() {
 	if d.etcdCli != nil {
 		notifyDDLJobByEtcdCh = d.etcdCli.Watch(d.ctx, addingDDLJobConcurrent)
 	}
-	ticker := time.NewTicker(1 * time.Second)
+	var ticker *time.Ticker
+	if intest.InTest {


Personally, I think it is ugly to introduce inTest into the code, should it be directly used as a variable, such as xxxInterval, and then directly change this variable for testing, rather than into the real code

Agreed. And by the way, why sleep for 1 second? Why not some formula with some variables like nn * TTL

The original code is https://github.com/pingcap/tidb/blob/release-6.4/ddl/ddl_worker.go#L182. Fine-tuning may have been done to support parallel DDL. I think it should be similar to the original logic implementation, which has some association with TTL (lease).

If there is any suggestion, can you propose a new PR?

domain/domain.go

blacktear23 · 2023-03-01T11:15:16Z

/retest

blacktear23 · 2023-03-01T13:05:23Z

Is anyone has any idea about why the unit-test always fail?

hawkingrei · 2023-03-01T13:17:06Z

/retest

ddl/job_table.go

hawkingrei · 2023-03-06T11:54:27Z

/retest

hawkingrei · 2023-03-06T13:20:54Z

/merge

ti-chi-bot · 2023-03-06T13:20:58Z

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

Commit hash: c8132d0

hawkingrei · 2023-03-07T07:45:36Z

/retest

hawkingrei · 2023-03-07T10:34:00Z

/test

ti-chi-bot · 2023-03-07T10:34:02Z

@hawkingrei: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test unit-test

Use /test all to run all jobs.

In response to this:

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

hawkingrei · 2023-03-07T10:34:30Z

/retest

Increase ddl unit test speed

9ca6838

Update makefile

043c0d2

hawkingrei requested review from wjhuang2016 and zimulala February 3, 2023 07:20

blacktear23 added 2 commits February 3, 2023 19:22

Merge branch 'master' into increase-ddl-test-speed

c40de38

Ignore job error sleep wait in test and job is cancelling

38e0244

Merge branch 'master' into increase-ddl-test-speed

406da79

ti-chi-bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Feb 9, 2023

Build all test binary should add intest tags

4cd4997

ti-chi-bot removed the do-not-merge/needs-triage-completed label Feb 13, 2023

Merge branch 'master' into increase-ddl-test-speed

c594c86

bb7133 reviewed Feb 18, 2023

View reviewed changes

ddl/ddl_worker.go Outdated Show resolved Hide resolved

Remove unused code

32cc979

ti-chi-bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Feb 18, 2023

Fix lint

8245f6a

Try to fix data race

4e11390

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

blacktear23 commented Feb 24, 2023

View reviewed changes

tangenta approved these changes Feb 27, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 27, 2023

hawkingrei approved these changes Feb 27, 2023

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 Feb 27, 2023

zimulala reviewed Feb 27, 2023

View reviewed changes

Refactor code

9480aaa

zimulala reviewed Feb 28, 2023

View reviewed changes

domain/domain.go Outdated Show resolved Hide resolved

Refactor mdl check loop duration

de6b78f

zimulala approved these changes Feb 28, 2023

View reviewed changes

blacktear23 commented Mar 1, 2023

View reviewed changes

ddl/job_table.go Outdated Show resolved Hide resolved

Change wait duration for ddl dispatch for unit test

c8132d0

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 6, 2023

ti-chi-bot merged commit 29adb0a into pingcap:master Mar 7, 2023

Defined2014 mentioned this pull request Oct 8, 2023

ddl: no more sleep when meets nonRetryable error #47461

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: Increase ddl unit test speed #41034

ddl: Increase ddl unit test speed #41034

blacktear23 commented Feb 3, 2023 •

edited

Loading

ti-chi-bot commented Feb 3, 2023 •

edited

Loading

ti-chi-bot commented Feb 3, 2023

bb7133 commented Feb 7, 2023

hawkingrei commented Feb 13, 2023

hawkingrei commented Feb 18, 2023

blacktear23 commented Feb 19, 2023

hawkingrei commented Feb 19, 2023

hawkingrei commented Feb 20, 2023

hawkingrei commented Feb 24, 2023

blacktear23 Feb 24, 2023

hawkingrei Feb 27, 2023

hawkingrei commented Feb 27, 2023

hawkingrei commented Feb 27, 2023

zimulala Feb 27, 2023

blacktear23 Feb 27, 2023

zimulala Feb 27, 2023

zimulala Feb 27, 2023

blacktear23 commented Mar 1, 2023

blacktear23 commented Mar 1, 2023

hawkingrei commented Mar 1, 2023

hawkingrei commented Mar 6, 2023

hawkingrei commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

hawkingrei commented Mar 7, 2023

hawkingrei commented Mar 7, 2023

ti-chi-bot commented Mar 7, 2023

hawkingrei commented Mar 7, 2023

ddl: Increase ddl unit test speed #41034

ddl: Increase ddl unit test speed #41034

Conversation

blacktear23 commented Feb 3, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Feb 3, 2023 • edited Loading

ti-chi-bot commented Feb 3, 2023

bb7133 commented Feb 7, 2023

hawkingrei commented Feb 13, 2023

hawkingrei commented Feb 18, 2023

blacktear23 commented Feb 19, 2023

hawkingrei commented Feb 19, 2023

hawkingrei commented Feb 20, 2023

hawkingrei commented Feb 24, 2023

blacktear23 Feb 24, 2023

Choose a reason for hiding this comment

hawkingrei Feb 27, 2023

Choose a reason for hiding this comment

hawkingrei commented Feb 27, 2023

hawkingrei commented Feb 27, 2023

zimulala Feb 27, 2023

Choose a reason for hiding this comment

blacktear23 Feb 27, 2023

Choose a reason for hiding this comment

zimulala Feb 27, 2023

Choose a reason for hiding this comment

zimulala Feb 27, 2023

Choose a reason for hiding this comment

blacktear23 commented Mar 1, 2023

blacktear23 commented Mar 1, 2023

hawkingrei commented Mar 1, 2023

hawkingrei commented Mar 6, 2023

hawkingrei commented Mar 6, 2023

ti-chi-bot commented Mar 6, 2023

hawkingrei commented Mar 7, 2023

hawkingrei commented Mar 7, 2023

ti-chi-bot commented Mar 7, 2023

hawkingrei commented Mar 7, 2023

blacktear23 commented Feb 3, 2023 •

edited

Loading

ti-chi-bot commented Feb 3, 2023 •

edited

Loading