Skip to content

eventservice: scope mounter by changefeed - #6337

Open
asddongmen wants to merge 1 commit into
pingcap:masterfrom
asddongmen:fix-changefeed-mounter
Open

asddongmen wants to merge 1 commit into
pingcap:masterfrom
asddongmen:fix-changefeed-mounter

Conversation

@asddongmen

@asddongmen asddongmen commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #6334

When multiple changefeeds share an upstream cluster, EventService reused the mounter configured by the first registered changefeed. Integrity checks for later changefeeds therefore depended on registration order.

What is changed and how it works?

Move the mounter from the cluster-scoped event broker to changefeed status. It is initialized from that changefeed integrity configuration and used by all of its dispatcher scans. A unit test verifies different changefeeds receive distinct mounters.

Check List

Tests

  • Unit test

Questions

Will it cause performance regression or break compatibility?

No. Broker sharing, dispatcher protocol, and integrity configuration format are unchanged.

Do you need to update user documentation, design documentation or monitoring documentation?

No.

Release note

Fix TiCDC integrity checks using the correct changefeed configuration when changefeeds share an upstream cluster.

Summary by CodeRabbit

  • Bug Fixes
    • Changefeeds now use their own event-processing configuration, improving isolation when multiple changefeeds have different integrity settings.
    • Replicas of the same changefeed continue to share the appropriate changefeed state.

@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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 22, 2026
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 2ea3eec9-c647-4d18-a9f6-606908d9931f

📥 Commits

Reviewing files that changed from the base of the PR and between 6a71504 and 7a2cb0c.

📒 Files selected for processing (4)
  • pkg/eventservice/dispatcher_stat.go
  • pkg/eventservice/event_broker.go
  • pkg/eventservice/event_broker_test.go
  • pkg/eventservice/event_service.go

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


📝 Walkthrough

Walkthrough

The event service now creates a separate event mounter for each changefeed. Scanning uses the changefeed-scoped mounter. Tests verify distinct mounters for different changefeeds and shared status for replicas.

Changes

Integrity mounter scope

Layer / File(s) Summary
Create changefeed-scoped mounters
pkg/eventservice/dispatcher_stat.go, pkg/eventservice/event_broker.go, pkg/eventservice/event_service.go
changefeedStatus stores a mounter. The broker stores its timezone and creates each mounter from the changefeed integrity configuration.
Use and validate scoped mounters
pkg/eventservice/event_broker.go, pkg/eventservice/event_broker_test.go
Scanning uses the changefeed status mounter. Tests verify separate mounters across changefeeds and status reuse for replicas.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: lidezhu

Merge Risk: ⚪ Minimal · up to 7a2cb

Integrity checks now use each changefeed’s own configuration, avoiding cross-changefeed configuration reuse. The change is mergeable.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: scoping the event mounter by changefeed.
Description check ✅ Passed The description includes the issue reference, problem statement, implementation details, unit test coverage, compatibility assessment, documentation assessment, and release note.
Linked Issues check ✅ Passed Issue #6334 requires each changefeed to use its own integrity configuration. The PR stores a pevent.Mounter in changefeedStatus, creates it with event.NewMounter(c.tz, info.GetIntegrity()), and …
Out of Scope Changes check ✅ Passed The changed production files update mounter ownership, broker construction, dispatcher scan selection, and changefeed registration. The test changes update the related helper and verify the required b…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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 hops through changefeed light
Each mounter keeps its settings right
No first feed leads the way
Replicas share their status stay
Integrity blooms in scopes today

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 Sep 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Sep 23, 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 added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Sep 23, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-09-23 05:21:07.90355597 +0000 UTC m=+168593.128777087: ☑️ agreed by 3AceShowHand.
  • 2026-09-23 11:14:59.33294113 +0000 UTC m=+189824.558162267: ☑️ agreed by lidezhu.

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

Labels

approved do-not-merge/needs-triage-completed lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eventservice: integrity behavior depends on the first registered changefeed

3 participants