MDEV-39788 CHANGE MASTER savefiles read and write one line to many#5148
MDEV-39788 CHANGE MASTER savefiles read and write one line to many#5148ParadoxV5 wants to merge 10 commits into
Conversation
MDEV-39788 found that the recent refactor on the `main` (now 12.3) branch forgot that the line count includes the line count line itself. This test checks future changes in any supported version that they don’t make this mistake again.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new regression test to verify the line count behavior of replication info files (master.info and relay-log.info). The reviewer provided feedback to improve test reliability and prevent flakiness, suggesting to shut down the server before modifying the replication info files to avoid Windows file locking and shutdown overwrite issues. Additionally, the reviewer recommended using start_mysqld.inc instead of restart_mysqld.inc to restart the server, and cleaning up the replication state using RESET SLAVE ALL; at the end of the test.
|
|
||
| # `using_gtid` is the key-value section, not the 33rd option. | ||
| --remove_file $MYSQLD_DATADIR/master.info |
There was a problem hiding this comment.
Modifying master.info and relay-log.info while the server is running can cause test flakiness and failures:
- Windows File Locking: On Windows, the running
mysqldprocess may hold active file locks on these files, causing--remove_fileor--write_fileto fail with permission errors. - Shutdown Overwrite: When the server is restarted via
restart_mysqld.inc, it first performs a shutdown. During shutdown,mysqldmay flush its active replication state and overwrite the custommaster.infoandrelay-log.infofiles you just wrote.
To avoid these issues, shut down the server before modifying the files, and then start it back up.
--source include/shutdown_mysqld.inc
# `using_gtid` is the key-value section, not the 33rd option.
--remove_file $MYSQLD_DATADIR/master.info
| --let $restart_parameters= --skip-slave-start | ||
| --source include/restart_mysqld.inc |
There was a problem hiding this comment.
Since the server should be shut down before modifying the info files, use start_mysqld.inc here instead of restart_mysqld.inc to bring the server back up with the --skip-slave-start parameter.
--let $start_parameters= --skip-slave-start
--source include/start_mysqld.inc
The MDEV-37530 refactor missed the detail that the line counts in `@@master_info_file` and `@@relay_log_info_file` also counted the line-count line itself. Consequently, although fresh 12.3 installations and downgrades to earlier versions are unaffected, upgrades from pre-12.3 will encounter surprises when the 12.3 upgrade tries to read and discard the extra line: * `master_use_gtid` is reset to DEFAULT. * `@@relay_log_info_file` fails to load unless its data has previously shrunk. Since downgrading remained compatible, this commit fixes this content difference by correcting the code to read and write one fewer (unused) line. Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
… update the result
|
Oh right, still needa cover the directory separator difference on WinDoze… |
1 participant
The MDEV-37530 refactor missed the detail that the line counts in
@@master_info_fileand@@relay_log_info_filealso counted the line-count line itself.Consequently, although fresh 12.3 installations and downgrades to earlier versions are unaffected, upgrades from pre-12.3 will encounter surprises when the 12.3 upgrade tries to read and discard the extra line:
master_use_gtidis reset to DEFAULT.@@relay_log_info_filefails to load unless its data has previously shrunk.(Explanation is in the bug description)
Since downgrading remained compatible, this commit fixes this content difference by correcting the code to read and write one fewer (unused) line.
(Ooh, how will our spambot treat merge commits? Well, to humans: it is temporary.)