Skip to content

api: reject changefeed creation during upstream import or restore - #6525

Merged
ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
wk989898:import-fix
Oct 14, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
wk989898:import-fix

Conversation

@wk989898

@wk989898 wk989898 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

This is a pick from pingcap/tiflow#8544

What problem does this PR solve?

Issue Number: close #1616

What is changed and how it works?

Reject changefeed creation when the upstream has running Lightning physical import, BR restore, or IMPORT INTO tasks.

Before setting up the GC safepoint, query task registrations under /tidb/brie/import in the upstream PD's embedded etcd and check their lease TTLs. Active leases block creation, expired leases are ignored, and errors querying registrations or leases abort creation. Both API v1 and v2 use this check.

Check List

Tests

  • Unit test
  • Integration test

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Fix an issue where a changefeed could be created while Lightning physical import, BR restore, or IMPORT INTO tasks were running on the upstream cluster, potentially causing incomplete replication.

Signed-off-by: wk989898 <nhsmwk@gmail.com>
@ti-chi-bot ti-chi-bot Bot added needs-cherry-pick-release-nextgen-202609 Should cherry pick this PR to release-nextgen-202609 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 13, 2026
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31b2328c-9257-4401-bb3b-60748e2c03c8
📥 Commits

Reviewing files that changed from the base of the PR and between 90a0a9a and 3ecef0f.

📒 Files selected for processing (5)
  • api/v2/changefeed.go
  • api/v2/changefeed_test.go
  • api/v2/helper.go
  • api/v2/helper_test.go
  • pkg/errors/error.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Changefeed creation now checks upstream etcd for leased import tasks before building changefeed metadata. An active task causes an upstream-running-import error. Lookup and lease-check failures return PD etcd API errors. Tests cover Lightning, restore, and import-into tasks.

Changes: Import-task guard

Layer / File(s) Summary
Active import-task detection
pkg/errors/error.go, api/v2/helper.go, api/v2/helper_test.go
A new error identifies an upstream with an active import task. The helper scans matching etcd keys and checks their lease TTLs. Tests cover active, expired, and zero TTLs, empty results, and etcd errors.
Changefeed creation gate
api/v2/changefeed.go, api/v2/changefeed_test.go
CreateChangefeed checks for active import tasks before building metadata. Tests verify that Lightning, restore, and import-into tasks cause an HTTP 500 response with the expected error details.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: 3aceshowhand

Merge Risk: ⚪ Minimal · up to 3ecef

Changefeed creation is now rejected while an upstream import or restore holds an active lease, as intended. No concrete merge-blocking risk was identified.

Security architecture risk: 🔵 Low · up to 3ecef

The change adds a read-only safeguard using existing upstream connections and preserves authentication and creation ordering. No introduced security vulnerability was established. Remaining uncertainty concerns deployed read permissions and whether concurrent import registration is coordinated with changefeed creation.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The gate is upstream-wide, without keyspace filtering. A positive lease beneath the fixed prefix can block new changefeed creation across keyspaces served by that upstream. This requires upstream registration-write authority; the API request itself cannot choose another endpoint, prefix or lease ID.

Trust Boundaries and Controls

  • observed — Both API versions retain authentication middleware before creation. When client-user authentication is enabled, failure aborts the request before the gate. When disabled, the existing policy allows access; this PR does not change that policy. The new upstream reads use server-owned connections rather than caller-selected authority.

Resilience and Maintainability Implications

  • inferred — The safeguard checks one prefix snapshot followed by individual TTL queries; it does not serialize import initiation with GC setup or metadata persistence. A later registration is therefore not excluded by this consumer alone. Producer coordination remains unverified, and the overlap exposure predates the PR because the base performed no import check.

Hardening Proposals

  • proposed — Before rollout, validate the minimum registration-read and lease-query permissions with the deployed identity. Resolve permission failures without granting broader upstream write authority. Treat the check as a snapshot safeguard unless the producer protocol establishes coordinated exclusion.
Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed The PR meets #1616. api/v2/changefeed.go calls hasRunningImport before changefeed setup continues. api/v2/helper.go scans /tidb/brie/import, blocks leases with TTL greater than zero, ignores e…
Out of Scope Changes check Passed The changes stay within the stated scope of #1616 and the current PR description. The helper, exported error, API integration, and tests all support rejection during upstream import or restore tasks. …
Title check Passed The title clearly summarizes the main change: rejecting changefeed creation during upstream import or restore tasks.
Description check Passed The description includes the required issue reference, problem statement, implementation details, tests, and release note. The compatibility and documentation questions are present but unanswered; the…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the lease at night
And keeps new changefeeds out of sight
When import tasks are still in play
The error shows the task at bay
Once leases end, the path is clear

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

@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Oct 13, 2026
@ti-chi-bot ti-chi-bot Bot added the lgtm label Oct 14, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3AceShowHand, lidezhu

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 [3AceShowHand,lidezhu]

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 Oct 14, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-10-13 04:57:15.649513926 +0000 UTC m=+707045.396744785: ☑️ agreed by lidezhu.
  • 2026-10-14 07:41:38.935981909 +0000 UTC m=+803308.683212748: ☑️ agreed by 3AceShowHand.

@wk989898

Copy link
Copy Markdown
Collaborator Author

/retest

@ti-chi-bot
ti-chi-bot Bot merged commit be38e50 into pingcap:master Oct 14, 2026
40 checks passed
@ti-chi-bot

Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-nextgen-202609: #6543.

@ti-chi-bot

Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #6629.
But this PR has conflicts, please resolve them!

wk989898 added a commit to ti-chi-bot/ticdc-1 that referenced this pull request Oct 21, 2026
Resolve the pingcap#6525 cherry-pick against release-8.5 while keeping
the branch-specific implementation and the original fix behavior.

Validated with focused race and failpoint unit tests, make fmt,
and make cdc using Go 1.25.12.
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. needs-cherry-pick-release-nextgen-202609 Should cherry pick this PR to release-nextgen-202609 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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.

when upstream have lightning or br task, changefeed can be created successfully

4 participants