dm: add a stand-alone load mode #11749

OliverS929 · 2024-11-14T13:08:54Z

What problem does this PR solve?

Issue Number: close #9230

What is changed and how it works?

Primary work to add a stand-alone load mode to DM, alongside with dump mode. Load&Sync mode might not be needed anymore and could be depreciated in the future.

UT/IT added are under testing and will be pushed soon.

Continue the work from #11738 as its intended change in openapi conflicted with that of #11729.

Check List

Tests

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

Questions

Will it cause performance regression or break compatibility?

N/A

Do you need to update user documentation, design documentation or monitoring documentation?

Yes. Description of those standalone modes would need to be added to docs for openapi only. Support for dmctl will be delayed further.

Release note

Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note.

If you don't think this PR needs a release note then fill it with `None`.

codecov · 2024-11-14T13:31:09Z

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 55.1530%. Comparing base (cc6fa6e) to head (6ec4675).
Report is 4 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files

Components	Coverage Δ
cdc	`59.5582% <ø> (-0.0188%)`	⬇️
dm	`50.0644% <57.1428%> (+0.0038%)`	⬆️
engine	`53.2336% <ø> (ø)`

Flag	Coverage Δ
unit	`55.1530% <57.1428%> (-0.0070%)`	⬇️

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

@@               Coverage Diff                @@
##             master     #11749        +/-   ##
================================================
- Coverage   55.1599%   55.1530%   -0.0070%     
================================================
  Files          1003       1003                
  Lines        137462     137492        +30     
================================================
+ Hits          75824      75831         +7     
- Misses        56091      56106        +15     
- Partials       5547       5555         +8

OliverS929 · 2024-11-14T15:25:33Z

/test engine-integration-test

OliverS929 · 2024-11-14T16:36:30Z

/test dm-integration-test

OliverS929 · 2024-11-15T04:23:41Z

/retest

River2000i · 2024-11-15T05:32:06Z

dm/tests/openapi/run.sh

+	run_dm_ctl_with_retry $WORK_DIR "127.0.0.1:$MASTER_PORT" \
+		"query-status $task_name_load" 100 \
+		"\"stage\": \"Finished\"" 1


how about adding the sync task after load task finish? For test the complete create DM task

Yes, we can definitely do that. I actually have a test written in E2E, which I'll submit in a separate PR. How about I also add that test to the OpenAPI test suite along with the E2E PR? Additionally, we could merge the dump test with the existing load test, as the load test above already covers the dump task.

engine/jobmaster/dm/task_manager.go

engine/jobmaster/dm/worker_manager.go

OliverS929 · 2024-11-18T11:45:06Z

/test dm-integration-test

dm/tests/openapi/run.sh

D3Hunter · 2024-11-20T09:25:58Z

cc @lance6716

not be needed anymore and could be depreciated in the future.

purelind · 2024-12-04T15:28:43Z

/test dm-compatibility-test

OliverS929 · 2024-12-05T00:59:57Z

/retest

River2000i

lgtm

ti-chi-bot · 2024-12-05T04:38:04Z

@River2000i: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

lgtm

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.

River2000i · 2024-12-05T04:40:50Z

dm/tests/openapi/run.sh

+	run_dm_ctl_with_retry $WORK_DIR "127.0.0.1:$MASTER_PORT" \
+		"query-status $task_name_dump" \
+		"\"stage\": \"Running\"" 1


maybe cannot get the Running stage, since sometime it will finish immediately. I think we can remove check Running

Also i have add a function to check stage by openapi. In this case using openapi check stage will be better

Sure. I remove the running stage check for dump. I also add back the check_dump_task_finished_status_success, which I think is the function you mentioned to check stage.

OliverS929 · 2024-12-05T07:58:12Z

/retest

OliverS929 · 2024-12-05T09:06:18Z

/retest

OliverS929 · 2024-12-05T10:16:10Z

/retest

dm/tests/openapi/run.sh

ti-chi-bot · 2024-12-06T01:21:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: D3Hunter, lance6716, River2000i
Once this PR has been reviewed and has the lgtm label, please assign benmeadowcroft for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

~~dm/OWNERS~~ [D3Hunter,lance6716]
dm/config/OWNERS
~~engine/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-12-06T01:21:02Z

[LGTM Timeline notifier]

Timeline:

2024-12-05 02:14:10.568441268 +0000 UTC m=+1293838.188095785: ☑️ agreed by D3Hunter.
2024-12-06 01:21:01.181144676 +0000 UTC m=+1377048.800799191: ☑️ agreed by lance6716.

OliverS929 · 2024-12-11T01:05:11Z

/cc @alastori @benmeadowcroft PTAL

ti-chi-bot · 2024-12-11T01:05:14Z

@OliverS929: GitHub didn't allow me to request PR reviews from the following users: alastori, BenMeadowcroft, PTAL.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @alastori @benmeadowcroft PTAL

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.

alastori · 2024-12-11T01:26:56Z

lgtm

OliverS929 force-pushed the 2411_dm_modes_rebase branch from d239bf5 to aacf64d Compare November 15, 2024 03:03

River2000i reviewed Nov 15, 2024

View reviewed changes

