MDEV-20749: Improve error reporting of mysqlbinlog when used with --flashback#4767
MDEV-20749: Improve error reporting of mysqlbinlog when used with --flashback#4767monthdev wants to merge 1 commit intoMariaDB:mainfrom
Conversation
gkodinov
added
the
External Contribution
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
I am afraid I need to insist on a regression test. If you've debugged this anyhow you should have a working test case. Even if it's some manually generated binary sequence.
Last resort this could be short binlog file from MySQL.
Also, please squash your commits into a single commit and add a commit message to it that complies with CODING_STANDARDS.md
|
Hi! I got some help on Zulip and made some MTR tests that grep for the improved error message. |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
The tests are looking good. I was hoping for a more functional test: i.e. an actual sample of bad binlog. But simulating it is OK too. Please also consider addressing the other points in my review.
|
@gkodinov I could look into an actual bad binlog, but I don't know what could trigger this failure path. I have already tried the first suggestion in this thread #general > MDEV-20749, but it did not hit the failure path. I resorted to the second suggestion to use the And yes, I will come back with changes for richer debug output! |
|
@gkodinov I tried to check off most of what was asked for regarding the debug output. Let me know if this is closer to how you wanted it! |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
LGTM after squashing all commits into a single one with a compliant commit message.
Please stand by for the final review.
Print row decode failures directly in calc_row_event_length() so the diagnostic can include decoder-local context such as image kind, offsets, and field/type metadata. Pass event context from the flashback caller so the message can report the row event type and end_log_pos. Add MTR coverage for truncated row image and bad field length failures using DBUG fault injection in mariadb-binlog.
|
My understanding of the squashed commit is we need the matching issue title and paragraph description underneath. Please let me know if I did it wrong! |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your efforts. Looks perfect now.
3 participants
Address MDEV-20749
Improves the flashback row-decode error message by distinguishing truncated row images from bad field-length decoding.
This patch is not accompanied by an automated regression test yet. If there is a minimal sample binlog or reduced reproduction method, I can add an MTR regression test.