This commit rewrites the rpl_master_has_bug() mechanism to solve a problem
with invalid memory access. The rpl_master_has_bug() mechanism detects
certain bugs depending on the master version, and uses that to enable
specific work-arounds on the slave. The problem was that
rpl_master_has_bug() accessed Relay_log_info::description_event_for_exec
that is not valid to access from concurrent parallel replication worker
threads, only from the SQL driver thread. Thus it could use the wrong event
or access invalid/freed memory.
This patch instead computes a bitmask of detected bugs when the SQL driver
thread processes the format description event, and reads that bitmask with
an atomic load from the worker threads. The bitmask of bugs can only change
when the master restarts with a new version, and we do not replicate events
concurrently across a format description event from a master restart. Thus,
the bitmask is safe to read concurrently from the Relay_log_info object
without locking.
This also avoids an expensive match of each entry in the bug list against
the master server version done for every single call to
rpl_master_has_bug(), which could be quite expensive when done eg. per field
in row events as in Field_string::compatible_field_size().
Also remove redundant conditional in table_def::compatible_with().
Thanks to Andrei Elkin for the idea to safely read the bitmask
concurrently from the Relay_log_info.
Reviewed-by: Andrei Elkin <andrei.elkin@pp.inet.fi>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
If the replication IO Thread receives a Rotate event following a Format
Description event (FDE) with no post-header length for Rotate events,
the Rotate event’s parser constructor indexes
the FDE’s post-header lengths array out of bounds.
This commit defends against this situation by checking before the
constructor that the FDE describes Rotate events as recognized at all.
In practice, because the Binlog Dump thread generates a Fake
`ROTATE_EVENT` **before** sending the FDE, it has pinned
Rotate events’ post-header length to 8 regardless of FDEs.
This fix solution considers that the FDE’s description should still be
respected, matching the constructor.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Neither the binary-parsing Format Description Event constructor
nor the constructor-bypassing `get_checksum_alg()` function
validated the content length of the passed event buffer.
If they receive an FDE with undersized contents,
they would obtain corrupt results from an erronous memory location,
if not outright crash the program with that memory error.
This commit fixes both sites by adding content length checks.
Because the goal is not to solve the existence of two binary parsers,
`get_checksum_alg()` receives a check duplicated from the Format
Description constructor and is no longer a function that never errors.
Note, it is implementation detail that `get_checksum_alg()`’s
fix catches the invalidity before any code reaches the
parser-contructor’s fix, though the latter would come
to effect if we refactor `get_checksum_alg()` away.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
A replica can crash when a compressed event from its master carries a
corrupted uncompressed length. A compressed Query event whose length
is near 4GB overwrites the IO thread's stack with the event's content.
A compressed rows event of the same shape makes the replica ask for an
allocation of 4GB. A debug build that gets that memory then fails an
assertion in binlog_buf_uncompress(). On a build where a ulong is 32
bits wide, both events overwrite the IO thread's stack.
The IO thread uncompresses a compressed event before writing the event
to the relay log, and offers the uncompress function 4096 bytes of its
own stack to hold the result. The function computes how large the event
will be once uncompressed, and that size decides where the uncompressed
event goes. An event that fits in the stack buffer is uncompressed
there. A larger event is uncompressed into an allocation the function
makes at that size. With either buffer, the function tells zlib that
the room available is the uncompressed length the event declared.
query_event_uncompress() never bounded the length read out of the
event. The function added the header length to that value, rounded the
sum up for alignment, and cast the result to uint32. The cast dropped
the high bits of a sum above 4GB, so a length near 4GB produced a size
of a few dozen bytes. That size fit the stack buffer, so content of any
size went onto the IO thread's stack. row_log_event_uncompress()
computes that sum in a ulong, which wraps where a ulong is 32 bits
wide. The Query_compressed_log_event constructor and
Rows_log_event::uncompress_buf() size their allocations from the same
unbounded length.
Reject an uncompressed length above MAX_MAX_ALLOWED_PACKET at every
point the length is read, before any size is computed from it. No
master writes a larger length, because max_allowed_packet is capped at
1GB. query_event_uncompress() also keeps that size in a size_t, so the
allocation is made from the whole computed value. Both functions return
the error code for the IO thread to report, so a length over the limit
stops the IO thread with ER_TOO_BIG_FOR_UNCOMPRESS rather than the
generic uncompress error. A compressed event with a corrupted
uncompressed length now stops the replica's IO thread with an error
instead of crashing the replica.
Alexandra Raveala wrote the original patch and its first regression
test, contributed through PR 5413. Brandon Nesterenko saw the work
through for the 10.6 release, replacing the test with one that makes
the master write the corrupt length and reworking how the IO thread
reports the error.
Co-authored-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
A slave stops with a relay log read failure when its master sends an
event whose header declares a length other than the number of bytes the
event arrived in. The file and position the slave reports for that
failure are not where the problem is. A master that logs no checksum
does not stop the slave at all. Such a master can make the slave run one
statement twice, leaving the slave's data holding a row the master's
binary log never carried. A "malicious" master can already send whatever
events it likes, so what this defeats is comparing a slave's applied
stream against the master's binary log.
The slave IO thread reads each event from the master as one network
packet, and writes that packet into the relay log unchanged, using the
packet's own length. Every later reader of that relay log frames the
events by a different length: the one each event's header declares at
EVENT_LEN_OFFSET. A master writing an event sets the two to the same
value. Log_event::read_log_event(), which parses the events that
queue_event() does not construct itself, checks only that the packet
reaches EVENT_LEN_OFFSET, and never compares the declared length with
the length of the packet.
queue_event() never compared the two lengths either. An event declaring
fewer bytes than the packet held reached the relay log with the extra
bytes behind the event. The SQL thread framed its next read from inside
the previous event. Where the master had placed a complete event in
those extra bytes, and no checksum covered the packet, the SQL thread
applied that second event.
This patch adds validation to ensure the lengths are equal. An event
whose lengths disagree will stop the IO thread with
ER_SLAVE_FATAL_ERROR, and the relay log will never receive the event.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
A slave can crash when the master's binlog_checksum setting differs
from the checksum the slave's relay log carries. On the first Rotate
event of such a connection, the slave IO thread copies the event onto
a stack buffer sized for the longest binary log file name a master can
send. A master that names a longer file makes the copy run off the end
of that buffer, and what it writes past the end is the file name
itself. A name that overruns the buffer by a few hundred bytes reaches
only the other buffers of the same stack frame, which the slave does
not read on this path, so it keeps running. A name that overruns the
whole frame takes the return address with it, and the slave crashes.
Builds with AddressSanitizer stop at the copy and report a
stack-buffer-overflow.
The fake Rotate event that opens a connection is rewritten when the
master's checksum policy and the relay log's disagree. One case adds a
checksum to the event, and the other strips one. Both copy the event
into rot_buf using the length the master sent, and neither compared
that length against the size of rot_buf. The Rotate_log_event
constructed just above bounds its own copy of the file name at
FN_REFLEN-1, which leaves the length the master sent untouched.
rot_buf was itself one checksum shorter than the event the first case
produces from the longest file name.
Bound the event before either case copies it. queue_event() now
compares the event's length against the longest Rotate a master can
send: the two headers, a file name of at most FN_REFLEN bytes, and the
checksum the master's own policy puts on the event. rot_buf now
reserves that checksum length as well. A Rotate event naming a longer
file stops the slave IO thread with an error reporting the length the
master sent, instead of running off the end of the buffer.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
A slave can crash when its master sends an event shorter than the
event's own header. A truncated heartbeat makes debug builds fail an
assertion in Binary_string::realloc_raw(). On 64-bit release builds it
stops the slave IO thread with an error message that omits the log
file name. If the master also declares an oversized common header in
its format description event, the slave allocates and fills nearly 4GB
instead. On 32-bit release builds it writes that error message past
the end of a stack buffer. An event under four bytes crashes every
build, because the slave checksums roughly 16 EiB and reads far past
the end of the packet.
Neither queue_event() nor the Heartbeat_log_event constructor bounded
the length the master sent. queue_event() handed that length to
event_checksum_test(), which subtracted the checksum length from it,
and the result wrapped on an event under four bytes. The constructor
computed the log file name length as event_len minus the header
lengths, and on a truncated heartbeat that subtraction wrapped to a
value near 4GB. queue_event() rejected the heartbeat as invalid, so
its error path appended the log file name to the error message using
the wrapped length.
Add the missing bound in both places. queue_event() now rejects an event
shorter than the common header before anything reads that header, and
the slave IO thread stops with an error. The Heartbeat_log_event
constructor now compares event_len against the combined header lengths
before the subtraction. A heartbeat that clears the first check but
still stops inside its headers keeps ident_len at 0 and log_ident at
NULL, so the error path appends nothing. That constructor check also
precedes the read of the extended log position, which previously ran on
a short event before any validation. A short event from the master now
stops the slave IO thread with an error instead of crashing the server.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Protect all READ_ONLY sysvars from run-time changes.
Put them into a separate section and use my_virtual_mem_protect()
to make this section read-only before the server starts accepting
connections.
Verify that they're all protected in the sys_var constructor.
One exception: opt_noacl (--skip-grant-tables) can be changed
from 1 to 0 on FLUSH PRIVILEGES. Let's briefly drop the
protection for this 1->0 change. It can be needed only once
in a server lifetime and only if it was started with --skip-grant-tables
On shutdown the protection is removed, making variables writable
again because shutdown resets some of them during the cleanup
Assisted-By: Claude:claude-4.8-opus
MDEV-36287 acknowledged that SHOW SLAVE STATUS needed to acquire a mutex
lock before accessing the SQL Thread’s THD, as otherwise it may access
invalid memory if a concurrent STOP SLAVE deletes the THD.
But it missed that the SQL Thread’s mutex is `mi->rli.run_lock`,
not `mi->run_lock`, so the bug was still not fixed.
This commit fills in the oversight by acquiring the correct
corresponding lock for each of `Slave_IO_State` & `Slave_SQL_State`.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
If relay log corruption caused an incomplete event group to appear (ie. a
new GTID before the prior event group is complete), this could cause
parallel replication to hang hard. The problem was that the worker thread
handling the incomplete event group would never get a COMMIT event, and thus
never signal the following GCO to start.
Fix by detecting in the SQL driver thread an incomplete event group, and
stopping the slave with an error in this case. The user can then investigate
the problem and recover the corruption as appropriate (eg.
--relay-log-recovery for example).
Giving an error here, rather than rolling back the incomplete transaction,
seems appropriate and safest here, to avoid silent data loss. If the relay
log is corrupted somehow (eg. lost events due to crash for example), we
cannot know that merely rolling back will be a correct fix, it might easily
cause lost transactions for example.
Fix a few bugs/inconsistencies in the rli->is_in_group() so that it can be
used to detect an incomplete event group: Make sure a standalone GTID is
also recognised as an event group, and terminate an incomplete event group
due to master crash or INCIDENT event.
Remove an incorrect assertion in write_ignored_events_info_to_relay_log().
This assertion in the slave IO thread was checking the is_in_group() state,
but that is completely invalid as is_in_group() is the state of the SQL
thread, not the IO thread.
Fix a few incorrect my_error(..., MYF(MY_WME), ...) calls.
Add a test case that simulates the original bug report where this occurred
in a production environment.
Adjust the test case rpl.rpl_parallel_deadlock_corrupt_binlog since
incomplete event group in the relay log now causes an error, not silent
rollback.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
This patch fixes an issue where Int_IO_CACHE::from_chars stops parsing at the
first invalid character but fails to consume the remainder of the line. This
caused trailing garbage on a numeric field (like Master_Port) to be interpreted
as the value for the subsequent field.
The fix introduces a strict validation helper is_string_blank_or_empty which
ensures that only whitespace or control characters follow the parsed numeric
value. The init_*_from_file functions now zero-initialize variables, perform
error checking immediately after string conversion, and safely reject files with
trailing garbage.
The test master_info_numeric_validation has been updated to use --move_file
for robust backup and restoration of the master.info file.
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.
- fix the leak by properly releasing resources before repeat
- rework the error path handling to wait for Wsrep readiness state
instead of repeating endlessly
- improve Wsrep readiness state by also singlaing node's shutdown
Replaced macros with optimized inline functions, that use memcpy
instead of unaligned access (translates to single instruction
on architectures we support). Used compiler byteswap intrinsics
where appropriate.
Removed byte_order_generic.h as well as Intel specific headers
byte_order_generic_{x86|x86_64}.h
Added some casts to places where intNstore writes into "const char*".
Master and relay log positions are 64-bit unsigned but were read using
atoi(), which only handles signed 32-bit integers. Values >= 2^31 overflow
and corrupt after restart.
Add init_ullongvar_from_file() using my_strtoll10() to correctly parse
64-bit values. Update rpl_rli.cc and rpl_mi.cc to use ulonglong variables
and this new function.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Partially revert this commit:
commit 6a1cb449fe
Author: Sergei Golubchik <serg@mariadb.org>
Date: Mon Jan 18 18:02:16 2021 +0100
cleanup: remove slave background thread, use handle_manager thread instead
This restores running the parallel replication deadlock killing in its own
dedicated thread, not in the manager thread shared with other unrelated
processing.
When a parallel replication conflict is detected, multiple threads can be
waiting for each other, potentially in a loop. It is critical for
correctness (as well as performance) that the blocking thread is killed
immediately to allow other threads to continue. If one of the threads being
blocked was the manager thread itself in some unrelated job, the kill could
end up being blocked indefinitely, causing replication to hang, usually
eventually timing out on innodb_lock_wait_timeout and failing replication
with an error like:
[ERROR] Slave worker thread retried transaction 10 time(s) in vain, giving up
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Delayed Replication does not delay on certain metadata events,
but not every type of “non-user” event.
GTID List and Binlog Checkpoint are two of them,
and because the primary sends them after a replica connects,
this replica delays upon START SLAVE even when it’s already up-to-date.
This commit fixes this oddity by changing this condition
set to check the entire non-group event category.
Prevent ER_CONNECTION_KILLED (1927) from being added to the slave_error_mask bitmap in init_slave_skip_errors(), even when explicitly listed in --slave-skip-errors or when using --slave-skip-errors=all. A warning is emitted in the error log when either is attempted.
- 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.
Replication can stop in error if a Heartbeat log event is sent to a
replica during rotation. There are two bugs at play:
1. Prior to MDEV-30128 (added in 11.0), there is a bug when checking
legacy events. When the replica rotates its relay logs, it
initializes its Format_description_log_event with binlog version 3
(this is hard-coded). So immediately after rotation (and until a
new Format_descriptor with binlog_format 4 is sent from the
master), the IO thread is expecting binlog_format 3 (i.e. it will
call queue_old_event() for incoming events). This invalidates any
events that are sent with an event type higher than 14. In theory,
we wouldn't expect any events to be sent in-between a rotate and
the next format descriptor log event, but if a long enough period
of time passes between then, the primary will generate and send a
Heartbeat event (of type 27). In such case, the slave will see the
heartbeat event of type 27, see it is higher than 14, and result
in an error mentioning 'Found invalid event in binary log', with
the expected log coordinates of the new log (which is
optimistically populated from the Rotate log event, not the new
event).
2. In all versions of MariaDB (11.0+), there is a bug when checking
the state of a Heartbeat log event, in that it doesn't consider a
rotated binary log. The check is meant to ensure that the
heartbeat provided by the master (i.e. the state of the master) is
greater than or equal to the state of the slave. In other words,
it checks that the slave isn't ahead of the master. However, if
the filename provided by the master heartbeat event is different
than the filename saved for the slave's state, the check always
fails. This is broken, because when the master rotates its logs,
the new binary log file will have a different filename (i.e. an
incremented index counter suffix). For example, if the master
rotates its binary logs from master-bin.000002 to
master-bin.000003, master-bin.000003 is ahead of
master-bin.000002, but the slave will see a difference between the
filenames and fail the check.
To fix the first problem, this patch disallows passing a heartbeat
event into queue_old_event (which is the source of the error, as it
tries to parse a heartbeat log event). This function (queue_old_event)
was removed with MDEV-30128, so bypassing it for heartbeat events is
not consequential (and it is already also done for
Format_description_events, which are not supported in old binlog file
versions). Note that backporting all of MDEV-30128 was also considered,
but this is less risky for GA.
To fix the second problem, we simply ignore heartbeat events on the
slave if the filenames don't match. This is because during rotation,
it can appear that the slave is ahead of the master, which breaks the
validity of the check (i.e. the check is to ensure the master is
ahead of the slave).
Additionally note that this patch restores a heartbeat check that was
incorrectly removed in 780db8e252
Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
When master-retry-count equals 0, it means the slave will
try to reconnect to the master continuously. But, when the number
of tries err_count overflows to 0, the connection will abort.
So, when an overflow occurs, we still need to check master-retry-cout
value. If master-retry-count equals 0, reconnection should continue.
Reviewed-by: ParadoxV5 <paradox.ver5@gmail.com>
`Seconds_Behind_Master` unsets from 0 when a
parallel replica processes an internal event.
Est. 8dad51481b of MDEV-10653, the idle status of the worker
threads directly checks the emptiness of the workers’ queue.
The problem is that the queue could be entirely
internal events that don’t record replicated content.
In contrast, the (main) SQL thread (in both serial and parallel
replicas) uses a state boolean `sql_thread_caught_up` that is
not unset if the event is internal. Therefore, this patch adds
a workers’ equivalent, `worker_threads_caught_up`, that
matches the behaviour of that for the (main) SQL thread.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Acked-by: Kristian Nielsen <knielsen@knielsen-hq.org>
1. Stricten MDEV-16091’s `rpl.rpl_seconds_behind_master_spike`
to prove that its fix covered MDEV-25999 as well
2. Add a Parallel Replication variant via a new `.combinations` file;
its current failure confirm MDEV-36840.
Because Parallel Replication queues Format Description Events as
position update metaevents, this variant uses a separate set of API-
compatible breakpoints to pause both the main and worker threads.
To support them, this commit replaces the MDEV-33327 patch c75905cacb
with a live-activation solution. In contrast, previous iterations queued
the activation pre-test and relied on other factors to enable the break.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
It is called from the Replica SQL thread, not IO thread;
and takes a Group Info param, likely once upgraded from the Log Info.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
When a replica stops an established semi-sync connection, it is
supposed to kill the corresponding binlog dump thread on the primary
server. However, when connections are configured to use SSL, this new
connection created by the replica to kill the dump thread doesn't have
any logic to configure SSL options, and thereby the connection can't be
made, and the dump thread will never be killed.
This patch adds logic to configure the semi-sync kill connection with
SSL. The exising logic to set up the connection options for the regular
connection was extracted into a function that the semi-sync kill
connection invokes.
Co-author: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This is a safetly fix to try to fix random failures in
parallel_backup_xa_debug reported as:
sync_slave_with_master failed:
'select master_pos_wait('master-bin.000001', 1034, 300, '')' returned -1
One possible reason could be lost signals, which this patch fixes.
In SHOW SLAVE STATUS, do not access members of the SQL thread's THD without
holding mi->run_lock. Otherwise the THD can go away in case of concurrent
STOP SLAVE, leading to invalid memory references and server crash.
Reviewed-by: Monty <monty@mariadb.org>
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
The effect is that 'show processlist' will show the Slave SQL thread
until the thread ends. This may help finding cases where the Slave SQL
thread could hang for some time during the cleanup part.
The Slave SQL thread will have the state "Slave SQL thread ending' during
this stage.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
In Log_event::read_log_event(), don't use IO_CACHE::error of the relay log's
IO_CACHE to signal an error back to the caller. When reading the active
relay log, this flag is also being used by the IO thread, and setting it can
randomly cause the IO thread to wrongly detect IO error on writing and
permanently disable the relay log.
This was seen sporadically in test case rpl.rpl_from_mysql80. The read
error set by the SQL thread in the IO_CACHE would be interpreted as a
write error by the IO thread, which would cause it to throw a fatal
error and close the relay log. And this would later cause CHANGE
MASTER to try to purge a closed relay log, resulting in nullptr crash.
SQL thread is not able to parse an event read from the relay log. This
can happen like here when replicating unknown events from a MySQL master,
potentially also for other reasons.
Also fix a mistake in my_b_flush_io_cache() introduced back in 2001
(fa09f2cd7e) where my_b_flush_io_cache() could wrongly return an error set
in IO_CACHE::error, even if the flush operation itself succeeded.
Also fix another sporadic failure in rpl.rpl_from_mysql80 where the outout
of MASTER_POS_WAIT() depended on timing of SQL and IO thread.
Reviewed-by: Monty <monty@mariadb.org>
Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Partial commit of the greater MDEV-34348 scope.
MDEV-34348: MariaDB is violating clang-16 -Wcast-function-type-strict
The functions queue_compare, qsort2_cmp, and qsort_cmp2
all had similar interfaces, and were used interchangable
and unsafely cast to one another.
This patch consolidates the functions all into the
qsort_cmp2 interface.
Reviewed By:
============
Marko Mäkelä <marko.makela@mariadb.com>
While applying CTAS log event, we peek the relay log to see if CTAS
contains inserted rows or if it's empty.
The peek function didn't check for end-of-file condition when tried to
get the next event from the log, and thus it hanged.
The fix includes checking for end-of-file while peeking for log events
and considering returned XID_EVENT value as a sign of an empty CTAS.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
This is a preparatory commit for MDEV-35109 to make its
testing code cleaner (and harden other tests too).
The DEBUG_DBUG point simulate_delay_semisync_slave_reply
up to this patch used my_sleep() to delay an ACK response,
but sleeps are prone to test failures on machines that
run tests when already having a heavy load (e.g. on
buildbot).
This patch changes this DEBUG_DBUG sleep to use DEBUG_SYNC
to coordinate exactly when a slave should send its reply,
which is safer and faster.
As DEBUG_SYNC can't be used while a server is shutting
down, to synchronize threads with SHUTDOWN WAIT FOR SLAVES
logic, we use and extend wait_for_pattern_in_file.inc to
wait for an informational error message in the logic to
indicate that the shutdown process has reached the
intended state (i.e. indicating that the shutdown has
been delayed to await semi-sync ACKs). Specifically, the
extensions are as follows:
1. wait_for_pattern_in_file.inc is extended with parameter
wait_for_pattern_count as a number that indicates the
number of times a pattern should occur in the file before
return control back to the calling script.
2. search_for_pattern_in_file.inc is extended with parameter
SEARCH_ABORT_IS_SUCCESS to inverse the error/success
logic, so the SEARCH_ABORT condition can be used to
indicate success, rather than error.
The problem was that when using clang + asan, we do not get a correct value
for the thread stack as some local variables are not allocated at the
normal stack.
It looks like that for example clang 18.1.3, when compiling with
-O2 -fsanitize=addressan it puts local variables and things allocated by
alloca() in other areas than on the stack.
The following code shows the issue
Thread 6 "mariadbd" hit Breakpoint 3, do_handle_one_connection
(connect=0x5080000027b8,
put_in_cache=<optimized out>) at sql/sql_connect.cc:1399
THD *thd;
1399 thd->thread_stack= (char*) &thd;
(gdb) p &thd
(THD **) 0x7fffedee7060
(gdb) p $sp
(void *) 0x7fffef4e7bc0
The address of thd is 24M away from the stack pointer
(gdb) info reg
...
rsp 0x7fffef4e7bc0 0x7fffef4e7bc0
...
r13 0x7fffedee7060 140737185214560
r13 is pointing to the address of the thd. Probably some kind of
"local stack" used by the sanitizer
I have verified this with gdb on a recursive call that calls alloca()
in a loop. In this case all objects was stored in a local heap,
not on the stack.
To solve this issue in a portable way, I have added two functions:
my_get_stack_pointer() returns the address of the current stack pointer.
The code is using asm instructions for intel 32/64 bit, powerpc,
arm 32/64 bit and sparc 32/64 bit.
Supported compilers are gcc, clang and MSVC.
For MSVC 64 bit we are using _AddressOfReturnAddress()
As a fallback for other compilers/arch we use the address of a local
variable.
my_get_stack_bounds() that will return the address of the base stack
and stack size using pthread_attr_getstack() or NtCurrentTed() with
fallback to using the address of a local variable and user provided
stack size.
Server changes are:
- Moving setting of thread_stack to THD::store_globals() using
my_get_stack_bounds().
- Removing setting of thd->thread_stack, except in functions that
allocates a lot on the stack before calling store_globals(). When
using estimates for stack start, we reduce stack_size with
MY_STACK_SAFE_MARGIN (8192) to take into account the stack used
before calling store_globals().
I also added a unittest, stack_allocation-t, to verify the new code.
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
The failing test case validates Seconds_Behind_Master for a delayed
slave, while STOP SLAVE is executed during a delay. The test fixes
initially added to the test (commit b04c857596) added a table lock
to ensure a transaction could not finish before validating the
Seconds_Behind_Master field after SLAVE START, but did not address a
possibility that the transaction could finish before running the
STOP SLAVE command, which invalidates the validations for the rest
of the test case. Specifically, this would result in 1) a timeout in
“Waiting for table metadata lock” on the replica, which expects the
transaction to retry after slave restart and hit a lock conflict on
the locked tables (added in b04c857596), and 2) that
Seconds_Behind_Master should have increased, but did not.
The failure can be reproduced by synchronizing the slave to the master
before the MDEV-32265 echo statement (i.e. before the SLAVE STOP).
This patch fixes the test by adding a mechanism to use DEBUG_SYNC to
synchronize a MASTER_DELAY, rather than continually increase the
duration of the delay each time the test fails on buildbot. This is
to ensure that on slow machines, a delay does not pass before the
test gets a chance to validate results. Additionally, it decreases
overall test time because the test can continue immediately after
validation, thereby bypassing the remainder of a full delay for each
transaction.