D3Hunter changed the title ~~Add a stand-alone load mode to DM.~~ dm: add a stand-alone load mode Nov 15, 2024

D3Hunter reviewed Nov 15, 2024

View reviewed changes

engine/jobmaster/dm/task_manager.go Outdated Show resolved Hide resolved

engine/jobmaster/dm/worker_manager.go Outdated Show resolved Hide resolved

OliverS929 mentioned this pull request Nov 15, 2024

[WIP] DM stand-alone dump/load mode and openapi sync stage statusr esponse improvement with TCP connection IO traffic statistics #11754

Open

D3Hunter reviewed Nov 20, 2024

View reviewed changes

dm/tests/openapi/run.sh Outdated Show resolved Hide resolved

D3Hunter reviewed Nov 20, 2024

View reviewed changes

dm/tests/openapi/run.sh Outdated Show resolved Hide resolved

OliverS929 added 12 commits December 4, 2024 13:39

Primary work to add a stand-alone load mode to DM. Load&Sync mode might

3a91c58

not be needed anymore and could be depreciated in the future.

Fix format.

6f2d0da

Update error.txt

7fb9591

Add stand-alone load mode into openapi.

05dbe7e

Rewrite load testing based on what we have already with dump.

934dc3f

Fix fmt issue with gen.go's.

4eebe9a

Fix openapi test case.

cfe0246

Typo fix.

feeeb0c

One source for openapi test is enough.

ee962e7

load and load&sync should not use subtask adjust.

2bf7968

Correct taskmanager cause sync mode never finish.

46cfc60

Revert changes done to engine for the moment.

c63d050

Dont check the Running stage for load.

b112f53

Dont do routings over target.

1f1cfe8

OliverS929 force-pushed the 2411_dm_modes_rebase branch from c28573a to 1f1cfe8 Compare December 5, 2024 00:06

D3Hunter approved these changes Dec 5, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 5, 2024

OliverS929 requested a review from River2000i December 5, 2024 02:15

River2000i approved these changes Dec 5, 2024

View reviewed changes

River2000i reviewed Dec 5, 2024

View reviewed changes

Remove running check from dump step as well, and add openapi part.

89dd7e0

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 5, 2024

Change task name for api check.

e40b180

River2000i reviewed Dec 5, 2024

View reviewed changes

dm/tests/openapi/run.sh Outdated Show resolved Hide resolved

Revert debug changes.

6ec4675

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 6, 2024

lance6716 approved these changes Dec 6, 2024

View reviewed changes

ti-chi-bot bot added the lgtm label Dec 6, 2024

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dm: add a stand-alone load mode #11749

dm: add a stand-alone load mode #11749

OliverS929 commented Nov 14, 2024

codecov bot commented Nov 14, 2024 •

edited

Loading

OliverS929 commented Nov 14, 2024

OliverS929 commented Nov 14, 2024

OliverS929 commented Nov 15, 2024

River2000i Nov 15, 2024

OliverS929 Nov 15, 2024

OliverS929 commented Nov 18, 2024

D3Hunter commented Nov 20, 2024

purelind commented Dec 4, 2024

OliverS929 commented Dec 5, 2024

River2000i left a comment

ti-chi-bot bot commented Dec 5, 2024

River2000i Dec 5, 2024

River2000i Dec 5, 2024

OliverS929 Dec 5, 2024

OliverS929 commented Dec 5, 2024

OliverS929 commented Dec 5, 2024

OliverS929 commented Dec 5, 2024

ti-chi-bot bot commented Dec 6, 2024

ti-chi-bot bot commented Dec 6, 2024

OliverS929 commented Dec 11, 2024

ti-chi-bot bot commented Dec 11, 2024

alastori commented Dec 11, 2024

dm: add a stand-alone load mode #11749

Are you sure you want to change the base?

dm: add a stand-alone load mode #11749

Conversation

OliverS929 commented Nov 14, 2024

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

codecov bot commented Nov 14, 2024 • edited Loading

Codecov Report

OliverS929 commented Nov 14, 2024

OliverS929 commented Nov 14, 2024

OliverS929 commented Nov 15, 2024

River2000i Nov 15, 2024

Choose a reason for hiding this comment

OliverS929 Nov 15, 2024

Choose a reason for hiding this comment

OliverS929 commented Nov 18, 2024

D3Hunter commented Nov 20, 2024

purelind commented Dec 4, 2024

OliverS929 commented Dec 5, 2024

River2000i left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Dec 5, 2024

River2000i Dec 5, 2024

Choose a reason for hiding this comment

River2000i Dec 5, 2024

Choose a reason for hiding this comment

OliverS929 Dec 5, 2024

Choose a reason for hiding this comment

OliverS929 commented Dec 5, 2024

OliverS929 commented Dec 5, 2024

OliverS929 commented Dec 5, 2024

ti-chi-bot bot commented Dec 6, 2024

ti-chi-bot bot commented Dec 6, 2024

[LGTM Timeline notifier]

OliverS929 commented Dec 11, 2024

ti-chi-bot bot commented Dec 11, 2024

alastori commented Dec 11, 2024

codecov bot commented Nov 14, 2024 •

edited

Loading