MDEV-35694: Mysqlbinlog --stop-position should warn if EOF not reached with --read-from-remote-server #3757
Conversation
This file contains 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
MDEV-35694: Mysqlbinlog --stop-position should warn if EOF not reached with --read-from-remote-server MDEV-27037 added functionality to warn users that a specified stop-position or stop-datetime was never reached. It only worked for local files though. The patch in MDEV-35528 changed the implementation for stop-datetime to work to provide the warning also when using --read-from-remote-server. The PR for that MDEV (#3670) was limited to only the stop-datetime field. The regression shows as a lack of an error message when running binlog.binlog_mysqlbinlog_warn_stop_position
…d with --read-from-remote-server MDEV-27037 added functionality to warn users that a specified stop-position or stop-datetime was never reached. It only worked for local files though. The patch in MDEV-35528 changed the implementation for stop-datetime to work to provide the warning also when using --read-from-remote-server. The PR for that MDEV (#3670) was limited to only the stop-datetime field. This patch updates the --stop-position warning to also work with --read-from-remote-server. Reviewed By: ============ <TODO>
ParadoxV5
approved these changes
Jan 14, 2025
Comment on lines
144
to
+146
| static ulonglong start_position, stop_position; | ||
| static const longlong stop_position_default= (longlong)(~(my_off_t)0); | ||
| static ulonglong last_processed_position= 0; |
There was a problem hiding this comment.
The comparison down there looks so wrong having to cast (C-style or static_const).
Will an unsigned do? Or even a this-file-only macro-constant?
Suggested change
| static ulonglong start_position, stop_position; | |
| static const longlong stop_position_default= (longlong)(~(my_off_t)0); | |
| static ulonglong last_processed_position= 0; | |
| static ulonglong start_position, stop_position; | |
| #define stop_position_default ~0ULL | |
| static ulonglong last_processed_position= 0; |
or ULLONG_MAX or std::numeric_limits<unsigned long long>::max()…
Rather, specializing this value would put ~0 as an input out of support, though I don’t think it has a practical difference from “your file is bigger than the legal limit”.
Comment on lines
3133
to
+3143
| warning("Did not reach stop datetime '%s' before end of input", | ||
| stop_datetime_str); | ||
|
|
||
| /* | ||
| Emit a warning in the event that we finished processing input | ||
| before reaching the boundary indicated by --stop-position. | ||
| */ | ||
| if ((static_cast<longlong>(stop_position) != stop_position_default) && | ||
| stop_position > last_processed_position) | ||
| warning("Did not reach stop position %llu before end of input", | ||
| stop_position); |
There was a problem hiding this comment.
☑️ Things are consistent with --stop-datetime (MDEV-35528).
Funny. The code used to check/warn datetime after position, but now it’s before. 😀
No need to swap back though. Positions are “absolute” while datetime (which can go out-of-order) are “relative”, so it feels intuitive to have that last.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
MDEV-27037 added functionality to warn users that a specified
stop-position or stop-datetime was never reached. It only worked for
local files though. The patch in MDEV-35528 changed the
implementation for stop-datetime to work to provide the warning also
when using --read-from-remote-server. The PR for that MDEV (#3670)
was limited to only the stop-datetime field.
This patch updates the --stop-position warning to also work with
--read-from-remote-server.
This PR is organized as follows:
result when running binlog.binlog_mysqlbinlog_warn_stop_position