codec(ticdc): improve error logging for Debezium encoding failures#12484
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
do-not-merge/release-note-label-needed
ti-chi-bot
bot
added
do-not-merge/work-in-progress
|
Hi @takaidohigasi. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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
bot
added
the
needs-ok-to-test
|
Welcome @takaidohigasi! |
Summary of ChangesHello @takaidohigasi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the diagnostic capabilities of the Debezium encoder by enriching error logs with more contextual information. The changes ensure that when encoding failures occur, such as with invalid enum values, the logs provide specific details about the schema, table, column, and value involved, as well as the changefeed and commit timestamp for broader event-level errors. This enhancement aims to streamline the debugging process for encoding-related issues. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
|
ti-chi-bot
bot
added
the
size/S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves error logging for Debezium encoding failures by adding valuable context to log messages. The changes in writeDebeziumFieldValues and runEncoder will greatly aid in debugging. The implementation is clear and effectively meets the PR's objectives. I have a couple of suggestions to make the logging even more clear and robust.
Add detailed context to error logs when Debezium encoding fails, including schema, table, column name, value, changefeed ID, and commitTs. This helps debugging encoding issues like invalid enum values. - Fix log message grammar for clarity - Truncate large values (>1024 bytes/chars) to avoid log flooding 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
takaidohigasi
force-pushed
the
improve-debezium-error-logging
branch
from
5edbb2b to
41ae67e
Compare
ti-chi-bot
bot
added
size/M
|
testing on my laptop |
|
/ok-to-test |
ti-chi-bot
bot
added
ok-to-test
|
/retest |
Add tests to verify the fix for issue pingcap#12474 where enum/set columns with DEFAULT values receive string type values instead of uint64. These tests demonstrate: 1. The improved error logging (PR pingcap#12484) shows schema, table, column, value 2. The fix (PR pingcap#12475) correctly handles string type enum/set values Test output with reverted code: [ERROR] ["failed to write Debezium field value"] [schema=test] [table=t_enum] [column=status] [value=active] [error="unexpected column value type string for enum column status"] Related: - Issue: pingcap#12474 - Fix: pingcap#12475 - Logging: pingcap#12484 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Revert enum/set handling to demonstrate the bug from issue pingcap#12474 where enum/set columns with DEFAULT values receive string type values instead of uint64, causing encoding failures. This branch contains: 1. Reverted codec.go that does NOT handle string type enum/set values 2. Unit tests that verify the improved error logging (PR pingcap#12484) Test output shows the improved error logging: [ERROR] ["failed to write Debezium field value"] [schema=test] [table=t_enum] [column=status] [value=active] [error="unexpected column value type string for enum column status"] Related: - Issue: pingcap#12474 - Fix: pingcap#12475 - Logging: pingcap#12484 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
|
playground with ticdc does not work on my laptop, so I added unit test to the different branch. |
|
/retest |
1 similar comment
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3AceShowHand, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/retest |
1 similar comment
|
/retest |
|
was merged |
? |
|
/retest |
1 similar comment
|
/retest |
|
all the test is faling for page 4 |
|
/retest |
|
this PR test succeeded ... |
|
/test pull-cdc-integration-kafka-test |
|
/retest |
|
finally test passed! |
|
/release-note-none |
ti-chi-bot
bot
added
release-note-none
|
/test pull-verify |
|
thanks so much for your help |
|
@takaidohigasi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
wk989898
added
the
needs-cherry-pick-release-8.5
|
In response to a cherrypick label: new pull request created to branch |
4 participants
What problem does this PR solve?
Issue Number: close #12485
Summary
writeDebeziumFieldValueserror logrunEncodererror logThis helps debugging encoding issues like invalid enum values where previously only the error message was logged without context about which row/column caused the failure.
The
commitTsin the log can be used to skip problematic events usingcdc cli changefeed resume --overwrite-checkpoint-ts.issue: #12485
PII Consideration
This change logs the column value (
zap.Any("value", col.Value)) when encoding fails. This may expose sensitive/PII data in logs. However:mysql.go:229,mysql.go:713) which already logs valuesUsers handling sensitive data should ensure appropriate log access controls are in place.
Check List
Tests
Unit Test: takaidohigasi#2
Questions
Will it cause performance regression or break compatibility?
I changed error log format a bit, but does not be much problem.
changes log is only for error, so it does not cause performance regression
Do you need to update user documentation, design documentation or monitoring documentation?
no
Test plan
go test ./pkg/sink/codec/debezium/...)Release note
🤖 Generated with Claude Code