This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 12.3.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
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>
Setting slave_skip_errors to 1677 (ER_SLAVE_CONVERSION_FAILED) doesn't work
as intended. The slave aborts replication on column mismatch. This fix
makes it log a warning and set the problematic field to the default
value gracefully.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
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>
The persistence code of CHANGE MASTER values needs to match file
lines with the formatting for the corresponding field’s type.
This is unstructured, repetitive (not DRY), and makes feature expansions
(such as MDEV-28302 `CHANGE MASTER …=DEFAULT`)
error-prone if not difficult.
This commit moves these functions and global constants from
`slave.cc`/`.h`, as well as the Master and Relay Log Info File
entries from `Rpl_mi` and `Rpl_rli`, to dedicated `rpl_*info_file.h`
files and under corresponding structs to facilitate organization.
Namely, this commit wraps those file entries with transparent
structs that inherit from a `Persistent` interface,
or shared helper structs that themselves inherit from `Persistent`.
By moving the file read/write helper functions to or behind
implementations of `Persistent`’s virtual methods,
reading or writing the file only takes a type-agnostic loop
over the (wrapped) CHANGE MASTER values.
* This commit also includes preemptive support
for preserving MDEV-28302’s `=DEFAULT`.
As such, unset fields (namely `master_connect_retry`)
now remember their `DEFAULT` states
rather than whatever the default is at CHANGE MASTER time.
* For consistency’s sake, `master_heartbeat_period` is
now reset at RESET REPLICA instead of CHANGE MASTER.
As this refactor will disconnect it from fixes for
some open bugs in prior versions, this commit also:
* Reimplements the value reader functions to be strict with their input
* Fixes MDEV-38010
number parsing ignores trailing garbage and overflows
* Supercedes MDEV-38020 integer overflow
* Changes master_heartbeat_period from a `float` of seconds
to a `uint32_t` of milliseconds (adding `/1000.0`s as needed)
* Fixes MDEV-35879 `Slave_heartbeat_period` is imprecise
* The maximum of `master_heartbeat_period` has been
increased to 4294967.295, i.e., (2³²-1)÷1000.
* `master_heartbeat_period` now rounds
instead of truncates (rounds down).
* Prepares to make `master_retry_count` 64-bit on
LLP64 (e.g., Windows) to match LP64 (e.g., Linux)
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This patch solves two problems:
1. Rows log events cannot be transmitted to the slave if their
size exceeds slave_max_packet_size (max 1GB at the time of
writing this patch, i.e. MariaDB 12.3)
2. Rows log events cannot be binlogged if they are larger than
4GB because the common binlog event header field event_len is
32-bits.
This patch adds support for fragmenting large Rows_log_events
through a new event type, Partial_rows_log_event. When any given
instantiation of a Rows_log_event (e.g. Write_rows_log_event, etc)
is too large to be sent to a replica (i.e. larger than the value
slave_max_allowed_packet, as configured on a replica), then the rows
event must be fragmented into sub-events (i.e.
Partial_rows_log_events), so the event can be transmitted to the
replica. The replica will then take the content of each of these
Partial_rows_log_events, and join them together into a large
Rows_log_event to be executed as normal. Partial_rows_log_events
are written to the binary log sequentially, and the replica assembles
the events in the order they are binlogged.
To control the size of each Partial_rows_log_event, a new system
variable is added: binlog_row_event_fragment_threshold. All
Partial_rows_log_events within the same group will have this size,
except for the last, which will take up the remaining length of the
underlying Rows_log_event.
Each Partial_rows_log_event stores its sequence number (seq_no) in the
overall series of fragments, the total number of fragments needed to
re-assemble the Rows_log_event (total_fragments), a uchar for flags for
embedding extra data, and any additional data as specified by the
flags. Currently, only the first event in a grouping will have
additional data: it will set the first bit in the flags
(FL_ORIG_EVENT_SIZE) to indicate it will be storing the total size of
the underlying Rows_log_event.
The cached Rows_log_event data is fragmented into
Partial_rows_log_events as follows. The primary will still generate a
Rows_log_event to write to the binlog; however, during the actual
writing process, the raw data of the rows event is split into
fragments, each covering some continuous section of the rows data. A
Partial_rows_log_event is created for each continuous section, and the
Partial_rows_log_events are written sequentially in-place of the
too-large Rows_log_event. The original data to be fragmented will
include a header and data header; however, will not include a checksum,
as each Partial_rows_log_event will have a checksum for validation, as
well as a sequence_number and total number of fragments to ensure all
fragments are present.
The re-assembly and execution of the original Rows_log_event on the
replica happens in Partial_rows_log_event::do_apply_event(). The rgi
is extended with a memory buffer that holds all data for the
original Rows_log_event. As each Partial_rows_log_event is
ingested, its Rows_log_event content is appended to this memory
buffer. Once the last fragment has added its content, a new
Rows_log_event is created using that buffer, and executed.
A new error message is added to indicate that the slave has received an
invalid stream of Partial_rows_log_events:
ER_PARTIAL_ROWS_LOG_EVENT_BAD_STREAM.
Note this commit only adds the server logic for fragmentic and
assembling events, the client logic (mysqlbinlog) is in the next
commit.
Alternative designs considered were:
1. Alternative 1: Change the master-slave communication protocol
such that the master would send events in chunks of size
slave_max_allowed_packet. Though this is still a valid idea,
and would solve the first problem described in this commit
message, this would still leave the limitation that
Rows_log_events could not exceed 4GB. Eventually, this change
should still be addressed (e.g. in MDEV-37853), and for users
of binlog-in-engine (MDEV-34705) which supports out-of-band
binlogging of events, this MDEV-32570 work will be superseded.
2. Alternative 2: Create a generic “Container_log_event” with the
intention to embed various other types of event data for
various purposes, with flags that describe the purpose of a
given container. This seemed overboard, as there is already a
generic Log_event framework that provides the necessary
abstractions to fragment/reassemble events without adding in
extra abstractions.
3. Alternative 3: Add a flag to Rows_log_event with semantics to
overwrite/correct the event_len field of the common event
header to use a 64-bit field stored in the data_header of the
Rows_log_event; and also do alternative 1, so the master would
send the large (> 4GB) rows event in chunks. This approach
would add too much complexity (changing both the binlogging
and transport layer); as well as introduce inconsistency to
the event definition (event_len and next_event_position would
no longer have consistent meanings).
Reviewed-by: Hemant Dangi <hemant.dangi@mariadb.com>
Acked-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This commit adds 3 new status variables to 'show all slaves status':
- Master_last_event_time ; timestamp of the last event read from the
master by the IO thread.
- Slave_last_event_time ; Master timestamp of the last event committed
on the slave.
- Master_Slave_time_diff: The difference of the above two timestamps.
All the above variables are NULL until the slave has started and the
slave has read one query event from the master that changes data.
- Added information_schema.slave_status, which allows us to remove:
- show_master_info(), show_master_info_get_fields(),
send_show_master_info_data(), show_all_master_info()
- class Sql_cmd_show_slave_status.
- Protocol::store(I_List<i_string_pair>* str_list) as it is not
used anymore.
- Changed old SHOW SLAVE STATUS and SHOW ALL SLAVES STATUS to
use the SELECT code path, as all other SHOW ... STATUS commands.
Other things:
- Xid_log_time is set to time of commit to allow slave that reads the
binary log to calculate Master_last_event_time and
Slave_last_event_time.
This is needed as there is not 'exec_time' for row events.
- Fixed that Load_log_event calculates exec_time identically to
Query_event.
- Updated RESET SLAVE to reset Master/Slave_last_event_time
- Updated SQL thread's update on first transaction read-in to
only update Slave_last_event_time on group events.
- Fixed possible (unlikely) bugs in sql_show.cc ...old_format() functions
if allocation of 'field' would fail.
Reviewed By:
Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Kristian Nielsen <knielsen@knielsen-hq.org>
When using binlog_row_image=FULL with sequence table inserts, a
replica can deadlock because it treats full inserts in a sequence as DDL
statements by getting an exclusive lock on the sequence table. It
has been observed that with parallel replication, this exclusive
lock on the sequence table can lead to a deadlock where one
transaction has the exclusive lock and is waiting on a prior
transaction to commit, whereas this prior transaction is waiting on
the MDL lock.
This fix for this is on the master side, to raise FL_DDL
flag on the GTID of a full binlog_row_image write of a sequence table.
This forces the slave to execute the statement serially so a deadlock
cannot happen.
A test verifies the deadlock also to prove it happen on the OLD (pre-fixes)
slave.
OLD (buggy master) -replication-> NEW (fixed slave) is provided.
As the pre-fixes master's full row-image may represent both
SELECT NEXT VALUE and INSERT, the parallel slave pessimistically
waits for the prior transaction to have committed before to take on the
critical part of the second (like INSERT in the test) event execution.
The waiting exploits a parallel slave's retry mechanism which is
controlled by `@@global.slave_transaction_retries`.
Note that in order to avoid any persistent 'Deadlock found' 2013 error
in OLD -> NEW, `slave_transaction_retries` may need to be set to a
higher than the default value.
START-SLAVE is an effective work-around if this still happens.
Problem
========
On a parallel, delayed replica, Seconds_Behind_Master will not be
calculated until after MASTER_DELAY seconds have passed and the
event has finished executing, resulting in potentially very large
values of Seconds_Behind_Master (which could be much larger than the
MASTER_DELAY parameter) for the entire duration the event is
delayed. This contradicts the documented MASTER_DELAY behavior,
which specifies how many seconds to withhold replicated events from
execution.
Solution
========
After a parallel replica idles, the first event after idling should
immediately update last_master_timestamp with the time that it began
execution on the primary.
Reviewed By
===========
Andrei Elkin <andrei.elkin@mariadb.com>
Removed redundant initialisation in unireg_init(): already done by
mysql_init_variables().
Slave threads already check THD::killed, which eliminates the need to
check abort_loop.
Removed unused wsrep_kill_mysql().
This patch changes how old rows in mysql.gtid_slave_pos* tables are deleted.
Instead of doing it as part of every replicated transaction in
record_gtid(), it is done periodically (every @@gtid_cleanup_batch_size
transaction) in the slave background thread.
This removes the deletion step from the replication process in SQL or worker
threads, which could speed up replication with many small transactions. It
also decreases contention on the global mutex LOCK_slave_state. And it
simplifies the logic, eg. when a replicated transaction fails after having
deleted old rows.
With this patch, the deletion of old GTID rows happens asynchroneously and
slightly non-deterministic. Thus the number of old rows in
mysql.gtid_slave_pos can temporarily exceed @@gtid_cleanup_batch_size. But
all old rows will be deleted eventually after sufficiently many new GTIDs
have been replicated.
This is about adding more options to force slave retries
Two new variables has been added:
slave_transaction_retry_errors
- Tells the slave thread to retry transaction for replication when a
query event returns an error from the provided list. Deadlock and
elapsed lock wait timeout errors are automatically added to this list
slave-transaction-retry-interval
- Interval of the slave SQL thread will retry a transaction
in case it failed with a deadlock or elapsed lock wait
timeout or listed in slave_transaction_retry_errors
Other changes:
- Simplifed code for slave_skip_errors (to be aligned with
slave_transaction_retry_errors)
- Renamed print_slave_skip_errors() to make_slave_skip_errors_printable()
- Remove printing error from init_slave_skip_errors as my_bitmap_init()
will do that if needed.
- Generalize has_temporary_error()
Benefits of this patch:
- Removed a lot of calls to strlen(), especially for field_string
- Strings generated by parser are now const strings, less chance of
accidently changing a string
- Removed a lot of calls with LEX_STRING as parameter (changed to pointer)
- More uniform code
- Item::name_length was not kept up to date. Now fixed
- Several bugs found and fixed (Access to null pointers,
access of freed memory, wrong arguments to printf like functions)
- Removed a lot of casts from (const char*) to (char*)
Changes:
- This caused some ABI changes
- lex_string_set now uses LEX_CSTRING
- Some fucntions are now taking const char* instead of char*
- Create_field::change and after changed to LEX_CSTRING
- handler::connect_string, comment and engine_name() changed to LEX_CSTRING
- Checked printf() related calls to find bugs. Found and fixed several
errors in old code.
- A lot of changes from LEX_STRING to LEX_CSTRING, especially related to
parsing and events.
- Some changes from LEX_STRING and LEX_STRING & to LEX_CSTRING*
- Some changes for char* to const char*
- Added printf argument checking for my_snprintf()
- Introduced null_clex_str, star_clex_string, temp_lex_str to simplify
code
- Added item_empty_name and item_used_name to be able to distingush between
items that was given an empty name and items that was not given a name
This is used in sql_yacc.yy to know when to give an item a name.
- select table_name."*' is not anymore same as table_name.*
- removed not used function Item::rename()
- Added comparision of item->name_length before some calls to
my_strcasecmp() to speed up comparison
- Moved Item_sp_variable::make_field() from item.h to item.cc
- Some minimal code changes to avoid copying to const char *
- Fixed wrong error message in wsrep_mysql_parse()
- Fixed wrong code in find_field_in_natural_join() where real_item() was
set when it shouldn't
- ER_ERROR_ON_RENAME was used with extra arguments.
- Removed some (wrong) ER_OUTOFMEMORY, as alloc_root will already
give the error.
TODO:
- Check possible unsafe casts in plugin/auth_examples/qa_auth_interface.c
- Change code to not modify LEX_CSTRING for database name
(as part of lower_case_table_names)
Intermediate commit.
Implement auto-creation of mysql.gtid_slave_pos* tables with needed engines,
if listed in --gtid-pos-auto-engines.
Uses an asynchronous approach to minimise locking overhead.
The list of available tables is extended with a flag. Extra entries are
added for --gtid-pos-auto-engines tables that do not exist yet, marked as
not existing but ready for auto-creation.
If record_gtid() needs a table marked for auto-creation, it sends a request
to the slave background thread to create the table, and continues to use an
existing table for the current and immediately coming transactions.
As soon as the slave background thread has made the new table available, it
will be used for all subsequent relevant transactions in record_gtid().
This asynchronous approach also avoids a lot of complex issues around trying
to do DDL in the middle of an on-going transaction.
The reason for this is that stop slave takes LOCK_active_mi over the
whole operation while some slave operations will also need LOCK_active_mi
which causes deadlocks.
Fixed by introducing object counting for Master_info and not taking
LOCK_active_mi over stop slave or even stop_all_slaves()
Another benefit of this approach is that it allows:
- Multiple threads can run SHOW SLAVE STATUS at the same time
- START/STOP/RESET/SLAVE STATUS on a slave will not block other slaves
- Simpler interface for handling get_master_info()
- Added some missing unlock of 'log_lock' in error condtions
- Moved rpl_parallel_inactivate_pool(&global_rpl_thread_pool) to end
of stop_slave() to not have to use LOCK_active_mi inside
terminate_slave_threads()
- Changed argument for remove_master_info() to Master_info, as we always
have this available
- Fixed core dump when doing FLUSH TABLES WITH READ LOCK and parallel
replication. Problem was that waiting for pause_for_ftwrl was not done
when deleting rpt->current_owner after a force_abort.
The original MySQL patch left some refactoring todo's, possibly
because of known conflicts with other parallel development (like
info-repository feature perhaps).
This patch fixes those todos/refactorings.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Initial merge of delayed replication from MySQL git.
The code from the initial push into MySQL is merged, and the
associated test case passes. A number of tasks are still pending:
1. Check full test suite run for any regressions or .result file updates.
2. Extend the feature to also work for parallel replication.
3. There are some todo-comments about future refactoring left from
MySQL, these should be located and merged on top.
4. There are some later related MySQL commits, these should be checked
and merged. These include:
e134b9362ba0b750d6ac1b444780019622d14aa5
b38f0f7857c073edfcc0a64675b7f7ede04be00f
fd2b210383358fe7697f201e19ac9779879ba72a
afc397376ec50e96b2918ee64e48baf4dda0d37d
5. The testcase from MySQL relies heavily on sleep and timing for
testing, and seems likely to sporadically fail on heavily loaded test
servers in buildbot or distro build farms.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
The function apply_event_and_update_pos() is called with the
rli->data_lock mutex held. However, there seems to be nothing in the
function actually needing the mutex to be held. Certainly not in the
parallel replication case, where sql_slave_skip_counter is always 0
since the non-zero case is handled by the SQL driver thread.
So this patch makes parallel replication use a variant of
apply_event_and_update_pos() without the need to take the
rli->data_lock mutex. This avoids one contended global mutex for each
event executed, which might improve performance on CPU-bound workloads
somewhat.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
When a deadlock kill is detected inside the storage engine, the kill
is not done immediately, to avoid calling back into the storage engine
kill_query method with various lock subsystem mutexes held. Instead the
kill is queued and done later by a slave background thread.
This patch in preparation for fixing TokuDB optimistic parallel
replication, as well as for removing locking hacks in InnoDB/XtraDB in
10.2.
Signed-off-by: Kristian Nielsen <knielsen at knielsen-hq.org>
Revert following bug fix:
Bug#20685029: SLAVE IO THREAD SHOULD STOP WHEN DISK IS
FULL
Bug#21753696: MAKE SHOW SLAVE STATUS NON BLOCKING IF IO
THREAD WAITS FOR DISK SPACE
This fix results in a deadlock between slave IO thread
and SQL thread.
(cherry picked from commit e3fea6c6dbb36c6ab21c4ab777224560e9608b53)
FULL
Bug#21753696: MAKE SHOW SLAVE STATUS NON BLOCKING IF IO
THREAD WAITS FOR DISK SPACE
Problem:
========
Currently SHOW SLAVE STATUS blocks if IO thread waits for
disk space. This makes automation tools verifying
server health block on taking relevant action. Finally this
will create SHOW SLAVE STATUS piles.
Analysis:
=========
SHOW SLAVE STATUS hangs on mi->data_lock if relay log write
is waiting for free disk space while holding mi->data_lock.
mi->data_lock is needed to protect the format description
event (mi->format_description_event) which is accessed by
the clients running FLUSH LOGS and slave IO thread. Note
relay log writes don't need to be protected by
mi->data_lock, LOCK_log is used to protect relay log between
IO and SQL thread (see MYSQL_BIN_LOG::append_event). The
code takes mi->data_lock to protect
mi->format_description_event during relay log rotate which
might get triggered right after relay log write.
Fix:
====
Release the data_lock just for the duration of writing into
relay log.
Made change to ensure the following lock order is maintained
to avoid deadlocks.
data_lock, LOCK_log
data_lock is held during relay log rotations to protect
the description event.
The problem was that wait_for_slave_io_to_start reported that the io thread
was ready, when it was still initializing. This caused test suite to
continue too early, for example before the semi sync plugin was properly
enabled.
Fixed by introducing a new internal stage: "Preparing". Slave_IO_Running is
now set to "Yes" only when all initializing is done and the IO thread is
ready to read things from the master.
The only test affected by this change is rpl_flsh_tbls, which got stuck in
the preparing phase while trying to read the GTID position from a table.
Fixed by having this test waiting for Preparing instead of Yes.
Other things:
- Avoid calling init_and_set_log_file_name() when opening binary log.
- Remove newlines early when reading from index file.
- Ensure that reset_logs() will work even if thd is 0 (Can happen on startup)
- Added thd to sart_slave_threads() for better error handling.
Add log_bin_index, log_bin_basename and relay_log_basename system
variables. Also, convert relay_log_index system variable to
NO_CMD_LINE and implement --relay-log-index as a command line
option.
