Bug: tp_foreach() found an engine in hton2plugin[] and passed the
result of plugin_lock() to plugin_hton() without a check. While
reap_plugins() deinitializes an uninstalled engine (PLUGIN_IS_DYING),
the slot stays set until the end of ha_finalize_handlerton(), and
plugin_lock() returns NULL. The server crashed in plugin_hton(), for
example in RESET MASTER through ha_commit_checkpoint_request().
This is a regression from aed5928207, which changed plugin_foreach()
to tp_foreach() and lost the PLUGIN_IS_READY state mask.
Fix: add plugin_lock_ready(), which locks only a PLUGIN_IS_READY plugin
and reports under LOCK_plugin whether a failed lock was for a READY
plugin (out of memory in debug builds). tp_foreach() skips a plugin that
is not READY and returns an error for a READY plugin that cannot be
locked. An uninstalled but busy engine (PLUGIN_IS_DELETED) is not
visited, as with plugin_foreach() before.
The test uses a DEBUG_SYNC point in ha_finalize_handlerton() to run
RESET MASTER while an engine is deinitialized.
Assisted-by: https://mariadb.org/governance/governance-ai-policy/
Sequence access conflicts with streaming replication could cause the
server to hang, as shown in MDEV-38869.
A sequence table is written from SEQUENCE::next_value() while
SEQUENCE::mutex is held. For a streaming transaction the row write in
handler::ha_write_row() would then replicate a fragment and block waiting
for certification and commit order, while an applier may be waiting for
the same mutex in SEQUENCE::set_value(). Neither side can proceed, the
node deadlocks and the BF abort of the local transaction can never be
delivered.
This commit avoids the deadlock by skipping the streaming replication
step for sequence table rows. The row is already in the write set and is
replicated with the following fragment, or at commit.
Only that one step is skipped. The skip is passed down as a parameter to
wsrep_after_row() and wsrep_after_row_internal() rather than by not
calling them at all, so the row is still counted against
wsrep_max_ws_rows and wsrep_check_pk() still runs. A transaction using
sequences heavily therefore cannot silently exceed the configured write
set row limit.
The commit has also a new mtr test for three sequence/SR conflict
scenarios: galera.galera_sequences_bf_kill_sr
- a streaming transaction and an applier competing for SEQUENCE::mutex,
where both are expected to commit
- the same, but with the applier also BF aborting the local transaction
over a gap lock. A streaming transaction cannot be replayed, so it is
rolled back and the client gets ER_LOCK_DEADLOCK. The applier is held
at the abort_trx_end sync point until the abort has been issued, so
that the local transaction cannot finish its fragment first
- twelve row inserts on both nodes with wsrep_trx_fragment_unit=rows, so
that each node reserves several sequence cache ranges and the sequence
table writes land inside fragments carrying several rows. The values
the two nodes hand out must not overlap
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>
Just like when creating a table, the name canonicalisation happens in
ha_create_table.
This still allows table name canonicalisation to still take place
normally (e.g. in a DROP TABLE statement), but also fixes call the
handlerton method to drop partitions from DDL recovery, as for files
corresponding to partitions, in the format of
<table_name>#P#<partition_name>, consistently only table names are
canonicalised. For example in ha_partition::drop_partitions, first the
table name is canonicalised, before the #P#<partition_name> part is
appended, and the full name is passed to file->delete_table.
Strengthen some previous failing testcases to show that partition
names don't get lowered.
Problem:
========
When creating tables with vector indexes, if the secondary table
creation fails after the main table is successfully created,
the server was not properly cleaning up the main table's .ibd file.
Stale file exists because create_table_impl() always called
ddl_log_complete() on any error, which disables DDL log entries
instead of executing them for cleanup. The main table would be
left behind even though the overall CREATE TABLE operation failed.
Solution:
=========
ha_create_table(): Make it responsible for cleaning up
everything it created when it returns an error.
On failure of the high level index creation, drop the main
table and shadow table.
When ALTER TABLE uses the copy algorithm, ha_create_table() is
invoked to build the shadow table. A failure there was previously
reported as "Can't create table", which is misleading because the
user never issued a CREATE.
Fix (per review by @midenok): keep errno 1005 / SQLSTATE HY000
unchanged and only swap the message text when HA_CREATE_TMP_ALTER
is set, via a new translatable template ER_CANT_CREATE_TMP_ALTER_TABLE
in errmsg-utf8.txt (no SQLSTATE clause -- it is a message template
rather than a wire error code).
The user-visible error becomes:
ERROR 1005 (HY000): Can't create temporary table for ALTER TABLE \
`db`.`tbl` (errno: N "...")
No existing error handler, stored procedure, tool, or application
that matches on errno 1005 or SQLSTATE HY000 is affected.
Notes on the 12.3 rebase:
* In 12.3 the ER_CANT_CREATE_TABLE reporting logic has been moved
from ha_create_table() into its new helper ha_create_table_from_share().
The HA_CREATE_TMP_ALTER branch is added in the helper so that both
the main shadow table and any high-level (vector) index sub-tables
go through the same path. Vector index sub-tables use a zero-
initialised index_cinfo, so HA_CREATE_TMP_ALTER is never set for
them and they still get the plain "Can't create table" message.
Review follow-ups (Round 6, @midenok):
* Moved the test content from suite/innodb/t/alter_table_error_message.test
into main/alter_table.test (no duplicate --include directives);
dropped the extra .test/.result files accordingly.
* Added the standard "End of 12.3 tests" section marker at the end.
* Removed the unnecessary DROP TABLE IF EXISTS cluster_t; in the
cleanup step (the test is deterministic, the table no longer
exists at that point).
Review follow-ups (Round 7, test re-record + errmsg formatting):
* Fixed ER_CANT_CREATE_TMP_ALTER_TABLE template to use the exact
ER_CANT_CREATE_TABLE format specifiers, so error frames produced
by my_error() are byte-identical to the existing error (only the
fixed prefix changes).
* Re-recorded innodb.innodb-index.result and innodb.foreign_key.result
which now observe the new "Can't create temporary table for ALTER
TABLE" message in ALTER TABLE error paths that used to print
"Can't create table".
* No code changes outside of the intentional message swap; all other
.result files remain untouched.
Review follow-ups (Round 8, additional test re-record from CI):
* Re-recorded innodb.table_flags.result for the MDEV-38079 scenario
(ALTER TABLE t1 PAGE_COMPRESSED=1, ALGORITHM=COPY under
INNODB_DEFAULT_ROW_FORMAT=REDUNDANT) -- this ALTER path now reports
the new "Can't create temporary table for ALTER TABLE" message.
Verified on all 10 page size / checksum combinations (4k/8k/16k/
32k/64k x strict_crc32/strict_full_crc32).
* Re-recorded parts.engine_defined_part_attributes.result for the
ALTER TABLE `t4` PARTITION BY RANGE(...) ENCRYPTED path. The four
remaining "Can't create table" occurrences in the same file refer
to pure CREATE TABLE failures and correctly retain the original
message.
* No code or test-case logic changes; only .result re-recordings.
Signed-off-by: a18792721831 <735809899@qq.com>
CLA: a18792721831 <735809899@qq.com>
Reason:
=======
Following the changes in MDEV-38734, the server no longer marks
all indexed virtual columns during an UPDATE operation.
Consequently, ha_innobase::update() populates the upd_t vector
with old_vrow but omits the actual data for these virtual columns.
Despite this omission, trx_undo_page_report_modify() continues to
write metadata for indexed virtual columns into the undo log. Because
the actual values are missing from the update vector, the undo log
entry is recorded without the historical data for these columns.
When the purge thread processes the undo log to reconstruct a
previous record state for MVCC, it identifies an indexed virtual
column but finds no associated data.
The purge thread incorrectly interprets this missing data as a NULL
value, rather than a "missing/unrecorded" value. The historical
record is reconstructed with an incorrect NULL for the virtual column.
This causes the purge thread to incorrectly identify and purge
records that are not actually delete-marked, leading to abort
of server.
Solution:
=========
ha_innobase::column_bitmaps_signal(): Revert the column-marking
logic to the state prior to commit a4e4a56720, ensuring all
indexed virtual columns are unconditionally marked during an UPDATE.
The previous "optimization" attempted to manually detect indexed
column changes before marking virtual columns. The manual check
for indexed column modifications is redundant. InnoDB already
provides the UPD_NODE_NO_ORD_CHANGE flag within row_upd_step().
This flag is being used in trx_undo_page_report_modify() and
trx_undo_read_undo_rec() should log or read virtual column values.
Refactored column_bitmaps_signal() to accept a mark_for_update
parameter that controls when indexed virtual columns are marked.
TABLE::mark_columns_needed_for_update() is the only place that needs
mark_for_update=true because only UPDATE operations need to mark
indexed virtual columns for InnoDB's undo logging mechanism.
INSERT operation is already handled by
TABLE::mark_virtual_columns_for_write(insert_fl=true).
Even the commit a4e4a56720 changes are
going to affect TABLE::mark_virtual_column_for_write(false) and
It is called during UPDATE operation, and that's when
column_bitmaps_signal() needs to mark indexed virtual columns.
Online DDL has separate code path which is handled by
row_log_mark_virtual_cols() for all DML operations
Problem:
=======
InnoDB DDL does ALTER TABLE PAGE_COMPRESSED=1 because:
1. check_if_supported_inplace_alter() reads srv_file_per_table
during precheck and does allow INSTANT operation.
2. User does change innodb_file_per_table later.
But prepare phase read srv_file_per_table again during execution.
3. If the global variable changed between these phases, causes
server abort for page_compressed tables.
Solution:
=========
- Add a file_per_table member to Alter_inplace_info to
capture the innodb_file_per_table value during the
precheck phase and use that consistent value throughout
the entire ALTER TABLE operation.
- Converted boolean flags (online, ignore, error_if_not_empty,
mdl_exclusive_after_prepare) from individual boolean
members to bitfields packed in a single byte
- Reduced Alter_inplace_info structure size by 24 bytes
remove copying of part_elem->option_struct to table->s->option_struct.
It meant that most of the time a handler had no access to its options
(that's why some handlers resorted to a complex process of guessing the
correct partition) and couldn't work concurrenty. Instead
* introduce handler::option_struct which will point to
table->s->option_struct or part_elem->option_struct as appropriate,
the handler can use it any time as needed. A handler should not use
table->s->option_struct unless it really knows what it's doing and has
verified that it doesn't break partitioning
* rename option_struct in table->s to option_struct_table and in
part_elem to option_struct_part to emphasize it's semantics and
prevent incorrect usage.
* fix engines accordingly.
Implement an improved binlog implementation that is integrated into
the storage engine. The new implementation is enabled with the
--binlog-storage-engine option. Initially the InnoDB storage engine
implements the binlog.
Integrating the binlog in the storage engine improves performance,
since it makes the InnoDB redo log the single source of truth and
avoids the need for expensive two-phase commit between binlog and
engine. It also makes it possible to disable durability (set
--innodb-flush-log-at-trx-commit=0) to further improve performance,
while still preserving the ability to recover the binlog and database
into a consistent state after a crash.
The new binlog implementation also greatly improves the internal
design and implementation of the binlog, and enables future
enhancements for replication.
This is a squash of the original 11.4-based patch series.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
...just like the when deleting the .frm file in the same function.
This improves consistency: internal duplicate deletion of .par file
and .frm file both fail silently.
Two transactions could binlog their completions in opposite to how it
is done in Engine. That is is rare situations ROLLBACK in Engine of
the dependency parent transaction could be scheduled by the
transaction before its binlogging. That give a follower dependency
child one get binlogged ahead of the parent.
For fixing this bug its necessary to ensure the binlogging phase is
always first one in the internal one-phase rollback protocol.
The commit combines
1. a code polishing piece over a part of MDEV-21117 that
made binlog handlerton always commit first in no-2pc cases and
2. the same rule now applies to the rollback.
An added test demonstrates how the child could otherwise reach binlog
before its parent.
Two transactions could binlog their completions in opposite to how it
is done in Engine. That is is rare situations ROLLBACK in Engine of
the dependency parent transaction could be scheduled by the
transaction before its binlogging. That give a follower dependency
child one get binlogged ahead of the parent.
For fixing this bug its necessary to ensure the binlogging phase is
always first one in the internal one-phase rollback protocol.
The commit makes sure the binlog handlerton always rollbacks as first
handlerton in no-2pc cases.
An added test demonstrates how the child could otherwise reach binlog
before its parent.
@@new_mode is a set of flags to control introduced features.
Flags are by default off. Setting a flag in @@new_mode will introduce
a new different server behaviour and/or set of features.
We also introduce a new option 'hidden_values' into some system variable
types to hide options that we do not wish to show as options.
- Don't print hidden values in mysqld --help output.
- Make get_options() use the same logic as check_new_mode_value() does.
- Setting @@new_mode=ALL shouldn't give warnings.
(cherry picked from commit f7387cb13d)
In the BASE of this patch when a slave parallel worker proceeds from
the wait-for-prior-commit stage into retrying it may have its
backup-lock related sub-state, specifically `THD::backup_commit_lock`,
not reset, that is the pointer dangling.
That caused segfault at the pointer's dereferencing in the worker retrying.
The reason THD::backup_commit_lock is left dangling was unexpected
state of THD having non-NULL of `THD::backup_commit_lock` and NULL of `mdl_backup.ticket`.
This combination turns out possible when the slave worker is killed
for retry *and* few instruction later it does not succeed
to (re-)acquire the Backup MDL at exiting from
`MYSQL_BIN_LOG::queue_for_group_commit()`.
While it did not succeed it also did not expose that fact with
timing out from `MDL_context::acquire_lock` and it did not have to,
as it before to start waiting it found itself killed.
The bug is fixed with amendment of `backup_commit_lock` reset condition
at the end of `ha_commit_trans()`. The amended reset remains careful to
affect only the stack allocated lock.
A test is added to confirm the fixes with reproducing all stages
described above. In the patch BASE it causes the segfault.
