localbackend: add cloud region job worker | tidb-test=ccf9f5de6817ed2e54c18f2c1757056d2c8b4e4c by D3Hunter · Pull Request #60543 · pingcap/tidb

D3Hunter · 2025-04-14T10:34:33Z

What problem does this PR solve?

Issue Number: ref #60418

Problem Summary:

What changed and how does it work?

add cloud region job worker
move executeJob from Backend to baseJobWorker

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

tiprow · 2025-04-14T10:34:49Z

Hi @D3Hunter. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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.

D3Hunter · 2025-04-14T10:38:48Z

+// If non-retryable error occurs, it will return the error.
+// If retryable error occurs, it will return nil and caller should check the stage
+// of the regionJob to determine what to do with it.
+func (w *regionJobBaseWorker) doRunJob(ctx context.Context, job *regionJob) error {


this is the previous local.executeJob

D3Hunter · 2025-04-14T10:48:11Z

+		//  worker need a lot of change before we can do it, will refactor it later.
+
+		// TODO: choose target stage based on error.
+		j.convertStageTo(needRescan)


will implement this later after the ingestcli.Client is implemented

codecov · 2025-04-14T11:02:10Z

Codecov Report

Attention: Patch coverage is 76.34409% with 66 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/ng-tidb@c7d7a96). Learn more about missing BASE report.

Additional details and impacted files

@@                 Coverage Diff                  @@
##             feature/ng-tidb     #60543   +/-   ##
====================================================
  Coverage                   ?   73.8241%           
====================================================
  Files                      ?       1742           
  Lines                      ?     481523           
  Branches                   ?          0           
====================================================
  Hits                       ?     355480           
  Misses                     ?     104077           
  Partials                   ?      21966

Flag	Coverage Δ
integration	`45.6216% <28.8209%> (?)`
unit	`72.4551% <79.2452%> (?)`

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

Components	Coverage Δ
dumpling	`52.6910% <0.0000%> (?)`
parser	`∅ <0.0000%> (?)`
br	`47.2367% <0.0000%> (?)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

D3Hunter · 2025-04-14T11:14:31Z

/retest

tiprow · 2025-04-14T11:14:57Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

D3Hunter · 2025-04-14T12:40:54Z

/retest

tiprow · 2025-04-14T12:41:16Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

lance6716 · 2025-04-15T12:37:03Z

-	doRunJobFn func(ctx context.Context, job *regionJob) error
+	writeFn  func(ctx context.Context, j *regionJob) (*tikvWriteResult, error)
+	ingestFn func(ctx context.Context, j *regionJob) error
+	runJobFn func(ctx context.Context, job *regionJob) error


My first feeling is write and ingest are sub-steps of runJob, they are not at the same level of concepts. Can you add another precheckFn and delete runJobFn for the special logic of opRegionJobWorker?

yes, writeFn/ingestFn are part of running one job, they are not part of the worker which runs many jobs, initially I tried to abstract a jobRunner, op/cloud have different implementation, and they share the same worker, but it doesn't make code more clearer, so i quit.

add another precheckFn

good idea, i will try make it a preRunJobFn

lance6716 · 2025-04-15T12:39:59Z

+			}
+		}
+
+		// if the job is empty, it might go to ingested stage directly.


this should be put above line 170?

it's trying to explain why we need to skip calling ingestFn

I hope we can refactor this part to following the state machine, now there are too many place changing fields of job

lance6716

6 / 11 files viewed

lance6716 · 2025-04-15T12:43:44Z

+				return nil
+			}
+			if res.emptyJob {
+				job.convertStageTo(ingested)


these j.convertStageTo are written outside of writeFn. But for ingestFn they are written inside ingestFn. It's not tidy.

yes, it's hard to refactor this part in a single PR, i write a todo in cloudRegionJobWorker.ingest

// TODO, we should let outer logic handle stage transition, currently, OP // worker need a lot of change before we can do it, will refactor it later.

D3Hunter · 2025-04-15T16:39:04Z

/retest

tiprow · 2025-04-15T16:39:27Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

ti-chi-bot · 2025-04-16T03:57:45Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lance6716, tangenta
Once this PR has been reviewed and has the lgtm label, please ask for approval from d3hunter, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lance6716,tangenta]
pkg/lightning/OWNERS

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 · 2025-04-16T03:57:48Z

[LGTM Timeline notifier]

Timeline:

2025-04-15 06:50:05.998201825 +0000 UTC m=+2757499.682437915: ☑️ agreed by tangenta.
2025-04-16 03:57:47.881745697 +0000 UTC m=+2833561.565981793: ☑️ agreed by lance6716.

D3Hunter · 2025-04-17T03:01:04Z

/retest

tiprow · 2025-04-17T03:01:25Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

D3Hunter · 2025-04-17T03:35:49Z

/retest

tiprow · 2025-04-17T03:36:11Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

D3Hunter · 2025-04-17T04:22:49Z

/retest

tiprow · 2025-04-17T04:23:12Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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.

change

9807d7e

ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 14, 2025

D3Hunter commented Apr 14, 2025

View reviewed changes

change

51ef9ed

D3Hunter requested a review from lance6716 April 14, 2025 10:45

D3Hunter commented Apr 14, 2025

View reviewed changes

lint

5d74f5e

tangenta approved these changes Apr 15, 2025

View reviewed changes

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 15, 2025

lance6716 reviewed Apr 15, 2025

View reviewed changes

D3Hunter added 2 commits April 15, 2025 22:34

fix comments

14fcb70

test

bd80f18

lance6716 approved these changes Apr 16, 2025

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 16, 2025

ti-chi-bot added the approved label Apr 16, 2025

D3Hunter changed the title ~~localbackend: add cloud region job worker~~ localbackend: add cloud region job worker | tidb-test=ccf9f5de6817ed2e54c18f2c1757056d2c8b4e4c Apr 17, 2025

ti-chi-bot Bot merged commit f234219 into pingcap:feature/ng-tidb Apr 17, 2025

D3Hunter deleted the cloud-worker branch April 17, 2025 05:03

D3Hunter mentioned this pull request Jun 12, 2025

add-index/import-into on global sort for next generation TiDB #60418

Closed

29 tasks

Uh oh!

Conversation

D3Hunter commented Apr 14, 2025

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow Bot commented Apr 14, 2025

Uh oh!

D3Hunter Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

D3Hunter commented Apr 14, 2025

Uh oh!

tiprow Bot commented Apr 14, 2025

Uh oh!

D3Hunter commented Apr 14, 2025

Uh oh!

tiprow Bot commented Apr 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented Apr 15, 2025

Uh oh!

tiprow Bot commented Apr 15, 2025

Uh oh!

ti-chi-bot Bot commented Apr 16, 2025

Uh oh!

ti-chi-bot Bot commented Apr 16, 2025

[LGTM Timeline notifier]

Uh oh!

D3Hunter commented Apr 17, 2025

Uh oh!

tiprow Bot commented Apr 17, 2025

Uh oh!

D3Hunter commented Apr 17, 2025

Uh oh!

tiprow Bot commented Apr 17, 2025

Uh oh!

D3Hunter commented Apr 17, 2025

Uh oh!

tiprow Bot commented Apr 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

D3Hunter Apr 14, 2025 •

edited

Loading

codecov Bot commented Apr 14, 2025 •

edited

Loading