Several error-handling paths returned without releasing a resource
already acquired earlier in the function, or checked the wrong handle
entirely, risking use of an unopened handle.
Changes:
- SysTablespace::read_lsn_and_check_flags(): close the datafile handle
on header-validation failure.
- xb_process_datadir(): check the freshly opened `dir` handle instead
of the stale `dbdir`, fixing a handle leak and a possible use of an
unopened directory handle.
- wsrep.cc / xb_load_list_file(): close file handles before die(), and
null-check fopen() results in wsrep.cc.
- datadir_iter_new(): free datadir_path and destroy the mutex on the
os_file_opendir() failure path.
GenError's custom command must specify headers as OUTPUT,
otherwise ninja cannot deduce that mysqld.cc depends on errmsg-utf8.txt
As a bonus, BYPRODUCTS lists generated files for `ninja clean`
With log_bin=ON a transaction is committed via two-phase commit (the
binary log is the second participant), so it passes through the InnoDB
XA-prepare state. While a donor is held in BLOCK_COMMIT for a mariabackup
backup, its parallel appliers (wsrep_slave_threads > 1) leave one or more
such writesets prepared-but-not-yet-committed, and the snapshot captures
them. On a freshly SST'd joiner nothing resolves these prepared
transactions: binlog crash recovery does not run (the joiner has no in-use
binlog to recover from), and the wsrep continuity-based commit is inactive
because wsrep_emulate_bin_log is FALSE when log_bin is ON. The leftover
prepared transactions then abort startup with "Found <N> prepared
transactions!". Note this does not depend on the prepared set being
non-contiguous - even a contiguous run aborts, because nothing commits
or rolls it back.
Rollback these transactions in xarecover_handlerton(). If rollback fails
flag error to cause unireg_abort().
After a mariabackup SST the joiner could fail with
ER_GTID_STRICT_OUT_OF_ORDER (error 1950)
while re-binlogging transactions received over IST.
The cause is that the binary log copied from the donor carries a
Gtid_list whose position can be ahead of the storage-engine snapshot
because getting snapshot and binlog are no mutually atomic, so transactions
can be present in the copied binlog that are not committed in the copied
engine snapshot.
After the SST the joiner reports the (committed) engine position to the
cluster, IST resends those transactions, and re-binlogging them under
gtid_strict_mode=ON collides with the ahead Gtid_list -> error 1950.
(MDEV-34483 made the engine snapshot stop short of the binlog, which is
what exposed this.)
The copied binary log carries no transactions the joiner needs - only a
Gtid_list - so instead of shipping and then having to truncate/reconcile
it, the joiner now starts a fresh binary log and seeds its GTID position
from the storage-engine checkpoint during recovery. That checkpoint is
the committed cluster position, i.e. exactly where IST resumes, so the
joiner's binary log stays in lockstep with the rest of the cluster and
no out-of-order GTID can occur.
This works for both wsrep_gtid_mode settings; only the binlog domain of
the cluster stream differs:
- wsrep_gtid_mode=ON : wsrep_gtid_domain_id (cluster writes are
re-tagged to it), which is the domain stored in the checkpoint;
- wsrep_gtid_mode=OFF: gtid_domain_id (cluster writes keep the node's
configured domain).
Async-replica positions (mysql.gtid_slave_pos) are part of the engine
snapshot and survive the SST unchanged, so a Galera node can still serve
as an async master or replica across the SST.
This commit:
- sql/log.cc: adds wsrep_seed_binlog_gtid_state(), called from
do_binlog_recovery() when the joiner has no binary log, seeding the
binlog GTID state for the cluster domain to the SE checkpoint position.
- scripts/wsrep_sst_mariabackup.sh: no longer moves the donor's binary
log into place on the joiner.
- extra/mariabackup: backward compatibility: keep shipping binlog file
in SST but
- on donor fix the race between rotation and shipping so that the file
shipped is the one that had been rotated;
- on joiner discard shipped binlog in favour of one generated by
wsrep_seed_binlog_gtid_state().
- sql/wsrep_sst.cc: logs the position actually adopted from storage
(the authoritative post-SST position) rather than the script-reported
one.
- sql/handler.cc: downgrades the "Discovered discontinuity in recovered
wsrep transaction XIDs" message in wsrep_order_and_check_continuity()
from warning to debug level. With parallel appliers a snapshot
routinely captures prepared XIDs that are not contiguous with the
engine checkpoint, so this is normal during SST recovery and of no
value in regular operation; the transactions past the checkpoint are
re-delivered by the cluster (IST/SST) regardless.
- Adds an MDEV-38147 MTR test reproducing the issue.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Problem:
========
During backup process, log copier thread stalls at lsn and fails with
the misleading "Was only able to copy log from X to Y, not Z; try increasing
innodb_log_file_size", even when the redo was intact and the log far
from full.
Problem is that copier advances recv_sys.lsn only on CRC-validated mtr,
but it parses the redo at the server is concurrently writing to it.
InnoDB rewrites the current partial write block repeatedly as
mini-transaction fills it, so an mmap read (or) pread of a page
being written can observe a torn/intermediate image of the tail block.
If that tail block happens to parse as a longer mtr which is still valid,
has a correct CRC then recv_sys.lsn, recv_sys.offset gets advanced wrongly
and points to middle of the mini-transaction. In that case, later
parse return GOT_EOF always and copier thread never reaches target
and fails
Solution:
=========
Poll the server's durably-flushed LSN (status var Innodb_lsn_flushed) and
limit the redo log copier based on it.
backup_log_parse(): parses one mtr and if it exceeds the limit
rolls back recv_sys.lsn/offset and reports GOT_EOF; the copier
re-parses those bytes on a later pass once the fence has
advanced past them. Used on both the mmap and buffered paths.
log_copying_thread(): opens its own connection (it runs
concurrently with the main thread, which owns mysql_connection) and refreshes
the max_limit for parsing of redo log in each pass.
In the final backup phase (BACKUP STAGE BLOCK_COMMIT) it gates
on the exact target max(metadata_last_lsn, metadata_to_lsn).
so the last partial block is copied in full up to the target.
The stall diagnostic / retry spin is suppressed on a reached_parse_limit wait
so a normal "caught up, wait" is not misreported as drift.
get_log_flushed_lsn(): Added to get log_flushed_lsn from SHOW STATUS outout;
MSAN/ASAN test environment, the addr2line was so high in memory
utilization that it was the pick of the OOM killer to resolve the OOM
situation. Once this occurred there wasn't a saved core or gdb backtrace
of the core to resolve the issue.
To resolve this, make stack-trace default to 0 (off) for the addr2line
base implementation under memory sanitizer and address sanitizer.
MariaDB-backup also forces the enabling of stack-trace. Disabling this
unconditionally reduces the risk of a user operational impact if a
lengthy stack trace starting in a mariadb-backup critical locked period.
The mysqld--help test now excludes the stack-trace as its result is
environment dependant. The "Defaults to..." output for suppressed
variables, currently only stack-trace, is excluded.
Since thread-stack is an excluded varable, the ubsan/asan exclusions
from commits dfa6fba959 and dfa6fba959 aren't required.
invalid pointer pairs are when the length/memory of one string are
intermixed with another.
For comp_err, the end null pointer was compared to soffset within
my_strtoll10. As we didn't need the end position a NULL arg option
was compatble.
For uca-dump, Address Sanitizer raises invalid pointer pair because
argv options (opt) don't have an obvious correlation of having a start
at opt_X.length, even though the lstrncmp makes this true.
The DBUG_ASSERTS of strmov (added MDEV-11752) where incompatible with
pointer-compare. Replaced strmov with static inline version in
m_string.h using memmove that allows overlaps, and being inline allows
the uneeded parts of the implementation to be optimized away.
scripts/fill_help_tables.sql from
https://github.com/mariadb-corporation/mariadb-docs/ as of
3fb208fcdfed06beb52ef9e305017de2387ff012
Other URL changes are taken from the HTTP redirection URL
from the old KB.
Some test cases where impacted:
* ps_missed_cmds{,_bin_prot} - used a topic_id of 0 that is
now used, used MAX_INT instead.
* range_notembedded used the help data as a test case. As its not
a stable table data, replaces with static data. Test case valid
by ensuring the ANALYZE EXPLAIN and optimizer trace had the same
structure. Also valided the MDEV-24739/MDEV-25069 bug fixes from
where the test case originaled, when reverted, exhibited the
previous behavour before the bug fixes.
rewrite mariadb-backup "early" option parsing to use my_getopt
+ proper handling of values separated from the option by a space (not =)
+ case insensitive and -/_ insensitive comparison
- multiple --defaults-group don't work
- multiple --login-path don't work
- --incremental-dir overwrites --target-dir, not "whatever comes first"
Remove the unscoped #pragma GCC diagnostic ignored
"-Wdeprecated-declarations" from include/violite.h and replace all
sprintf/vsprintf calls with snprintf/vsnprintf across the codebase.
Where possible, pass actual buffer sizes as function parameters
instead of using hardcoded constants or recomputing strlen().
Notable API changes:
- get_date(): added to_len parameter for buffer size
- nice_time(), end_timer() in client/mysql.cc: added buff_size
- generate_new_name(): added name_size parameter
- calc_md5(): added buffer_size parameter (fixes off-by-one)
- MD5_HASH_TO_STRING macro: added _size parameter
- Added MYSQL_UDF_MAX_RESULT_LENGTH constant in mysql_com.h
Also fixed a pre-existing bug in sql/rpl_mi.cc where the ellipsis
"..." was written to the wrong buffer (dbuff instead of buff).
Vendored code is excluded: extra/readline, extra/wolfssl,
wsrep-lib, libmariadb, storage/mroonga/vendor, zlib.
Problem:
`mariabackup` ignores `my_close()` failures, resulting in false
success report with a 'completed OK!' message.
Fix:
Update `local_close()` function in `mariabackup` to check `my_close()`
failures and enusre errors are reported.
log_sys.latch: Remove the PERFORMANCE_SCHEMA instrumentation.
We already know that this is a very busy latch. All code paths
where a shared log_sys.latch is being held should already be
highly optimized. The few paths where an exclusive log_sys.latch
is being held are known to be potentially problematic, but rare.
Removing the PERFORMANCE_SCHEMA instrumentation for this latch
is expected to improve performance. On a quick Sysbench run with
performance_schema_instrument=wait/synch/rwlock/%=on
I observed an 2.9% improvement on throughput.
srw_lock_debug_simple: A non-instrumented version of srw_lock_debug,
to allow PERFORMANCE_SCHEMA to work WITH_INNODB_EXTRA_DEBUG=ON.
Reviewed by: Alessandro Vetere
Concurrent calls to log_checkpoint_low() were possible from multiple
threads, and they could cause redundant writes of FILE_CHECKPOINT
records that refer to the same checkpoint. Let us simplify the logic
by making the dedicated buf_flush_page_cleaner() thread responsible
for checkpoints.
log_t::write_checkpoint(lsn_t end_lsn): Add the parameter checkpoint,
which will replace the data member log_sys.next_checkpoint_lsn.
log_sys.checkpoint_pending: Remove. Only the buf_flush_page_cleaner()
thread will write checkpoints or initiate page writes.
log_checkpoint_low(), log_checkpoint(): Remove the return value,
because there cannot be any concurrent log checkpoint in progress.
buf_flush_wait(): Add a parameter for waiting for a full checkpoint.
This function replaces buf_flush_wait_flushed().
log_t::checkpoint_margin(): Replaces log_checkpoint_margin().
lot_t::write_buf(): Remove a call set_check_for_checkpoint(false)
that commit 7443ad1c8a (MDEV-32374)
had added. The flag should only be cleared when the checkpoint has
advanced far enough.
log_make_checkpoint(): Simply wrap buf_flush_sync_batch(0, true).
buf_flush_sync_batch(): Add the parameter bool checkpoint, to wait
for an empty FILE_CHECKPOINT record to be written. Outside recovery,
pass lsn=0.
buf_flush_sync_for_checkpoint(): On shutdown, update the systemd
watchdog and keep flushing until a final checkpoint has been written.
buf_flush_page_cleaner(): Revise the shutdown logic so that all
changes will be written out and a checkpoint with just a FILE_CHECKPOINT
record can be written.
buf_flush_buffer_pool(): Remove.
buf_flush_wait_flushed(): Require the caller to acquire
buf_pool.flush_list_mutex.
logs_empty_and_mark_files_at_shutdown(): Simplify the logic,
and return the shutdown LSN.
fil_names_clear(): Fix an off-by-one error that would prevent
removal from fil_system.named_spaces.
innodb_shutdown(): Always invoke logs_empty_and_mark_files_at_shutdown().
srv_undo_tablespaces_reinit(): Simplify the logic and remove the
fault injection point after_reinit_undo_abort, which would cause
a failure on Microsoft Windows. Changing innodb_undo_tablespaces is
not fully crash-safe.
Reviewed by: Thirunarayanan Balathandayuthapani
Tested by: Saahil Alam
Currently, posix_fadvise(..., 0, 0, POSIX_FADV_DONTNEED) is executed
repeatedly after each my_write. In case of pipes, it evaluates to
ESPIPE and fails. This results in millions of redundant syscalls,
introducing massive overhead.
As proposed by Daniel Black, call posix_fadvise with a length/size
of 0 once when the file is opened, rather than repetitively after
every write. This change moves the fadvise call to the respective
_open functions for ds_local, ds_stdout and ds_tmpfile datasinks.
It fails fast on pipes early on and eliminates the redundant
syscall overhead for subsequent write operations.
When a cluster donor node executes mariabackup SST, it will use same
approximation for max open files limit as was set for the mariadbd
process during the server startup. This may be a problem for installation
where mariabackup execution would need higher open file count, and might
crash for exceeding the too tight open file limit.
The reason for this behavior is that when mariadbd server calculates the
expected max open files count, it will record this open file count approximation
as system ulimit value, both as soft and hard limit. Later, when the node
operates as SST donor, it spawns mariabackup executable, which now inherits the
ulmit setting used for the mariadbd process. Mariabackup tries to enforce
open_files_limit variable value configured in [mariabackup] group in the my.cnf
fle, but this will fail if hard ulimit value is smaller.
The fix in this commit records the approximated max open file count only as soft
ulimit value. If hard ulimit is higher or unlimited, there remains head room for
the mariabackup to use higher open_files_limit configuration.
The InnoDB bootstrap is creating unnecessary log checkpoints
and even invalidating the buffer pool for no reason, making it
necessary to read pages into the buffer pool during bootstrap.
Furthermore, the page cleaner thread attempts to invoke
recv_sys.apply(true), even though most calls were unreachable
starting with commit 685d958e38 (MDEV-14425)
if not earlier.
buf_flush_sync(): Replaced with buf_flush_sync_batch(LSN_MAX),
possibly preceded by recv_sys.apply(true).
buf_pool_t::assert_all_freed(): Assert that buf_pool.mutex is
being held by the caller.
buf_dblwr_t::create(): Evict each doublewrite buffer pool page
immediately from the buffer pool, so that there will be no need
to invoke buf_pool_invalidate(). Also, remove the unnecessary
call to buf_flush_wait_flushed(); there is nothing special about
the doublewrite buffer creation ever since
commit e2c63a7062 (MDEV-38595)
made the redo logging actually crash-safe.
buf_pool_invalidate(): Moved to the same compilation unit with
the only remaining caller, recv_sys_t::apply().
srv_start(): Do not invoke buf_flush_sync(). We already created
an initial checkpoint in log_t::create(lsn_t), and we will create
the final one during shutdown. Together with the simplification of
buf_dblwr_t::create(), this should noticeably speed up the
server bootstrap. Also, simplify the log file rebuild by extending
the critical section of log_sys.latch and buf_pool.flush_list_mutex.
create_log_file(): Assert and assume that both log_sys.latch and
buf_pool.flush_list_mutex were acquired by the caller.
This prevents any race condition with buf_flush_page_cleaner()
when the log is being rebuilt.
srv_prepare_to_delete_redo_log_file(): Invoke recv_sys.apply(true)
before invoking buf_flush_sync_batch(LSN_MAX).
Remove a redundant call to fil_system.sys_space->open(false)
because srv_start() must already have successfully invoked that
in order for us to reach this code path. Remove some duplicated
assertions as well as a redundant call to log_write_up_to() from
the end. If we crash while rebuilding a latest_format log file,
the subsequent startup will perform crash recovery normally.
This avoids a redundant write to the old log.
The backup of encrypted Aria tables was not supported.
Added support for this. One complication is that the page checksum is
for the not encrypted page. To be able to verify the checksum I have to
temporarly decrypt the page.
In the backup we store the encrypted pages.
Other things:
- Fixed some (not critical) memory leaks in mariabackup
Problem:
========
When an inplace ALTER operation is rolled back, InnoDB drops
intermediate tables and their associated FTS internal tables.
However, MariaBackup's DDL tracking can incorrectly report
this as a backup failure.
The issue occurs because backup_set_alter_copy_lock() downgrades the
MDL_BACKUP_DDL lock before the inplace phase of ALTER,
allowing FTS internal tables to be dropped during the
later phases of backup when DDL tracking is still active.
Solution:
========
backup_file_op_fail(): Ignore delete operations on FTS internal
tables when not using --no-lock option, preventing false
positive backup failures.
multiple file tablespace
Problem:
=======
- innochecksum was incorrectly interpreting doublewrite buffer
pages as index pages, causing confusion about stale tables
in the system tablespace.
- innochecksum fails to parse the multi-file system tablespace
Solution:
========
1. Rewrite checksum of doublewrite buffer pages
are skipped.
2. Introduced the option --tablespace-flags which can be used
to initialize page size. This option can handle the ibdata2,
ibdata3 etc without parsing ibdata1.
This is a cherry-pick of commit 9f8716ab61
It was possible that a log checkpoint was completed and the server killed
between the time that fil_ibd_create() durably wrote a FILE_CREATE record,
and the initialization of the tablespace. This could lead to a failure
to start up after the server was killed during TRUNCATE TABLE or any
table-rebuilding operation such as OPTIMIZE TABLE.
In the case of TRUNCATE TABLE, an attempt to rename a file #sql-ibNNN.ibd
(the contents of the table before truncation) to tablename.ibd would fail,
because both files existed and the file tablename.ibd would have been
filled with NUL bytes. It was possible to resume from this error by
deleting the file tablename.ibd and restarting the server.
We will prevent this class of errors by ensuring that both the FILE_CREATE
record and the records written by fsp_header_init() will be part of the
same atomic transaction, which must be durably written before any file
is created or allocated.
NOTE: There is another possible crash recovery problem, which we are not
attempting to solve here and which will be covered by the subsequent
change (MDEV-38026). If fil_ibd_create() fails to create the file and
the server were killed, recovery would not even attempt to create the file
at all.
fil_space_t::create(): Remove the DBUG_EXECUTE_IF fault injection that
was the only cause of return nullptr. This allows us to simplify
several callers.
fil_space_t::set_stopped(), fil_space_t::clear_stopped(): Accessor
functions for fil_ibd_create() for preventing any concurrent access
to an incompletely created tablespace.
fil_ibd_create(): In a single atomic mini-transaction, write the
FILE_CREATE record as well as the log for initializing the tablespace.
After durably writing the log, create the file in the file system
Only after the file has been successfully created and allocated,
open the tablespace for business. Finally, release the exclusive page
latches so that the header pages may be written to the file.
fil_ibd_open(): Move some fault injection from fil_space_t::create()
to a higher level, to the place where an existing file is being opened.
Reviewed by: Thirunarayanan Balathandayuthapani
Tested by: Saahil Alam
