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.

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-27037 mysqlbinlog emits a warning when reaching EOF before stop-condition #3400

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

DaveGosselin-MariaDB
Copy link
Member

@DaveGosselin-MariaDB DaveGosselin-MariaDB commented Jul 15, 2024

Emit warnings if finished processing input files before reaching the boundary conditions indicated by either --stop-position or --stop-datetime.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@DaveGosselin-MariaDB DaveGosselin-MariaDB force-pushed the 10.5-mdev-27037-stop-pos-warning branch 2 times, most recently from dfccd95 to 269b3d7 Compare July 15, 2024 16:47
Copy link
Contributor

@bnestere bnestere left a comment

Choose a reason for hiding this comment

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

Hi @DaveGosselin-MariaDB!

Thanks for the PR! Please see my notes.

client/mysqlbinlog.cc Outdated Show resolved Hide resolved
client/mysqlbinlog.cc Outdated Show resolved Hide resolved
mysql-test/suite/rpl/t/rpl_warn_statement.test Outdated Show resolved Hide resolved
mysql-test/suite/rpl/t/rpl_warn.inc Outdated Show resolved Hide resolved
mysql-test/suite/rpl/t/rpl_warn.inc Outdated Show resolved Hide resolved
Copy link
Contributor

@bnestere bnestere left a comment

Choose a reason for hiding this comment

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

Thanks @DaveGosselin-MariaDB for a quick update, the patch is coming along nicely 😄. I left a few more notes.

client/mysqlbinlog.cc Outdated Show resolved Hide resolved
client/mysqlbinlog.cc Show resolved Hide resolved
client/mysqlbinlog.cc Show resolved Hide resolved
client/mysqlbinlog.cc Outdated Show resolved Hide resolved
@DaveGosselin-MariaDB
Copy link
Member Author

I need to investigate why some of the builders failed on my new test for the second commit while others did not.

@DaveGosselin-MariaDB DaveGosselin-MariaDB force-pushed the 10.5-mdev-27037-stop-pos-warning branch 3 times, most recently from 266a32d to aa1207f Compare July 24, 2024 13:42
Copy link
Contributor

@bnestere bnestere left a comment

Choose a reason for hiding this comment

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

Looks good, thank you @DaveGosselin-MariaDB ! I just had one last minor thought for cleanup.


--source include/have_binlog_format_statement.inc

--let assert_file= $MYSQLTEST_VARDIR/tmp/warn_pos_test_file.out
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the name assert_file makes it seem like the file itself has content that will be validated, esp hinting towards assert_grep.inc. But it is actually ignored. I think the name should set the expectation that the file is ignored.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I made that change and added --echo commands that reflect what's --exec'd (to have parity with the ...warn_stop_position.test file).

…condition

Emit a warning in the event that we finished processing input files
before reaching the boundary indicated by --stop-position.
…datetime

Emit a warning in the event that we finished processing input files
before reaching the boundary indicated by --stop-datetime.
@DaveGosselin-MariaDB DaveGosselin-MariaDB merged commit 9588526 into 10.5 Sep 12, 2024
11 of 12 checks passed
@DaveGosselin-MariaDB DaveGosselin-MariaDB deleted the 10.5-mdev-27037-stop-pos-warning branch September 12, 2024 12:43
ParadoxV5 added a commit that referenced this pull request Oct 29, 2024
…e stop-condition

* `binlog_mysqlbinlog_warn_stop_position`:
  Name the temporary output redirect as ignored to match
  `binlog_mysqlbinlog_warn_stop_datetime`
  * #3400 (comment)
* `binlog_mysqlbinlog_warn_stop_datetime`:
  Remove duplicate case
ParadoxV5 added a commit that referenced this pull request Oct 29, 2024
This commit adds warnings for `--stop-position` GTIDs that
were not reached at EOF, mainly as a follow-up to
MDEV-27037 Mysqlbinlog should output a warning if EOF is found before its stop condition
(#3400).

`--stop-position` warnings inform possible mistakes in the input,
especially for progress reporting of scripts/wrappers.
MDEV-34614 enchances MDEV-27037 with individualized GTID validation, for
GTID range selection weren’t in all versions that MDEV-27037 targeted.

Unlike #3400, `Binlog_gtid_state_validator` provides the code behind
this feature rather than `mysqlbinlog` itself.
Motivations behind this decision are:
* For a patch that’ll apply on older versions,
  this choice yields a smaller change footprint.
* `Binlog_gtid_state_validator` is already prompting unreached
  *`--start-position`s* for `--gtid-strict-mode`/`-vvv`.
ParadoxV5 added a commit that referenced this pull request Oct 29, 2024
This commit adds warnings for `--stop-position` GTIDs that
were not reached at EOF, mainly as a follow-up to
MDEV-27037 Mysqlbinlog should output a warning if EOF is found before its stop condition
(#3400).

`--stop-position` warnings inform possible mistakes in the input,
especially for progress reporting of scripts/wrappers.
MDEV-34614 enhances MDEV-27037 with individualized GTID validation, for
GTID range selection weren’t in all versions that MDEV-27037 targeted.

Unlike #3400, `Binlog_gtid_state_validator` provides the code behind
this feature rather than `mysqlbinlog` itself.
Motivations behind this decision are:
* For a patch that’ll apply on older versions,
  this choice yields a smaller change footprint.
* `Binlog_gtid_state_validator` is already prompting unreached
  *`--start-position`s* for `--gtid-strict-mode`/`-vvv`.
ParadoxV5 added a commit that referenced this pull request Oct 31, 2024
This commit adds warnings for `--stop-position` GTIDs
that were not reached at EOF, mainly as a follow-up to
MDEV-27037 Mysqlbinlog should output a warning if EOF is found before its stop condition
(#3400).

`--stop-position` warnings inform possible mistakes in the input,
especially for progress reporting of scripts/wrappers.
MDEV-34614 enhances MDEV-27037 with individualized GTID validation, for
GTID range selection weren’t in all versions that MDEV-27037 targeted.

Unlike #3400, `Binlog_gtid_state_validator` provides the code behind
this feature rather than `mysqlbinlog` itself.
Motivations behind this decision are:
* For a patch that’ll apply on older versions,
  this choice yields a smaller change footprint.
* `Binlog_gtid_state_validator` is already prompting unreached
  *`--start-position`s* for `--gtid-strict-mode`/`-vvv`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants