lightning: enhance import into backend for dm by GMHDBJD · Pull Request #65473 · pingcap/tidb

GMHDBJD · 2026-01-08T04:20:50Z

What problem does this PR solve?

Issue Number: ref #65092

Problem Summary:

What changed and how does it work?

Updated JobOrchestrator to handle job submission errors gracefully, ensuring submitted jobs are cancelled if submission fails.
Introduced job progress estimator to calculate job progress based on processed and total sizes.
Implemented logging redaction for sensitive information in SQL commands during job submission.
fix mysql checkpoint bug.

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 · 2026-01-08T04:21:11Z

Hi @GMHDBJD. 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.

codecov · 2026-01-08T05:11:07Z

Codecov Report

❌ Patch coverage is 76.39257% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.3966%. Comparing base (7f129e5) to head (433dd1a).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #65473        +/-   ##
================================================
+ Coverage   77.8399%   78.3966%   +0.5567%     
================================================
  Files          1983       1914        -69     
  Lines        542760     532288     -10472     
================================================
- Hits         422484     417296      -5188     
+ Misses       118616     114556      -4060     
+ Partials       1660        436      -1224

Flag	Coverage Δ
integration	`44.3453% <41.7867%> (-3.8424%)`	⬇️
unit	`76.8102% <73.4748%> (+0.3463%)`	⬆️

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

Components	Coverage Δ
dumpling	`56.7974% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.7502% <ø> (-12.2964%)`	⬇️

🚀 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

rest lgtm

lightning/pkg/importinto/checkpoint.go

lightning/pkg/importinto/importer.go

lightning/pkg/importinto/job_monitor_test.go

GMHDBJD · 2026-01-09T05:57:31Z

/hold Progress regression detected.

…riod and progress tracking - Added cancellation grace period and polling interval for job cancellation by group key. - Updated JobOrchestrator to handle job submission errors gracefully, ensuring submitted jobs are cancelled if submission fails. - Introduced job progress estimator to calculate job progress based on processed and total sizes. - Implemented logging redaction for sensitive information in SQL commands during job submission. - Added tests for job progress estimation, job submission error handling, and job cancellation scenarios. - Created a new ProgressUpdater interface to facilitate progress updates during job execution.

… error handling

lightning/pkg/importinto/job_orchestrator.go

D3Hunter · 2026-01-15T02:35:20Z

lightning/pkg/importinto/job_orchestrator.go

 				}

-				if err := o.recordSubmission(egCtx, job); err != nil {
+				cpCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), cancelTimeout)


what's the difference between WithoutCancel and background ctx?

and why we have to add a timeout for recording submission?

WithoutCancel will retain context values like logging and tracing, change to background to avoid confuse.

Timeout is to avoid the import hang due to network/db/lock, so I add a timeout here.

lightning/pkg/importinto/job_submitter.go

D3Hunter · 2026-01-15T02:45:38Z

lightning/pkg/importinto/job_progress.go

