MDEV-40236: Generalize --flashback error messages for non-flashback#5454
Open
ParadoxV5 wants to merge 2 commits into
Open
MDEV-40236: Generalize --flashback error messages for non-flashback#5454ParadoxV5 wants to merge 2 commits into
--flashback error messages for non-flashback#5454ParadoxV5 wants to merge 2 commits into
Conversation
Handle corrupted row events by reporting whether the field length could not be determined from metadata or whether the field extends past the row buffer instead of falling out through debug-only assertion paths or ad hoc exits during flashback row conversion. Propagate flashback conversion failures through the normal mysqlbinlog error path so debug builds produce the intended diagnostics cleanly. Add replication tests covering corrupted BLOB metadata and corrupted BLOB length prefixes using server-side debug injection to write broken binlog contents and mysqltest-friendly mysqlbinlog invocation patterns. Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com> Reviewed-by: Georgi Kodinov <joro@mariadb.org>
This comment was marked as spam.
This comment was marked as spam.
MDEV-20749 moved its improved error reporting from the flashback code path down to a common function that non-flashback mode also uses. This brings a side effect that MDEV-20749 also added error reporting to `mariadb-binlog` non-flashback mode for this particular code path, whose errors (if anticipated) were **completely silent** previously. These error messages assume that `mariadb-binlog` is in flashback mode, though; this commit removes this description from these messages so they make sense in both flashback and non-flashback modes. It keeps these messages at this location to maintain the detail level and cross-mode sharing (albeit only bug MDEV-40511 is known to trigger one of these descriptive errors outside of flashback).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR is based on #4767.
It targets
mainwith #4767 included because targeting #4767 would make a PR in the contributor’s fork.MDEV-20749 moved its improved error reporting from the flashback code path down to a common function that non-flashback mode also uses.
This brings a side effect that MDEV-20749 also added error reporting to
mariadb-binlognon-flashback mode for this particular code path, whose errors (if anticipated) were completely silent previously.These error messages assume that
mariadb-binlogis in flashback mode, though; this commit removes this description from these messages so they make sense in both flashback and non-flashback modes.It keeps these messages at this location to maintain the detail level and cross-mode sharing (albeit only bug MDEV-40511 is known to trigger one of these descriptive errors outside of flashback).
I have not included any tests (yet) because I find that whether
mariadb-binlogreaches these errors with improved reporting depends on the implementation details of the duplicated code paths.It’d be an entirely new feature, complete with reviews and QA, to turn this happy accident into a formally specified requirement.
Specifically,
I suspect it is because
mariadb-binlogcaught these faults and silently aborted at an earlier point.Although we can generalize its error to be an unrecognized data type, what is the guarantee that unrecognized types should trigger one of these errors?
Not to mention unrecognized types becoming recognized.