ingest: skip write when the region has no leader by D3Hunter · Pull Request #64442 · pingcap/tidb

D3Hunter · 2025-11-12T12:37:56Z

What problem does this PR solve?

Issue Number: close #64422, ref #61702

Problem Summary:

What changed and how does it work?

this issue only happens in nextgen, as classic region job worker already have an additional leader check after write to tikv, we fix this by check region leader before write, so for classic kernel we can avoid the useless write

tidb/pkg/lightning/backend/local/region_job.go

Line 632 in 87152f7

if len(leaderPeerMetas) == 0 {

also log the unretryable error, to help debug and direct us to add more retryable error

Check List

Tests

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

we run the same scenario in the issue, no such error again, and we can see errors like below when there is no leader for some scanned region

[2025/11/13 15:46:11.734 +08:00] [INFO] [local.go:1497] ["put job back to jobCh to retry later"] [keyspaceName=SYSTEM] [task-id=120016] [task-key=keyspace_a/ImportInto/120016] [subtaskID=270022] [step=ingest] [startKey=780000017480000000000000C55F7280000000003C8F88] [endKey=780000017480000000000000C55F7280000000003C9901] [stage=regionScanned] [retryCount=1] [waitUntil=2025/11/13 15:46:13.734 +08:00] [error="[KV:ErrNoLeader]region has no leader, region '326782'"]

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

ti-chi-bot · 2025-11-12T12:37:59Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2025-11-12T12:38:18Z

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-11-12T12:46:06Z

/cherry-pick release-nextgen-20251011

ti-chi-bot · 2025-11-12T12:46:08Z

@D3Hunter: once the present PR merges, I will cherry-pick it on top of release-nextgen-20251011 in the new PR and assign it to you.

Details

In response to this:

/cherry-pick release-nextgen-20251011

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 ti-community-infra/tichi repository.

codecov · 2025-11-13T04:28:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.6174%. Comparing base (d4c1408) to head (566becb).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #64442        +/-   ##
================================================
+ Coverage   72.6813%   73.6174%   +0.9361%     
================================================
  Files          1866       1889        +23     
  Lines        506188     514301      +8113     
================================================
+ Hits         367904     378615     +10711     
+ Misses       115892     112742      -3150     
- Partials      22392      22944       +552

Flag	Coverage Δ
integration	`44.4777% <78.2608%> (?)`
unit	`72.5267% <91.3043%> (+0.2729%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.8700% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`46.2438% <ø> (+0.0901%)`	⬆️

🚀 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-11-13T04:55:34Z

/retest

tiprow · 2025-11-13T04:56:00Z

@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-11-13T05:18:54Z

/hold

…rr-type

GMHDBJD

LGTM

ti-chi-bot · 2025-11-13T07:30:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GMHDBJD, 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~~ [GMHDBJD,joechenrh]
~~pkg/ingestor/OWNERS~~ [GMHDBJD]
~~pkg/lightning/OWNERS~~ [GMHDBJD]

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-11-13T07:30:27Z

[LGTM Timeline notifier]

Timeline:

2025-11-13 05:09:07.275011022 +0000 UTC m=+937996.718040891: ☑️ agreed by joechenrh.
2025-11-13 07:30:26.56697029 +0000 UTC m=+946476.010000159: ☑️ agreed by GMHDBJD.

lance6716 · 2025-11-13T07:59:32Z

+			// Also note, valid store ID > 0.
+			if job.region.Leader.GetStoreId() == 0 {
+				job.lastRetryableErr = errdef.ErrNoLeader.GenWithStackByArgs(job.region.Region.GetId())
+				job.convertStageTo(needRescan)


If the raft member state are not change and only PD losts the latest state, change to "regionScanned" is better

how to determine raft member state are not change and only PD losts the latest state?

If we optimistically & wrongly change to regionScanned, later in write RPC TiDB will see "epoch not match", "request too old" and it comes to needRescan. Luckily we don't need to scan region, but the tradeoff is we may have an extra round to process the job.

I don't know the background context of this issue. If only PD reboot, I think TiKV's raft epoch is not changed, regionScanned is better.

if we cannot determine based on the scanned region info, we have to write or ingest and let tikv return the error back to us

in the case of this PR, the above check is to check whether the scanned region have leader or not, if no, we have to rescan

D3Hunter · 2025-11-13T08:48:52Z

/retest

tiprow · 2025-11-13T08:49: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.

D3Hunter · 2025-11-13T08:56:25Z

/unhold

…rr-type

D3Hunter · 2025-11-13T10:56:12Z

/retest

tiprow · 2025-11-13T10:56:37Z

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

ref pingcap#61702, close pingcap#64422

ti-chi-bot · 2025-11-13T11:34:17Z

@D3Hunter: new pull request created to branch release-nextgen-20251011: #64470.

Details

In response to this:

/cherry-pick release-nextgen-20251011

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 ti-community-infra/tichi repository.

ref #61702, close #64422

D3Hunter added 3 commits November 12, 2025 14:42

change

430b8b0

change

bbd7636

change

87152f7

ti-chi-bot Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 12, 2025

change

63560ec

ti-chi-bot Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 12, 2025

err

722f7b6

D3Hunter changed the title ~~ingest: skip write when the region has no need~~ ingest: skip write when the region has no leader Nov 12, 2025

D3Hunter added 2 commits November 12, 2025 22:47

log err

111b337

change

1aa292b

D3Hunter marked this pull request as ready for review November 13, 2025 04:09

ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 13, 2025

joechenrh approved these changes Nov 13, 2025

View reviewed changes

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

fix test

d9d66dd

ti-chi-bot Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 13, 2025

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 Nov 13, 2025

revert back to check store id

3b8249c

joechenrh reviewed Nov 13, 2025

View reviewed changes

Comment thread pkg/lightning/backend/local/job_worker.go

D3Hunter added 2 commits November 13, 2025 14:55

Merge remote-tracking branch 'upstream/master' into log-unretryable-e…

2a5122e

…rr-type

fix failed case

db0fc29

GMHDBJD approved these changes Nov 13, 2025

View reviewed changes

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

lance6716 reviewed Nov 13, 2025

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 Nov 13, 2025

Merge remote-tracking branch 'upstream/master' into log-unretryable-e…

566becb

…rr-type

ti-chi-bot Bot merged commit 8e799f7 into pingcap:master Nov 13, 2025
28 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 13, 2025

ingest: skip write when the region has no leader (pingcap#64442)

20777b7

ref pingcap#61702, close pingcap#64422

ti-chi-bot mentioned this pull request Nov 13, 2025

ingest: skip write when the region has no leader (#64442) #64470

Merged

13 tasks

D3Hunter deleted the log-unretryable-err-type branch November 13, 2025 11:39

ti-chi-bot Bot pushed a commit that referenced this pull request Nov 14, 2025

ingest: skip write when the region has no leader (#64442) (#64470)

e62af4b

ref #61702, close #64422

This was referenced Nov 20, 2025

retry: avoid log un-retryable error when it's nil or context cancelled #64590

Merged

DXF service on nextgen kernel #61702

Open

Uh oh!

Conversation

D3Hunter commented Nov 12, 2025 • 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!

ti-chi-bot Bot commented Nov 12, 2025

Uh oh!

tiprow Bot commented Nov 12, 2025

Uh oh!

D3Hunter commented Nov 12, 2025

Uh oh!

ti-chi-bot commented Nov 12, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

D3Hunter commented Nov 13, 2025

Uh oh!

tiprow Bot commented Nov 13, 2025

Uh oh!

D3Hunter commented Nov 13, 2025

Uh oh!

Uh oh!

GMHDBJD left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Nov 13, 2025

Uh oh!

ti-chi-bot Bot commented Nov 13, 2025

[LGTM Timeline notifier]

Uh oh!

lance6716 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

D3Hunter Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

lance6716 Nov 13, 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!

lance6716 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

D3Hunter Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

lance6716 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented Nov 13, 2025

Uh oh!

tiprow Bot commented Nov 13, 2025

Uh oh!

D3Hunter commented Nov 13, 2025

Uh oh!

D3Hunter commented Nov 13, 2025

Uh oh!

tiprow Bot commented Nov 13, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

D3Hunter commented Nov 12, 2025 •

edited

Loading

codecov Bot commented Nov 13, 2025 •

edited

Loading

lance6716 Nov 13, 2025 •

edited

Loading