in the $case=2 - it's wrong to kill after the first binlog EOF,
because that might happen between INSERT(4) and INSERT(5).
So, wait for the slave to acknowledge INSERT(5) before killing
the master, that is, both connection threads must pass
repl_semisync_master.wait_after_sync()
The root cause of the failure is a bug in the Linux network stack:
https://lore.kernel.org/netdev/87sf0ldk41.fsf@urd.knielsen-hq.org/T/#u
If the slave does a connect(2) at the exact same time that kill -9 of the
master process closes the listening socket, the FIN or RST packet is lost in
the kernel, and the slave ends up timing out waiting for the initial
communication from the server. This timeout defaults to
--slave-net-timeout=120, which causes include/master_gtid_wait.inc to time
out first and fail the test.
Work-around this problem by reducing the --slave-net-timeout for this test
case. If this problem turns up in other tests, we can consider reducing the
default value for all tests.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
If replicating an event in ROW format, and InnoDB detects a deadlock
while searching for a row, the row event will error and rollback in
InnoDB and indicate that the binlog cache also needs to be cleared,
i.e. by marking thd->transaction_rollback_request. In the normal
case, this will trigger an error in Rows_log_event::do_apply_event()
and cause a rollback. During the Rows_log_event::do_apply_event()
cleanup of a successful event application, there is a DBUG_ASSERT in
log_event_server.cc::rows_event_stmt_cleanup(), which sets the
expectation that thd->transaction_rollback_request cannot be set
because the general rollback (i.e. not the InnoDB rollback) should
have happened already. However, if the replica is configured to skip
deadlock errors, the rows event logic will clear the error and
continue on, as if no error happened. This results in
thd->transaction_rollback_request being set while in
rows_event_stmt_cleanup(), thereby triggering the assertion.
This patch fixes this in the following ways:
1) The assertion is invalid, and thereby removed.
2) The rollback case is forced in rows_event_stmt_cleanup() if
transaction_rollback_request is set.
Note the differing behavior between transactions which are skipped
due to deadlock errors and other errors. When a transaction is
skipped due to an ignored deadlock error, the entire transaction is
rolled back and skipped (though note MDEV-33930 which allows
statements in the same transaction after the deadlock-inducing one
to commit). When a transaction is skipped due to ignoring a
different error, only the erroring statements are rolled-back and
skipped - the rest of the transaction will execute as normal. The
effect of this can be seen in the test results. The added test case
to rpl_skip_error.test shows that only statements which are ignored
due to non-deadlock errors are ignored in larger transactions. A
diff between rpl_temporary_error2_skip_all.result and
rpl_temporary_error2.result shows that all statements in the errored
transaction are rolled back (diff pasted below):
: diff rpl_temporary_error2.result rpl_temporary_error2_skip_all.result
49c49
< 2 1
---
> 2 NULL
51c51
< 4 1
---
> 4 NULL
53c53
< * There will be two rows in t2 due to the retry.
---
> * There will be one row in t2 because the ignored deadlock does not retry.
57d56
< 1
59c58
< 1
---
> 0
Reviewed By:
============
Andrei Elkin <andrei.elkin@mariadb.com>
On Windows systems, occurrences of ERROR_SHARING_VIOLATION due to
conflicting share modes between processes accessing the same file can
result in CreateFile failures.
mysys' my_open() already incorporates a workaround by implementing
wait/retry logic on Windows.
But this does not help if files are opened using shell redirection like
mysqltest traditionally did it, i.e via
--echo exec "some text" > output_file
In such cases, it is cmd.exe, that opens the output_file, and it
won't do any sharing-violation retries.
This commit addresses the issue by introducing a new built-in command,
'write_line', in mysqltest. This new command serves as a brief alternative
to 'write_file', with a single line output, that also resolves variables
like "exec" would.
Internally, this command will use my_open(), and therefore retry-on-error
logic.
Hopefully this will eliminate the very sporadic "can't open file because
it is used by another process" error on CI.
The test failure in rpl.rpl_domain_id_filter_restart is caused by
MDEV-33887. That is, the test uses master_pos_wait() (called
indirectly by sync_slave_with_master) to try and wait for the
replica to catch up to the master. However, the waited on
transaction is ignored by the configured
CHANGE MASTER TO IGNORE_DOMAIN_IDS=()
As MDEV-33887 reports, due to the IO thread updating the binlog
coordinates and the SQL thread updating the GTID state, if the
replica is stopped in-between these updates, the replica state will
be inconsistent. That is, the test expects that the GTID state will
be updated, so upon restart, the replica will be up-to-date.
However, if the replica is stopped before the SQL thread updates its
GTID state, then upon restart, the replica will fetch the previously
ignored event, which is no longer ignored upon restart, and execute
it. This leads to the sporadic extra row in t2.
This patch changes master_pos_wait() to use master_gtid_wait() to
ensure the replica state is consistent with the master state.
increase the MASTER_CONNECT_RETRY time under valgrind,
otherwise the slave gives up retrying before the master is ready
also, cosmetic cleanup of rpl_semi_sync_master_shutdown.test
The crash at running mysqlbinlog on a SEQUENCE containing binlog file
was caused MDEV-29621 fixes that did not check which of the slave
or binlog applier executes a block introduced there.
The block is meaningful only for the parallel slave applier, so
it's safe to fix this bug with identified the actual applier and
skipping the block when it's the mysqlbinlog one.
The signal handler thread can use various different runtime
resources when processing a SIGHUP (e.g. master-info information)
due to calling into reload_acl_and_cache(). Currently, the shutdown
process waits for the termination of the signal thread after
performing cleanup. However, this could cause resources actively
used by the signal handler to be freed while reload_acl_and_cache()
is processing.
The specific resource that caused MDEV-30260 is a race condition for
the hostname_cache, such that mysqld would delete it in
clean_up()::hostname_cache_free(), before the signal handler would
use it in reload_acl_and_cache()::hostname_cache_refresh().
Another similar resource is the active_mi/master_info_index. There
was a race between its deletion by the main thread in end_slave(),
and their usage by the Signal Handler as a part of
Master_info_index::flush_all_relay_logs.read(active_mi) in
reload_acl_and_cache().
This patch fixes these race conditions by relocating where server
shutdown waits for the signal handler to die until after
server-level threads have been killed (i.e., as a last step of
close_connections()). With respect to the hostname_cache, active_mi
and master_info_cache, this ensures that they cannot be destroyed
while the signal handler is still active, and potentially using
them.
Additionally:
1) This requires that Events memory is still in place for SIGHUP
handling's mysql_print_status(). So event deinitialization is moved
into clean_up(), but the event scheduler still needs to be stopped
in close_connections() at the same spot.
2) The function kill_server_thread is no longer used, so it is
deleted
3) The timeout to wait for the death of the signal thread was not
consistent with the comment. The comment mentioned up to 10 seconds,
whereas it was actually 0.01s. The code has been fixed to wait up to
10 seconds.
4) A warning has been added if the signal handler thread fails to
exit in time.
5) Added pthread_join() to end of wait_for_signal_thread_to_end()
if it hadn't ended in 10s with a warning. Note this also removes
the pthread_detached attribute from the signal_thread to allow
for the pthread_join().
Reviewed By:
===========
Vladislav Vaintroub <wlad@mariadb.com>
Andrei Elkin <andrei.elkin@mariadb.com>
Keep track of each recently active XID, recording which worker it was queued
on. If an XID might still be active, choose the same worker to queue event
groups that refer to the same XID to avoid conflicts.
Otherwise, schedule the XID freely in the next round-robin slot.
This way, XA PREPARE can normally be scheduled without restrictions (unless
duplicate XID transactions come close together). This improves scheduling
and parallelism over the old method, where the worker thread to schedule XA
PREPARE on was fixed based on a hash value of the XID.
XA COMMIT will normally be scheduled on the same worker as XA PREPARE, but
can be a different one if the XA PREPARE is far back in the event history.
Testcase and code for trimming dynamic array due to Andrei.
Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
A GTID event can have variable length, with contributing factors
such as the variable length from the flags2 and optional extra flags
fields. These fields are bitmaps, where each set bit indicates an
additional value that should be appended to the event, e.g.
multi-engine transactions append a number to indicate the number of
additional engines a transaction uses. However, if a flags bit is
set, and no additional fields are appended to the event, MDEV-33672
reports that the server can still try to read from memory as if it
did exist. Note, however, in debug builds, this condition is
asserted for FL_EXTRA_MULTI_ENGINE.
This patch fixes this to check that the length of the event is
aligned with the expectation set by the flags for FL_PREPARED_XA,
FL_COMPLETED_XA, and FL_EXTRA_MULTI_ENGINE.
Reviewed By
============
Kristian Nielsen <knielsen@knielsen-hq.org>
MDEV-26473 fixed a segmentation fault at startup between the handle
manager thread and the binlog background thread, such that the
binlog background thread could be started and submit a job to the
handle manager, before it had initialized. Where MDEV-26473 made it
so the handle manager would initialize before the main thread
started the normal binary logs, it did not account for the recovery
case. That is, there is still a possibility of a segmentation fault
when a server is recovering using the binary logs such that it can
open the binary logs, start the binlog background thread, and submit
a job to the handle manager before it is initialized.
This patch fixes this by moving the initialization of the mysql
handler manager to happen prior to recovery.
Reviewed By:
============
Andrei Elkin <andrei.elkin@mariadb.com>
Some fixes related to commit f838b2d799 and
Rows_log_event::do_apply_event() and Update_rows_log_event::do_exec_row()
for system-versioned tables were provided by Nikita Malyavin.
This was required by test versioning.rpl,trx_id,row.
When using semi-sync replication with
rpl_semi_sync_master_wait_point=AFTER_COMMIT, the performance of the
primary can significantly reduce compared to AFTER_SYNC's
performance for workloads with many concurrent users executing
transactions. This is because all connections on the primary share
the same cond_wait variable/mutex pair, so any time an ACK is
received from a replica, all waiting connections are awoken to check
if the ACK was for itself, which is done in mutual exclusion.
This patch changes this such that the waiting THD will use its own
local condition variable, and the ACK receiver thread only signals
connections which have been ACKed for wakeup. That is, the
THD::LOCK_wakeup_ready condition variable is re-used for this
purpose, and the Active_tranx queue nodes are extended to hold the
waiting thread, so it can be signalled once ACKed.
Additionally:
1) Removed part of MDEV-11853 additions, which allowed suspended
connection threads awaiting their semi-sync ACKs to live until their
ACKs had been received. This part, however, wasn't needed. That is,
all that was needed was for the Ack_thread to survive. So now the
connection threads are killed during phase 1. Thereby
THD::is_awaiting_semisync_ack, and all its related code was removed.
2) COND_binlog_send is repurposed to signal on the condition when
Active_tranx is emptied during clear_active_tranx_nodes.
3) At master shutdown (when waiting for slaves), instead of the
main loop individually waiting for each ACK, await_slave_reply()
(renamed await_all_slave_replies()) just waits once for the
repurposed COND_binlog_send to signal it is empty.
4) Test rpl_semi_sync_shutdown_await_ack is updates as following:
4.1) Added test case (adapted from Kristian Nielsen) to ensure
that if a thread awaiting its ACK is killed while SHUTDOWN WAIT FOR
ALL SLAVES is issued, the primary will still wait for the ACK from
the killed thread.
4.2) As connections which by-passed phase 1 of thread killing no
longer are delayed for kill until phase 2, we can no longer query
yes/no tx after receiving an ACK/timeout. The check for these
variables is removed.
4.3) Comment descriptions are updated which mention that the
connection is alive; and adjusted to be the Ack_thread.
Reviewed By:
============
Kristian Nielsen <knielsen@knielsen-hq.org>
Though the test itself doesn't create any transactions
directly, the added test suppressions are replicated,
and when the SQL thread is stopped mid-execution,
it is set into an error state because these are
non-transactional events being aborted.
This patch fixes the test by ensuring that the test
suppressions are fully replicated before continuing
An earlier patch for MDEV-13577 fixed the most common instances of this, but
missed one case for tables without primary key when the scan reaches the end
of the table. This patch adds similar code to handle this case, converting
the error to HA_ERR_RECORD_CHANGED when doing optimistic parallel apply.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
The patch for MDEV-15530 incorrectly added a column in the middle of SHOW
SLAVE STATUS output. This is wrong, as it breaks backwards compatibility
with existing applications and scripts. In this case, it even broke
mariadb-dump, which is included in the server source tree!
Revert the incorrect change, putting the new Replicate_Rewrite_DB at the end
of SHOW SLAVE STATUS output.
Add a testcase for the mariadb-dump --dump-slave wrong output problem. Also
add a testcase rpl.rpl_show_slave_status to hopefully prevent any future
incorrect additions to SHOW SLAVE STATUS.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Warnings are added to net_server.cc when
global_system_variables.log_warnings >= 4.
When the above condition holds then:
- All communication errors from net_serv.cc is also written to the
error log.
- In case of a of not being able to read or write a packet, a more
detailed error is given.
Other things:
- Added detection of slaves that has hangup to Ack_receiver::run()
- vio_close() is now first marking the socket closed before closing it.
The reason for this is to ensure that the connection that gets a read
error can check if the reason was that the socket was closed.
- Add a new state to vio to be able to detect if vio is acive, shutdown or
closed. This is used to detect if socket is closed by another thread.
- Testing of the new warnings is done in rpl_get_lock.test
- Suppress some of the new warnings in mtr to allow one to run some of
the tests with -mysqld=--log-warnings=4. All test in the 'rpl' suite
can now be run with this option.
- Ensure that global.log_warnings are restored at test end in a way
that allows one to use mtr --mysqld=--log-warnings=4.
Reviewed-by: <serg@mariadb.org>,<brandon.nesterenko@mariadb.com>
If a server has a default configuration (e.g. in a my.cnf file) with
rpl_semi_sync_slave_enabled set, on server start, the corresponding
rpl_semi_sync_slave_status variable will also be ON initially, even
if the slave was never configured/started. This is because the
Repl_semi_sync_slave initialization logic (function init_object())
sets the running status to the enabled value during
init_server_components().
This patch fixes this by removing the statement which sets the
semi-sync slave running status from the initialization logic. An
additional change needed from this is to semi-sync recovery: this
status variable was used as a condition to determine binlog
truncation during server recovery. This patch also switches this
condition to reference the global rpl_semi_sync_slave_enabled
variable. Though note, the semi-sync recovery condition is to be
changed entirely with the MDEV-33424 agenda.
Reviewed By:
============
Andrei Elkin <andrei.elkin@mariadb.com>
In an addition to test rpl.rpl_parallel_sbm added by MDEV-32265, the
test uses sleep statements alone to test Seconds_Behind_Master with
delayed replication. On slow running machines, the test can pass the
intended MASTER_DELAY duration and Seconds_Behind_Master can become
0, when the test expects the transaction to still be actively in a
delaying state.
This can be consistently reproduced by adding a sleep statement
before the call to
--let = query_get_value(SHOW SLAVE STATUS, Seconds_Behind_Master, 1)
to sleep past the delay end point.
This patch fixes this by locking the table which the delayed
transaction targets so Second_Behind_Master cannot be updated before
the test reads it for validation.
Aria temporary tables account allocated memory as specific to the current
THD. But this fails for slave threads, where the temporary tables need to be
detached from any specific THD.
Introduce a new flag to mark temporary tables in replication as "global",
and use that inside Aria to not account memory allocations as thread
specific for such tables.
Based on original suggestion by Monty.
Reviewed-by: Monty <monty@mariadb.org>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
binlog_space_limit is a variable in Percona server used to limit the total
size of all binary logs.
This implementation is based on code from Percona server 5.7.
In MariaDB we decided to call the variable max-binlog-total-size to be
similar to max-binlog-size. This makes it easier to find in the output
from 'mariadbd --help --verbose'). MariaDB will also support
binlog_space_limit for compatibility with Percona.
Some internal notes to explain implementation notes:
- When running MariaDB does not delete binary logs that are either
used by slaves or have active xid that are not yet committed.
Some implementation notes:
- max-binlog-total-size is by default 0 (no limit).
- max-binlog-total-size can be changed without server restart.
- Binlog file sizes are checked on startup, or if
max-binlog-total-size is set to a value > 0, not for every log write.
The total size of all binary logs is cached and dynamically updated
when updating the binary log on binary log rotation.
- max-binlog-total-size is checked against existing log files during
serverstart, binlog rotation, FLUSH LOGS, when writing to binary log
or when max-binlog-total-size changes value.
- Option --slave-connections-needed-for-purge with 1 as default added.
This allows one to ensure that we do not delete binary logs if there
is less than 'slave-connections-needed-for-purge' connected.
Without this option max-binlog-total-size would potentially delete
binlogs needed by slaves on server startup or when a slave disconnects
as there are then no connected slaves to protect active binlogs.
- PURGE BINARY LOGS TO ... will be executed as if
slave-connectitons-needed-for-purge would be zero. In other words
it will do the purge even if there is no slaves connected. If there
are connected slaves working on the logs, these will be protected.
- If binary log is on and max-binlog-total_size <> 0 then the status
variable 'Binlog_disk_use' shows the current size of all old binary
logs + the state of the current one.
- Removed test of strcmp(log_file_name, log_info.log_file_name) in
purge_logs_before_date() as this is tested in can_purge_logs()
- To avoid expensive calls of log_in_use() we cache the result for the
last log that is in use by a slave. Future calls to can_purge_logs()
for this binary log will be quickly detected and false will be returned
until a slave starts working on a new log.
- Note that after a binary log rotation caused by max_binlog_size,
the last log will not be purged directly as it is still in use
internally. The next binary log write will purge binlogs if needed.
Reviewer:Kristian Nielsen <knielsen@knielsen-hq.org>
This test was prone to failures for a few reasons, summarized below:
1) MDEV-32168 introduced “only_running_threads=1” to
slave_stop.inc, which allowed the stop logic to bypass an
attempting-to-reconnect IO thread. That is, the IO thread could
realize the master shutdown in `read_event()`, and thereby call into
`try_to_reconnect()`. This would leave the IO thread up when the
test expected it to be stopped. Fixed by explicitly stopping the
IO thread and allowing an error state, as the above case would
lead to errno 2003.
2) On slow systems (or those running profiling tools, e.g. MSAN),
the waiting-for-ack transaction can complete before the system
processes the `SHUTDOWN WAIT FOR ALL SLAVES`. There was shutdown
preparation logic in-between the transaction and shutdown itself,
which contributes to this problem. This patch also moves this
preparation logic before the transaction, so there is less to do
in-between the calls.
3) Changed work-around for MDEV-28141 to use debug_sync instead
of sleep delay, as it was still possible to hit the bug on very
slow systems.
4) Masked MTR variable reset with disable/enable query log
Reviewed By:
============
Kristian Nielsen <knielsen@knielsen-hq.org>
A race condition with the SQL thread, where depending on if it was
killed before or after it had executed the fake/generated IGN_GTIDS
Gtid_list_log_event, may or may not update gtid_slave_pos with the
position of the ignored events. Then, the slave would be restarted
while resetting IGNORE_DOMAIN_IDS to be empty, which would result in
the slave requesting different starting locations, depending on
whether or not gtid_slave_pos was updated. And, because previously
ignored events could now be requested and executed (no longer
ignored), their presence would fail the test.
This patch fixes this in two ways. First, to use GTID positions for
synchronization rather than binlog file positions. Then second, to
synchronize the SQL thread’s gtid_slave_pos with the ignored events
before killing the SQL thread.
To consistently reproduce the test failure, the following patch can
be applied:
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
index f51f5b7deec..de62233acff 100644
--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -3686,6 +3686,12 @@ Gtid_list_log_event::do_apply_event(rpl_group_info *rgi)
void *hton= NULL;
uint32 i;
+ sleep(1);
+ if (rli->sql_driver_thd->killed || rli->abort_slave)
+ {
+ return 0;
+ }
+
Reviewed By:
============
Kristian Nielsen <knielsen@knielsen-hq.org>
This reverts commit c37b2087b4.
In c37b20887, when re-binlogging a GTID event on a replica,
it will overwrite the thread_id from the primary to be the
value of the slave applier (SQL thread or parallel worker).
This should be the value of the original thread_id on the
master connection though, to both help track temporary
tables, and be consistent with Query_log_event.
Reverting the commit to re-target 11.5, so we can re-test
with the corrected thread_id.
enable ssl + ssl_verify_server_cert in the internal client too
* fix replication tests to disable master_ssl_verify_server_cert
because accounts are passwordless - except rpl.rpl_ssl1
* fix federated/federatedx/connect to disable SSL_VERIFY_SERVER_CERT
because they cannot configure an ssl connection
* fix spider to disable ssl_verify_server_cert, if configuration
says so, as spider _can_ configure an ssl connection
* memory leak in embedded test-connect