+	return &jobProgressEstimator{logger: logger}
+}
+
+func (e *jobProgressEstimator) parseHumanSize(jobID int64, sizeText string, warnMsg string) (int64, bool) {


we can avoid those pause if we return the number directly in SHOW RAW IMPORT JOB as a JSON field
we can enhance this later

lightning/pkg/importinto/job_progress.go

lightning/pkg/importinto/job_orchestrator.go

lightning/pkg/importinto/job_monitor.go

Copilot

Pull request overview

This PR enhances the import-into backend for DM (Data Migration) by adding robust job management capabilities. It introduces progress tracking, secure logging with credential redaction, graceful error handling during job submission, and failover-aware cancellation support.

Changes:

Added job progress estimator to calculate and report import progress based on job phases and step completion
Implemented SQL redaction to hide sensitive credentials (access keys, secret keys) in logs
Enhanced job orchestrator with graceful cancellation including grace period and polling for late-appearing jobs
Added failover-aware cancellation that preserves running jobs during DM worker transitions
Fixed MySQL checkpoint schema bug by removing redundant db_name column and using composite table_name

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lightning/pkg/importinto/job_progress.go	Implements progress estimation logic for global-sort and non-global-sort import phases
lightning/pkg/importinto/job_progress_test.go	Tests for progress calculation with various job states and phase transitions
lightning/pkg/server/lightning.go	Integrates progress adapter to bridge LightningStatus with ProgressUpdater interface
lightning/pkg/importinto/job_submitter.go	Adds SQL redaction using regex to mask credentials in cloud storage URLs
lightning/pkg/importinto/job_submitter_test.go	Validates that sensitive credentials are properly redacted in logs
lightning/pkg/importinto/job_orchestrator.go	Enhances error handling with automatic cancellation of submitted jobs on failure and adds grace period polling
lightning/pkg/importinto/job_orchestrator_test.go	Tests cancellation scenarios including late-appearing jobs and submission errors
lightning/pkg/importinto/job_monitor.go	Integrates progress estimator and removes inline cancellation logic (delegated to orchestrator)
lightning/pkg/importinto/job_monitor_test.go	Updates tests to verify progress tracking and non-rollback behavior
lightning/pkg/importinto/importer.go	Adds failover cancellation support via ErrFailoverCancel to preserve jobs during transitions
lightning/pkg/importinto/importer_test.go	Tests both user-initiated and failover-triggered cancellation paths
lightning/pkg/importinto/checkpoint.go	Fixes schema by removing db_name column and making table_name the sole primary key
lightning/pkg/importinto/BUILD.bazel	Adds new dependencies and increases test shard count for new test files
lightning/pkg/importinto/mock/import_mock.go	Adds MockProgressUpdater for testing progress tracking
Makefile	Updates mock generation command to include ProgressUpdater interface

lightning/pkg/importinto/job_orchestrator.go

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Copilot · 2026-01-19T09:15:31Z

lightning/pkg/importinto/checkpoint.go

 	createTableSQL := fmt.Sprintf(`CREATE TABLE IF NOT EXISTS %s.%s (
-		db_name VARCHAR(64) NOT NULL,
-		table_name VARCHAR(64) NOT NULL,
+		table_name VARCHAR(256) NOT NULL,
 		job_id BIGINT NOT NULL,
 		status TINYINT NOT NULL,
 		message TEXT,
 		group_key VARCHAR(128),
 		update_time TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
-		PRIMARY KEY (db_name, table_name)
+		PRIMARY KEY (table_name)
 	)`, common.EscapeIdentifier(m.schemaName), common.EscapeIdentifier(m.tableName))


The checkpoint table schema has been changed from having separate db_name and table_name columns with a composite primary key to a single table_name column. However, the CREATE TABLE IF NOT EXISTS statement won't alter existing tables. If users upgrade from a previous version with the old checkpoint schema, they'll encounter issues because the code expects the new schema but the existing table has the old schema. Consider adding migration logic or documenting this as a breaking change that requires manual checkpoint table recreation.

Replace local clamp01 helper with mathutil.Clamp.

ti-chi-bot · 2026-01-19T09:57:12Z

[LGTM Timeline notifier]

Timeline:

2026-01-09 02:40:51.051519219 +0000 UTC m=+929206.869827651: ☑️ agreed by joechenrh.
2026-01-19 09:57:10.940084696 +0000 UTC m=+401458.554041542: ☑️ agreed by D3Hunter.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Copilot · 2026-01-19T10:12:52Z

lightning/pkg/importinto/checkpoint.go

 	createTableSQL := fmt.Sprintf(`CREATE TABLE IF NOT EXISTS %s.%s (
-		db_name VARCHAR(64) NOT NULL,
-		table_name VARCHAR(64) NOT NULL,
+		table_name VARCHAR(256) NOT NULL,


The table_name column is being changed from VARCHAR(64) to VARCHAR(256), which may not be sufficient for all cases. The UniqueTable function formats table names as backtick-quoted identifiers in the form `schema`.`table`.

In MySQL, identifiers can be up to 64 characters each, so a fully qualified table name with backticks would be: `{64 chars}`.`{64 chars}` = 64 + 1 + 64 + 4 backticks = 133 characters minimum. However, backticks within identifiers are escaped as double backticks, so in the worst case (all 64 characters are backticks), you could have: `{128 chars}`.`{128 chars}` = 128 + 1 + 128 + 4 = 261 characters, which exceeds VARCHAR(256).

Consider increasing the column size to VARCHAR(300) or VARCHAR(512) to safely accommodate escaped identifiers.

Suggested change

table_name VARCHAR(256) NOT NULL,

table_name VARCHAR(512) NOT NULL,

GMHDBJD · 2026-01-19T13:33:35Z

/retest

tiprow · 2026-01-19T13:34:00Z

@GMHDBJD: 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.

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.

Benjamin2037

LGTM

ti-chi-bot · 2026-01-20T03:36:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, D3Hunter, joechenrh

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Benjamin2037,D3Hunter,joechenrh]
~~lightning/OWNERS~~ [Benjamin2037,D3Hunter]

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

Benjamin2037 · 2026-01-20T03:43:20Z

/retest

GMHDBJD · 2026-01-20T06:21:11Z

/unhold

GMHDBJD · 2026-01-20T07:11:06Z

/retest

GMHDBJD · 2026-01-20T07:15:17Z

/retest

GMHDBJD · 2026-01-20T07:42:19Z

/retest

GMHDBJD · 2026-01-20T07:44:25Z

/retest

GMHDBJD · 2026-01-20T08:21:36Z

/retest

GMHDBJD · 2026-01-20T08:57:35Z

/retest

tiprow · 2026-01-20T08:58:00Z

@GMHDBJD: 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.

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.

GMHDBJD · 2026-01-20T10:18:09Z

/retest

tiprow · 2026-01-20T10:18:35Z

@GMHDBJD: 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.

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.

GMHDBJD · 2026-01-20T11:44:42Z

/retest

tiprow · 2026-01-20T11:45:08Z

@GMHDBJD: 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.

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 bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 8, 2026

ti-chi-bot bot removed do-not-merge/needs-linked-issue do-not-merge/needs-triage-completed labels Jan 8, 2026

ti-chi-bot 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 Jan 9, 2026

joechenrh approved these changes Jan 9, 2026

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 9, 2026

D3Hunter reviewed Jan 9, 2026

View reviewed changes

lightning/pkg/importinto/checkpoint.go Show resolved Hide resolved

lightning/pkg/importinto/importer.go Outdated Show resolved Hide resolved

lightning/pkg/importinto/job_monitor_test.go Outdated Show resolved Hide resolved

ti-chi-bot bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 9, 2026

GMHDBJD force-pushed the fixImportInto branch from a0a82e5 to 336ffa4 Compare January 13, 2026 13:45

GMHDBJD force-pushed the fixImportInto branch from 336ffa4 to de16341 Compare January 14, 2026 09:16

refactor(importinto): streamline job cancellation process and enhance…

7220e57

… error handling

D3Hunter reviewed Jan 15, 2026

View reviewed changes

joechenrh reviewed Jan 15, 2026

View reviewed changes

lightning/pkg/importinto/job_orchestrator.go Show resolved Hide resolved

lightning/pkg/importinto/job_monitor.go Outdated Show resolved Hide resolved

joechenrh requested a review from Copilot January 15, 2026 03:25

Copilot started reviewing on behalf of joechenrh January 15, 2026 03:25 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

lightning/pkg/importinto/job_orchestrator.go Outdated Show resolved Hide resolved

GMHDBJD added 4 commits January 19, 2026 11:59

Merge remote-tracking branch 'upstream/master' into fixImportInto

a6059e7

lightning/importinto: return error from processJobStatuses

ebf8695

lightning/importinto: log checkpoint update error on cancel

02caefe

lightning/importinto: drop phaseFromStep fallback

a7b31fe

Copilot AI reviewed Jan 19, 2026

View reviewed changes

lightning/importinto: use mathutil.Clamp

ca5412e

Replace local clamp01 helper with mathutil.Clamp.

D3Hunter approved these changes Jan 19, 2026

View reviewed changes

ti-chi-bot bot added lgtm approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 19, 2026

fix bzel

433dd1a

Copilot AI review requested due to automatic review settings January 19, 2026 10:02

Copilot started reviewing on behalf of GMHDBJD January 19, 2026 10:03 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Benjamin2037 approved these changes Jan 20, 2026

View reviewed changes

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

ti-chi-bot bot merged commit 6dd3a58 into pingcap:master Jan 20, 2026
37 of 38 checks passed

	table_name VARCHAR(256) NOT NULL,
	table_name VARCHAR(512) NOT NULL,

Conversation

GMHDBJD commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Jan 8, 2026

Uh oh!

codecov bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GMHDBJD commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

D3Hunter Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

GMHDBJD Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

GMHDBJD Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

D3Hunter Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jan 19, 2026

[LGTM Timeline notifier]

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

GMHDBJD commented Jan 19, 2026

Uh oh!

tiprow bot commented Jan 19, 2026

Uh oh!

Benjamin2037 left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jan 20, 2026

Uh oh!

Benjamin2037 commented Jan 20, 2026

Uh oh!

GMHDBJD commented Jan 20, 2026

Uh oh!

GMHDBJD commented Jan 20, 2026

Uh oh!

GMHDBJD commented Jan 8, 2026 •

edited

Loading

codecov bot commented Jan 8, 2026 •

edited

Loading