MDEV-38907 Optimistic Relay Log Crash Recovery#4803
Conversation
This commit adds an automatic recovery procedure to GTID relay log initialization. This procedure scans the write end of a log to remove any trailing (or corrupted) incomplete event or event group. Unlike `@@relay_log_recovery`, this procedure won’t discard the portion of logs that managed to survive the crash. For exceptional cases that require manual intervention, `@@relay_log_recovery` is not altered. Note, because START SLAVE purges the relay log in GTID mode, this feature doesn’t bring immediate value. However, this commit is a crash-safety preparation for lifting that limitation. Non-GTID mode can also build upon this procedure to improve its crash safety precautions. Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
ParadoxV5
left a comment
ParadoxV5
left a comment
There was a problem hiding this comment.
I wonder, what was the intent with (non-Engine-based) binary logs to leave them crash unsafe without pessimistic configs, in contrast to now that we are improving the crash safety of the relay log with automatic truncation?
| } | ||
|
|
||
|
|
||
| bool do_relaylog_recovery(MYSQL_BIN_LOG *relay_log, const char *log_name) |
There was a problem hiding this comment.
TC_LOG_BINLOG::recover() has @@gtid_binlog_state restoration and XA recovery entangled, not to mention missed optimizations.
Thus, I decided to roll a separate function to guide me in figuring out how the two recovery procedures could merge code.
| @@ -11685,8 +11879,9 @@ MYSQL_BIN_LOG::close_engine() | |||
| */ | |||
| void MYSQL_BIN_LOG::clear_inuse_flag_when_closing(File file) | |||
There was a problem hiding this comment.
Would it be beneficial for the relay log to make use of the in-use flag, and only run the recovery procedure if the flag is not cleared?
Problems include:
- One of the next steps in the MDEV-4698 project is to restore
Gtid_IO_Pos(MDEV-38909, which will always require relay log scanning similar to this recovery procedure (unless there’s a.statefile in good condition). - Relay logs upgraded from before MDEV-4698 to after will miss this auto recovery.
(Even if not crashed and corrupted, they may contain incomplete groups that the project task MDEV-38906 wants to eliminate. - To check: the SQL thread might have special handling for Format Descriptions that have the in-use flag set.
| and not safe for non-GTID mode until MDEV-39051 | ||
| */ | ||
| !is_relay_log_recovery && mi->master_use_gtid)) |
There was a problem hiding this comment.
I’m deferring the additional work required for non-GTID replication (MDEV-39051) for now.
Let me know if it’s better together, especially since this procedure is extraneous in GTID replication until we lift the START SLAVE limitation.
File-pos replication can reconnect mid-group without complex skip-enqueue logic, so it actually doesn’t need to check event groups and only needs to truncate a corrupted event.
Though I’m concerned that this’ll be yet another difference that hurts interoperability between GTID and legacy modes (if that future is possible).
| /* | ||
| A slave will verify the checksum in the SQL thread; | ||
| the relay log does not care about checksums until then. | ||
| */ | ||
| event= Log_event::read_log_event(&log, &err, &fdev, false, false) |
There was a problem hiding this comment.
TODO: fix comment: SQL thread checksum verification can be disabled.
Regardless, should this procedure verify checksums as if it’s the IO thread?
| fdev.reset_crypto(); | ||
| } | ||
|
|
||
| if (truncate_entry && (err= relay_log->truncate_and_remove_binlogs( |
There was a problem hiding this comment.
It is easier said than done to skip truncation when truncating at EOF.
It might be more beneficial deeper in the call chain… but then, I wonder if this optimization will become negligible…
| if (rli->relay_log.open(ln, 0, 0, SEQ_READ_APPEND, | ||
| (ulong)(rli->max_relay_log_size ? rli->max_relay_log_size : | ||
| max_binlog_size), 1, TRUE)) | ||
| if (rli->relay_log.open(ln, nullptr, 0, SEQ_READ_APPEND, |
There was a problem hiding this comment.
🧐 Is this section unreachable, or merely repeated code?
| @@ -4643,29 +4838,28 @@ bool MYSQL_BIN_LOG::open(const char *log_name, | |||
| ulong max_size_arg, | |||
| bool null_created_arg, | |||
| bool need_mutex, | |||
There was a problem hiding this comment.
Need what mutex?
do_binlog_recovery() calls functions with need_lock passed with 1, so what does the code documentation mean by “you must have a locks on LOCK_log and LOCK_index”?
Some issues might be existing bugs, where relay log open errors leads to further problems similar to MDEV-24625.
1 participant
This commit adds an automatic recovery procedure to GTID relay log initialization.
This procedure scans the write end of a log to remove any trailing (or corrupted) incomplete event or event group.
Unlike
@@relay_log_recovery, this procedure won’t discard the portion of logs that managed to survive the crash.For exceptional cases that require manual intervention,
@@relay_log_recoveryis not altered.Note, because START SLAVE purges the relay log in GTID mode, this feature doesn’t bring immediate value.
However, this commit is a crash-safety preparation for lifting that limitation.
Non-GTID mode can also build upon this procedure to improve its crash safety precautions.