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.
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
...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.
- Removed duplicate words, like "the the" and "to to"
- Removed duplicate lines (one double sort line found in mysql.cc)
- Fixed some typos found while searching for duplicate words.
Command used to find duplicate words:
egrep -rI "\s([a-zA-Z]+)\s+\1\s" | grep -v param
Thanks to Artjoms Rimdjonoks for the command and pointing out the
spelling errors.
We had a protection against it, by allowing versioned delete if:
trx->id != table->vers_start_id()
For replace this check fails: replace calls ha_delete_row(record[2]), but
table->vers_start_id() returns the value from record[0], which is irrelevant.
The same problem hits Field::is_max, which may have checked the wrong record.
Fix:
* Refactor Field::is_max to optionally accept a pointer as an argument.
* Refactor vers_start_id and vers_end_id to always accept a pointer to the
record. there is a difference with is_max is that is_max accepts the pointer to
the
field data, rather than to the record.
Method val_int() would be too effortful to refactor to accept the argument, so
instead the value in record is fetched directly, like it is done in
Field_longlong.
See also MDEV-30046.
Idempotent write_row works same as REPLACE: if there is a duplicating
record in the table, then it will be deleted and re-inserted, with the
same update optimization.
The code in Rows:log_event::write_row was basically copy-pasted from
write_record.
What's done:
REPLACE operation was unified across replication and sql. It is now
representred as a Write_record class, that holds the whole state, and allows
re-using some resources in between the row writes.
Replace, IODKU and single insert implementations are split across different
methods, reluting in a much cleaner code.
The entry point is preserved as a single Write_record::write_record() call.
The implementation to call is chosen on the constructor stage.
This allowed several optimizations to be done:
1. The table key list is not iterated for every row. We find last unique key in
the order of checking once and preserve it across the rows. See last_uniq_key().
2. ib_handler::referenced_by_foreign_key acquires a global lock. This call was
done per row as well. Not all the table config that allows optimized replace is
folded into a single boolean field can_optimize. All the fields to check are
even stored in a single register on a 64-bit platform.
3. DUP_REPLACE and DUP_UPDATE cases now have one less level of indirection
4. modified_non_trans_tables is checked and set only when it's really needed.
5. Obsolete bitmap manipulations are removed.
Also:
* Unify replace initialization step across implementations:
add prepare_for_replace and finalize_replace
* alloca is removed in favor of mem_root allocation. This memory is reused
across the rows.
* An rpl-related callback is added to the replace branch, meaning that an extra
check is made per row replace even for the common case. It can be avoided with
templates if considered a problem.
in case of a long unique conflict ha_write_row() used delete_row()
to remove the newly inserted row, and it used rnd_pos() to position
the cursor before deletion. This rnd_pos() was freeing and reallocating
blobs in record[0]. So when the code for FOR PORTION OF did
store_record(record[2]);
ha_write_row()
restore_record(record[2]);
it ended up with blob pointers to a freed memory.
Let's use lookup_handler for deletion.
let's disallow UPDATE IGNORE in READ COMMITTED with the table
has UNIQUE constraint that is USING HASH or is WITHOUT OVERLAPS
This rarely-used combination should not block a release, with be
fixed in MDEV-37233
followup for 9703c90712 (MDEV-37199 UNIQUE KEY USING HASH accepting duplicate records)
ha_partition can return HA_ERR_KEY_NOT_FOUND even in the middle
of the index scan
followup for 9703c90712 (MDEV-37199 UNIQUE KEY USING HASH accepting duplicate records)
maintain the invariant, that handler::ha_update_row()
is always invoked as handler::ha_update_row(record[0], record[1])
followup for 9703c90712 (MDEV-37199 UNIQUE KEY USING HASH accepting duplicate records)
when looking for long unique duplicates and the new row is already
inserted, we cannot simply "skip one conflict" we must skip exactly
the new row and find a conflict which isn't a new row - otherwise
table->file->dup_ref can be set incorrectly and REPLACE won't work.
Server-level UNIQUE constraints (namely, WITHOUT OVERLAPS and USING HASH)
only worked with InnoDB in REPEATABLE READ isolation mode, when the
constraint was checked first and then the row was inserted or updated.
Gap locks prevented race conditions when a concurrent connection
could've also checked the constraint and inserted/updated a row
at the same time.
In READ COMMITTED there are no gap locks. To avoid race conditions,
we now check the constraint *after* the row operation. This is
enabled by the HA_CHECK_UNIQUE_AFTER_WRITE table flag that InnoDB
sets in the READ COMMITTED transactions.
Checking the constraint after the row operation is more complex.
First, the constraint will see the current (inserted/updated) row,
and needs to skip it. Second, IGNORE operations become tricky,
as we need to revert the insert/update and continue statement execution.
write_row() (INSERT IGNORE) is reverted with delete_row(). Conveniently
it deletes the current row, that is, the last inserted row.
update_row(a,b) (UPDATE IGNORE) is reverted with a reversed update,
update_row(b,a). Conveniently, it updates the current row too.
Except in InnoDB when the PK is updated - in this case InnoDB internally
performs delete+insert, but does not move the cursor, so the "current"
row is the deleted one and the reverse update doesn't work.
This combination now throws an "unsupported" error and will
be fixed in MDEV-37233
Lots of different cases, SELECT, SELECT DEFAULT(),
UPDATE t SET x=DEFAULT, prepares statements,
opening of a table for the I_S, prelocking (so TL_WRITE),
insert with subquery (so SQLCOM_SELECT), etc.
Don't check NEXTVAL privileges in fix_fields() anymore, it cannot
possibly handle all the cases correctly. Make a special method
Item_func_nextval::check_access() for that and invoke it from
* fix_fields on explicit SELECT NEXTVAL()
(but not if NEXTVAL() is used in a DEFAULT clause)
* when DEFAULT bareword in used in, say, UPDATE t SET x=DEFAULT
(but not if DEFAULT() itself is used in a DEFAULT clause)
* in CREATE TABLE
* in ALTER TABLE ALGORITHM=INPLACE (that doesn't go CREATE TABLE path)
* on INSERT
helpers
* Virtual_column_info::check_access() to walk the item tree and invoke
Item::check_access()
* TABLE::check_sequence_privileges() to iterate default expressions
and invoke Virtual_column_info::check_access()
also, single-table UPDATE in prepared statements now associates
value items with fields just as multi-update already did, fixes the
case of PREPARE s "UPDATE t SET x=?"; EXECUTE s USING DEFAULT.
handler::clone() call did not work with read only tables like S3.
It gave a wrong error message (out of memory instead of a permission
error) and aborted the query.
The issue was that the clone call had a wrong parameter to ha_open().
This now fixed. I also changed the clone call to provide the correct
error message if things fails.
This patch fixes an 'out of memory' error when using the S3 engine
for queries that could use multiple indexes together to find the matching
rows, like the following:
SELECT * FROM t1 WHERE key1 = 99 OR key2 = 2
This commit fixes a bug where Aria tables are used in
(master->slave1->slave2) and a backup is taken on slave2. In this case
it is possible that the replication position in the backup, stored in
mysql.gtid_slave_pos, will be wrong. This will lead to replication
errors if one is trying to use the backup as a new slave.
Analyze:
Replicated row events are committed with trans_commit_stmt() and
thd->transaction->all.ha_list != 0.
This means that backup_commit_lock is not taken for Aria tables,
which means the rows are committed and binary logged on the slave
under BLOCK_COMMIT which should not happen.
This issue does not occur on the master as thd->transaction->all.ha_list
is == 0 under AUTO_COMMIT, which sets 'is_real_trans' and 'rw_trans'
which in turn causes backup_commit_lock to be taken.
Fixed by checking in ha_check_and_coalesce_trx_read_only() if all handlers
supports rollback and if not, then wait for BLOCK_COMMIT also for
statement commit.
To check the rows, the table needs to be opened. To that end, and like
MDEV-36038, we force COPY algorithm on ALTER TABLE ... SEQUENCE=1.
This also results in checking the sequence state / metadata.
The table structure was already validated before this patch.
