Skip to content

syncer(dm): debounce repeated unhandled-event warnings#12579

Merged
ti-chi-bot[bot] merged 6 commits intopingcap:masterfrom
GMHDBJD:dm-debounce-unhandled-events-12499
Apr 2, 2026
Merged

syncer(dm): debounce repeated unhandled-event warnings#12579
ti-chi-bot[bot] merged 6 commits intopingcap:masterfrom
GMHDBJD:dm-debounce-unhandled-events-12499

Conversation

@GMHDBJD
Copy link
Copy Markdown
Contributor

@GMHDBJD GMHDBJD commented Apr 1, 2026

What problem does this PR solve?

Issue Number: close #12499

What is changed and how it works?

This change reduces DM syncer log noise from repeated unhandled binlog events.

  • use TiDB's sampled logger for the DM syncer unhandled-event warnings
  • keep the existing warning messages, but sample repeated logs by message instead of logging every event immediately
  • keep the event type in the emitted warning fields
  • add a unit test covering the sampling behavior

With the current settings, the sampler allows the first matching warning through once every 5 minutes for the same log message.

Check List

Tests

  • Unit test

Questions

Will it cause performance regression or break compatibility?

No material compatibility change. This replaces repeated per-event warning logs with sampled warning logs, which reduces log volume in noisy cases.

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

No.

Release note

Reduce repeated DM syncer "unhandled event" warnings by using sampled logging.

@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. area/dm Issues or PRs related to DM. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 1, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to aggregate and periodically log unhandled binlog events, replacing immediate warning logs to prevent log flooding. It adds a background cron job that flushes accumulated event counts every five minutes. A review comment suggests optimizing the recordUnhandledEvent function by moving the reflection-based type string generation outside of the mutex lock to reduce contention during the binlog processing path.

Comment thread dm/syncer/syncer.go Outdated
@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

Comment thread dm/syncer/syncer.go Outdated
s.runWg.Add(1)
go s.updateTSOffsetCronJob(s.runCtx.Ctx)
s.runWg.Add(1)
go s.logUnhandledEventsCronJob(s.runCtx.Ctx)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we use SampleLogger inside tidb, instead of a routine to aggregate

@ti-chi-bot ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 2, 2026
@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Apr 2, 2026
@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Apr 2, 2026

@joechenrh: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

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.

@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

3 similar comments
@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

@GMHDBJD
Copy link
Copy Markdown
Contributor Author

GMHDBJD commented Apr 2, 2026

/retest

Copy link
Copy Markdown
Contributor

@Benjamin2037 Benjamin2037 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Apr 2, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, D3Hunter, 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:

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 Apr 2, 2026
@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Apr 2, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-04-02 03:17:47.452004959 +0000 UTC m=+407872.657365017: ☑️ agreed by D3Hunter.
  • 2026-04-02 10:42:11.378502305 +0000 UTC m=+434536.583862362: ☑️ agreed by Benjamin2037.

@ti-chi-bot ti-chi-bot bot merged commit a00349b into pingcap:master Apr 2, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved area/dm Issues or PRs related to DM. 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.

DM: Reduce "unhandled event" log level

4 participants