Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDEV-35694: Mysqlbinlog --stop-position should warn if EOF not reached with --read-from-remote-server #3757

Open
wants to merge 2 commits into
base: 10.5
Choose a base branch
from
Open

MDEV-35694: Mysqlbinlog --stop-position should warn if EOF not reached with --read-from-remote-server #3757

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aabb764
MDEV-35694: Regression only
bnestere Jan 13, 2025
76013ff
MDEV-35694: Mysqlbinlog --stop-position should warn if EOF not reache…
bnestere Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions client/mysqlbinlog.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ static uint verbose= 0;

static ulonglong start_position, stop_position;
static const longlong stop_position_default= (longlong)(~(my_off_t)0);
static ulonglong last_processed_position= 0;
Comment on lines 144 to +146
Copy link
Contributor

Choose a reason for hiding this comment

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

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”.

#define start_position_mot ((my_off_t)start_position)
#define stop_position_mot ((my_off_t)stop_position)

Expand Down Expand Up @@ -997,6 +998,8 @@ static bool print_row_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
determining how to print.
@param[in] ev Log_event to process.
@param[in] pos Offset from beginning of binlog file.
@param[in] ev_end_pos Offset that represents the end of the event in the
binlog file.
@param[in] logname Name of input binlog.

@retval ERROR_STOP An error occurred - the program should terminate.
Expand All @@ -1005,7 +1008,8 @@ static bool print_row_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
events to process has been reached and the program should terminate.
*/
Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
my_off_t pos, const char *logname)
my_off_t pos, my_off_t ev_end_pos,
const char *logname)
{
char ll_buff[21];
Log_event_type ev_type= ev->get_type_code();
Expand Down Expand Up @@ -1462,6 +1466,7 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
end:
rec_count++;
last_processed_datetime= ev_when;
last_processed_position= ev_end_pos;

DBUG_PRINT("info", ("end event processing"));
/*
Expand Down Expand Up @@ -2379,7 +2384,8 @@ static Exit_status handle_event_text_mode(PRINT_EVENT_INFO *print_event_info,
if (old_off != BIN_LOG_HEADER_SIZE)
*len= 1; // fake event, don't increment old_off
}
Exit_status retval= process_event(print_event_info, ev, old_off, logname);
Exit_status retval= process_event(print_event_info, ev, old_off,
old_off + (*len - 1), logname);
if (retval != OK_CONTINUE)
DBUG_RETURN(retval);
}
Expand All @@ -2397,7 +2403,8 @@ static Exit_status handle_event_text_mode(PRINT_EVENT_INFO *print_event_info,
DBUG_RETURN(ERROR_STOP);
}

retval= process_event(print_event_info, ev, old_off, logname);
retval= process_event(print_event_info, ev, old_off, old_off + (*len - 1),
logname);
if (retval != OK_CONTINUE)
{
my_close(file,MYF(MY_WME));
Expand Down Expand Up @@ -2794,9 +2801,9 @@ static Exit_status check_header(IO_CACHE* file,
the new one, so we should not do it ourselves in this
case.
*/
Exit_status retval= process_event(print_event_info,
new_description_event, tmp_pos,
logname);
Exit_status retval=
process_event(print_event_info, new_description_event, tmp_pos,
my_b_tell(file), logname);
if (retval != OK_CONTINUE)
return retval;
}
Expand Down Expand Up @@ -2944,22 +2951,10 @@ static Exit_status dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
}
// else file->error == 0 means EOF, that's OK, we break in this case

/*
Emit a warning in the event that we finished processing input
before reaching the boundary indicated by --stop-position.
*/
if (((longlong)stop_position != stop_position_default) &&
stop_position > my_b_tell(file))
{
retval = OK_STOP;
warning("Did not reach stop position %llu before "
"end of input", stop_position);
}

goto end;
}
if ((retval= process_event(print_event_info, ev, old_off, logname)) !=
OK_CONTINUE)
if ((retval= process_event(print_event_info, ev, old_off, my_b_tell(file),
logname)) != OK_CONTINUE)
goto end;
}

Expand Down Expand Up @@ -3138,6 +3133,15 @@ int main(int argc, char** argv)
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);
Comment on lines 3133 to +3143
Copy link
Contributor

@ParadoxV5 ParadoxV5 Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

☑️ 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.


/*
If enable flashback, need to print the events from the end to the
beginning
Expand Down