Skip to content

br: ensure schemaLease is the normal (45s) instead of test value (1s)#66092

Merged
ti-chi-bot[bot] merged 3 commits intopingcap:masterfrom
kennytm:hotfix-br-schema-lease-v8.5.2
Mar 6, 2026
Merged

br: ensure schemaLease is the normal (45s) instead of test value (1s)#66092
ti-chi-bot[bot] merged 3 commits intopingcap:masterfrom
kennytm:hotfix-br-schema-lease-v8.5.2

Conversation

@kennytm
Copy link
Contributor

@kennytm kennytm commented Feb 5, 2026

What problem does this PR solve?

Issue Number: close #66110

Problem Summary: Without calling session.SetSchemaLease, BR will use the default schema lease duration of 1s which is only intended for testing, not production. When the PD TSO somehow lags the BR wall clock by over 0.5s (1/2 of schemaLease) it will not be able to leave the schema reload loop.

What changed and how does it work?

Call session.SetSchemaLease with the default lease (45s) when the glue is initialized, which is invoked on start-up for standalone BR.

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.
  • The customer which encountered this bug can successfully perform br restore point after applying this fix.

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.

Fixed a bug in PiTR which caused it to be stuck at `Waiting for schema info finishes reloading` for 15 minutes and fail.

Summary by CodeRabbit

  • Chores
    • Improved the default schema lease applied at startup to use the production value earlier in initialization. This reduces intermittent stalls or delays when external time services lag, yielding more reliable database startup and more consistent readiness for users.

@ti-chi-bot

This comment was marked as outdated.

@ti-chi-bot 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. do-not-merge/cherry-pick-not-approved 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 Feb 5, 2026
@tiprow

This comment was marked as outdated.

@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 42defd6a-4fed-4121-a06b-5dfb4ecf125b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f243d2 and a5dab00.

📒 Files selected for processing (1)
  • br/pkg/gluetidb/BUILD.bazel

📝 Walkthrough

Walkthrough

This change sets the default schema lease early during BR's TiDB glue initialization by importing sessionctx/vardef, calling vardef.SetSchemaLease(config.DefSchemaLease) in New(), and adds //pkg/sessionctx/vardef to the gluetidb Bazel deps.

Changes

Cohort / File(s) Summary
Schema Lease Initialization
br/pkg/gluetidb/glue.go
Imported sessionctx/vardef and added vardef.SetSchemaLease(config.DefSchemaLease) in New() prior to updating global config to set the schema lease early.
Build Dependency
br/pkg/gluetidb/BUILD.bazel
Added //pkg/sessionctx/vardef to the deps of the //br/pkg/gluetidb:gluetidb go_library rule so the new import is available at build time.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 I hopped into glue at early light,

Set the lease so restores take flight,
No more stalls in schema queues,
A tiny nibble of code, and BR resumes,
Thump-thump — a rabbit's quiet delight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: ensuring schemaLease uses the normal 45s value instead of test 1s value.
Description check ✅ Passed The description includes issue number, problem summary explaining the bug, what changed, manual test confirmation, and a release note.
Linked Issues check ✅ Passed The PR directly addresses issue #66110 by implementing the fix to use the normal 45s schema lease instead of test 1s value during BR initialization.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the schema lease issue: adding vardef import and calling SetSchemaLease in glue initialization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-triage-completed release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-tests-checked labels Mar 6, 2026
@kennytm kennytm marked this pull request as ready for review March 6, 2026 07:09
@ti-chi-bot 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 Mar 6, 2026
@kennytm kennytm changed the title [DNM] br: ensure schemaLease is the normal (45s) instead of test value (1s) br: ensure schemaLease is the normal (45s) instead of test value (1s) Mar 6, 2026
coderabbitai[bot]

This comment was marked as resolved.

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.9225%. Comparing base (ef17075) to head (a5dab00).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #66092        +/-   ##
================================================
+ Coverage   77.6919%   78.9225%   +1.2306%     
================================================
  Files          2008       1946        -62     
  Lines        549590     539382     -10208     
================================================
- Hits         426987     425694      -1293     
+ Misses       120943     112653      -8290     
+ Partials       1660       1035       -625     
Flag Coverage Δ
integration 44.8840% <100.0000%> (-3.3053%) ⬇️
unit 76.7753% <100.0000%> (+0.4106%) ⬆️

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

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

coderabbitai[bot]

This comment was marked as resolved.

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 6, 2026
@ti-chi-bot ti-chi-bot bot added the approved label Mar 6, 2026
@ti-chi-bot ti-chi-bot bot added the lgtm label Mar 6, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Mar 6, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Leavrth, YuJuncen

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 removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 6, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Mar 6, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-03-06 09:16:02.127489461 +0000 UTC m=+1018.626546186: ☑️ agreed by Leavrth.
  • 2026-03-06 09:31:46.234789944 +0000 UTC m=+1962.733846696: ☑️ agreed by YuJuncen.

@YuJuncen
Copy link
Contributor

YuJuncen commented Mar 6, 2026

nogo: missing strict dependencies:

br/pkg/gluetidb/glue.go: import of "github.com/pingcap/tidb/pkg/sessionctx/vardef"

@kennytm
Copy link
Contributor Author

kennytm commented Mar 6, 2026

/test pull-integration-e2e-test
/test pull-mysql-client-test

@tiprow
Copy link

tiprow bot commented Mar 6, 2026

@kennytm: 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:

/test pull-integration-e2e-test
/test pull-mysql-client-test

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 ti-chi-bot bot merged commit 8506cf9 into pingcap:master Mar 6, 2026
30 checks passed
@Leavrth Leavrth added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Mar 7, 2026
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #66761.

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

Labels

approved lgtm needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

br restore point may stuck at ["waiting for schema info finishes reloading"] for 15 minutes

4 participants