Skip to content

Conversation

@Leavrth
Copy link
Contributor

@Leavrth Leavrth commented Nov 18, 2025

What problem does this PR solve?

Issue Number: close #64514

Problem Summary:
the store id of region leader scanned may be 0.

What changed and how does it work?

add region scan checker to check the leader store id after each region scan API.

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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com>
@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-tests-checked do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-tests-checked labels Nov 18, 2025
@tiprow
Copy link

tiprow bot commented Nov 18, 2025

Hi @Leavrth. 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
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 92.30769% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.7180%. Comparing base (4abaac6) to head (3dcbd40).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #64530        +/-   ##
================================================
+ Coverage   77.8359%   79.7180%   +1.8820%     
================================================
  Files          1971       1895        -76     
  Lines        540836     529087     -11749     
================================================
+ Hits         420965     421778       +813     
+ Misses       118211     105852     -12359     
+ Partials       1660       1457       -203     
Flag Coverage Δ
integration 47.9240% <58.9743%> (-0.2755%) ⬇️
unit 76.8564% <92.3076%> (+0.3995%) ⬆️

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

Components Coverage Δ
dumpling 56.7974% <ø> (ø)
parser ∅ <ø> (∅)
br 66.4936% <92.3076%> (+5.3766%) ⬆️
🚀 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.

@lance6716
Copy link
Contributor

please help push PD side to accept this request tikv/pd#8442

@lance6716 lance6716 requested a review from okJiang November 18, 2025 09:34
Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com>
@okJiang
Copy link
Member

okJiang commented Nov 21, 2025

please help push PD side to accept this request tikv/pd#8442

Got it. I will try

}

func checkRegionConsistency(startKey, endKey []byte, regions []*RegionInfo) error {
func checkRegionConsistency(startKey, endKey []byte, regions []*RegionInfo, limitted bool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment for limited?

lastRegions = regions

if err = checkRegionConsistency(startKey, endKey, regions); err != nil {
if err = checkRegionConsistency(startKey, endKey, regions, false); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving this to here and not call checkRegionConsistency?

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need also check the consistency between the adjacent batches, not only the check here. Maybe this duplicate check doesn't consume too much resource?

Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com>
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 18, 2025
@ti-chi-bot
Copy link

ti-chi-bot bot commented Jan 5, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, okJiang

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:

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 ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 5, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Jan 5, 2026

[LGTM Timeline notifier]

Timeline:

  • 2025-12-18 03:45:13.648015885 +0000 UTC m=+1703858.461793457: ☑️ agreed by okJiang.
  • 2026-01-05 10:00:34.408256435 +0000 UTC m=+609990.226564857: ☑️ agreed by 3pointer.

@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 6, 2026

/ok-to-test

@ti-chi-bot ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 6, 2026
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 7, 2026

/retest

3 similar comments
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 7, 2026

/retest

@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 7, 2026

/retest

@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 7, 2026

/retest

@hawkingrei
Copy link
Member

/retest

5 similar comments
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 12, 2026

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com>
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 13, 2026

/retest

1 similar comment
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 13, 2026

/retest

@hawkingrei
Copy link
Member

/retest

6 similar comments
@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@YangKeao
Copy link
Member

/retest

@ti-chi-bot ti-chi-bot bot merged commit fa72152 into pingcap:master Jan 13, 2026
33 checks passed
@Leavrth
Copy link
Contributor Author

Leavrth commented Jan 26, 2026

/cherry-pick release-nextgen-20251011

@ti-chi-bot
Copy link
Member

@Leavrth: new pull request created to branch release-nextgen-20251011: #65796.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BR restore failed with error "type:UNKNOWN message:\"invalid store ID 0, not found\" when simulated pd network partition or pd rolling restart

7 participants