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 slave can crash or leak heap content when its master sends a Table_map
event that declares columns whose types need more field metadata than
the event carries. Building its description of the table, the slave
reads past the end of the field metadata the event holds. The bytes
beyond the buffer are decoded as column metadata.
A Table_map event carries a column count, one type byte per column,
and a block of field metadata. A column's type fixes how many metadata
bytes it consumes: two for the string, enum, set, bit, varchar, and
decimal types, one for the blob, float, and high precision temporal
types, none for the rest. The master writes exactly that many bytes
per column, so the metadata block a well formed event carries is the
sum over its columns. The slave sizes the metadata block from the
event and, while building its table_def, walks the columns in order
and reads each type's metadata bytes from that block.
The slave checked only that the metadata block was no larger than two
bytes per column. It never checked the block against the column types
themselves. table_def() then walked every declared column and indexed
the metadata block for each, so a column whose type needed metadata
the block did not hold read past the end of it. A column count large
enough carried that read far past the allocation.
The Table_map reader now sums the metadata the declared column types
consume and rejects the event when the block is smaller than that
sum, so is_valid() reports the event as invalid and the SQL thread
stops with a relay log read failure before any column is decoded. A
block larger than the sum is still accepted, because every consumer
reads only the bytes the types call for. A column type this slave
does not recognize counts as zero in the sum, so Table_map events
from a newer master using such a type still parse. A Table_map whose
metadata cannot cover its column types now stops the slave with an
error instead of reading past the buffer.
Note that the regression test does not crash a regular debug build;
an ASAN build is required to show the invalid memory access. The 24
column table map it injects overruns the metadata allocation by a
bounded amount that lands in mapped heap, which a debug build reads
without faulting and then stops the SQL thread with
ER_SLAVE_CONVERSION_FAILED on the column type mismatch.
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 a Table_map event whose table
name is longer than an identifier can be. The SQL thread applying the
event copies the name into a fixed buffer and overruns it, and the SQL
thread crashes.
A Table_map event carries its database and table names each behind a
one-byte length. The constructor that parses the event sizes a buffer
for each name from its declared length, and do_apply_event() copies
both names out of the event into NAME_LEN+1 byte buffers with strmov().
strmov() copies up to the terminating null. A real identifier is at
most NAME_LEN bytes, so the name fits the buffer and carries that null.
The constructor never bounded the two lengths. Each length is read as a
byte, so each can reach 255. The constructor copied each name with
strncpy() over the declared length, which writes no terminating null
when the source holds none. do_apply_event() then copied the name into
its NAME_LEN+1 byte buffer with strmov(), which ran past the buffer
until it reached a null elsewhere in the heap. pack_info() read the
same names with %s for SHOW RELAYLOG EVENTS, past the buffer in the
same way.
Bound the lengths in the constructor. It now rejects a Table_map event
whose declared database name, or whose declared table name, exceeds
NAME_LEN, before it sizes the name buffers, so is_valid() returns
false. It also terminates each name it copies, so a source that holds
no null does not leave the name unterminated. A master naming a table
with an oversized name now stops the slave's SQL thread with
ER_SLAVE_RELAY_LOG_READ_FAILURE instead of crashing it.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
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>
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>
When a replica already has Using_Gtid=No and a CHANGE MASTER TO is
issued with log coordinates (e.g. relay_log_pos, master_log_file),
the server emits a spurious warning:
Note 4190 CHANGE MASTER TO is implicitly changing the value of
'Using_Gtid' from 'No' to 'No'
The value isn't actually changing, so the warning is misleading.
In change_master() (sql/sql_repl.cc), when log coordinates are specified
without an explicit master_use_gtid, the code implicitly sets Using_Gtid
to No and emits a warning. The condition only checks whether
master_use_gtid=No was explicitly given but does not check whether
Using_Gtid is already No.
Added a check that the current Using_Gtid value differs from
USE_GTID_NO before emitting the warning. The warning now only fires
when the value actually changes.
Re-recorded rpl.rpl_from_mysql80 which previously expected the
spurious No-to-No warning.
Reviewed-by: Georgi Kodinov <joro@mariadb.org>
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
https://github.com/MariaDB/server/pull/4678
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*".
Many CHANGE MASTER fields typically have the same
configurations between multi-source connections – namely:
master_connect_retry
master_ssl
master_ssl_ca
master_ssl_capath
master_ssl_cert
master_ssl_cipher
master_ssl_key
master_ssl_verify_server_cert
master_ssl_crl
master_ssl_crlpath
master_use_gtid
master_retry_count
master_heartbeat_period
When MDEV-25674 added `master_retry_count` to CHANGE MASTER, it kept
the server option `--master-retry-count` to be its default value.
This commit back-adds corresponding server options
for the defaults of the rest of those fields.
With them, the command line or config files can set up common
configurations across replication sources (and even replicas).
`--autoset-master-use-gtid` and `--autoset-master-heartbeat-period` can
also reset their prior corresponding options back to their unset states.
CHANGE MASTER can override unset
(defaulted) configurations per connection.
This commit also adds `DEFAULT` keyword support for all of those fields,
so overridden configurations can reset
to the default without RESET REPLICA.
Supporting passing the `DEFAULT` keyword also enables setting
`master_connect_retry` and `master_retry_count` to 0,
which was previously disregarded.
While here, the commit also increases `master_retry_count`
to 64-bit on LLP64 (e.g., Windows) to match LP64 (e.g., Linux).
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Reviewed-by: Andrei Elkin <andrei.elkin@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>
Other things:
- Extended mysqltest to write GTID's for master and slave if
sync_slave_with_master fails.
Reviewer: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Implement an improved binlog implementation that is integrated into
the storage engine. The new implementation is enabled with the
--binlog-storage-engine option. Initially the InnoDB storage engine
implements the binlog.
Integrating the binlog in the storage engine improves performance,
since it makes the InnoDB redo log the single source of truth and
avoids the need for expensive two-phase commit between binlog and
engine. It also makes it possible to disable durability (set
--innodb-flush-log-at-trx-commit=0) to further improve performance,
while still preserving the ability to recover the binlog and database
into a consistent state after a crash.
The new binlog implementation also greatly improves the internal
design and implementation of the binlog, and enables future
enhancements for replication.
This is a squash of the original 11.4-based patch series.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
`LOG_EVENT_OFFSET` is a duplicate of `EVENT_TYPE_OFFSET`
and `BIN_LOG_HEADER_SIZE` depending on the context.
While there, centralize `BIN_LOG_HEADER_SIZE` to `sql/rpl_constants.h`
(previously from `sql/unireg.h` and `client/mysqlbinlog.cc`)
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
CHANGE MASTER can set `Master_Retry_Count` to be lower than
`Connects_Tried`, which’ll look strange to those unaware of that CHANGE.
Now, setting `Master_Retry_Count` also resets `Connects_Tried` to 0.
Reviewed-by: Susil Behera <susil.behera@mariadb.com>
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
When the IO thread (re)connect to a primary,
no updates are available besides unique errors that cause the failure.
These new `Master_info` numbers supplement SHOW SLAVE STATUS’s (most-
recent) ‘Connecting’ state with statistics on (re)connect attempts:
* `Connects_Tried`: how many retries have been attempted so far
This was previously a local variable that only counted re-attempts;
it’s now meaningful even after the “Connecting” state concludes.
* `Master_Retry_Count` (from MDEV-25674): out of how many configured
Side-note: Some of the tests updated by this commit dump the entire
SHOW SLAVE STATUS, which might include non-deterministic entries.
Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This new CHANGE MASTER TO field specifies the `--master-retry-count`
(global option: the number of Primary connection attempts)
for each multi-source replica; i.e, per-channel `performance_schema.`
`replication_connection_configuration.CONNECTION_RETRY_COUNT`.
`--master-retry-count` remains the default for new `CHANGE MASTER TO`s.
This new keyword and `master-info` entry
matches those of pre-‘REPLICATION SOURCE’ MySQL.
* rpl.rpl_system_versioning_partitions updated for MDEV-32188
* innodb.row_size_error_log_warnings_3 changed error for MDEV-33658
(checks are done in a different order)
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>
cannot have an assert in Warning_info::push_warning()
because SQL command SIGNAL can set an absolutely arbitrary
message, even an empty one or ending with '\n'
move the assert into push_warning() and my_message_sql().
followup for 9508a44c37
MDEV-29533 Crash when MariaDB is replica of MySQL 8.0
MySQL 8.0 has added the following new events in the MySQL binary log
PARTIAL_UPDATE_ROWS_EVENT
TRANSACTION_PAYLOAD_EVENT
HEARTBEAT_LOG_EVENT_V2
- PARTIAL_UPDATE_ROWS_EVENT is used by MySQL to generate update
statements using JSON_SET, JSON_REPLACE and JSON_REMOVE to make
update of JSON columns more efficient. These events can be
disabled by setting 'binlog-row-value-options=""'
- TRANSACTION_PAYLOAD_EVENT is used by MySQL to signal that a
row event is compressed. It an be disably by setting
'binlog_transaction_compression=0'.
- HEARTBEAT_LOG_EVENT_V2 is written to the binary log many times
per seconds. It can be ignored by the server.
What this patch does:
- If PARTIAL_UPDATE_ROWS_EVENT or TRANSACTION_PAYLOAD_EVENT is found,
the server will stop with an error message of how to disable the
MySQL server to generate such events.
- HEARTBEAT_LOG_EVENT_V2 events are ignored.
- mariadb-binlog will write the name of the new events.
- mariadb-binlog will stop if PARTIAL_UPDATE_ROWS_EVENT or
TRANSACTION_PAYLOAD_EVENT is found, unless --force is given.
- Fixes a crash in mariadb-binlog if a character set unknown to
MariaDB is found. (MDEV-29533)
From Kristian Nielsen:
- Add test case for MySQL 8.0 to MariaDB replication and fixed a
a small typo in post_header_len initialization.
Reviewer: knielsen@mariadb.org
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>
PURGE BINARY LOGS did not always purge binary logs. This commit fixes
some of the issues and adds notifications if a binary log cannot be
purged.
User visible changes:
- 'PURGE BINARY LOG TO log_name' and 'PURGE BINARY LOGS BEFORE date'
worked differently. 'TO' ignored 'slave_connections_needed_for_purge'
while 'BEFORE' did not. Now both versions ignores the
'slave_connections_needed_for_purge variable'.
- 'PURGE BINARY LOG..' commands now returns 'note' if a binary log cannot
be deleted like
Note 1375 Binary log 'master-bin.000004' is not purged because it is
the current active binlog
- Automatic binary log purges, based on date or size, will write a
note to the error log if a binary log matching the size or date
cannot yet be deleted.
- If 'slave_connections_needed_for_purge' is set from a config or
command line, it is set to 0 if Galera is enabled and 1 otherwise
(old default). This ensures that automatic binary log purge works
with Galera as before the addition of
'slave_connections_needed_for_purge'.
If the variable is changed to 0, a warning will be printed to the error
log.
Code changes:
- Added THD argument to several purge_logs related functions that needed
THD.
- Added 'interactive' options to purge_logs functions. This allowed
me to remove testing of sql_command == SQLCOM_PURGE.
- Changed purge_logs_before_date() to first check if log is applicable
before calling can_purge_logs(). This ensures we do not get a
notification for logs that does not match the remove criteria.
- MYSQL_BIN_LOG::can_purge_log() will write notifications to the user
or error log if a log cannot yet be removed.
- log_in_use() will return reason why a binary log cannot be removed.
Changes to keep code consistent:
- Moved checking of binlog_format for Galera to be after Galera is
initialized (The old check never worked). If Galera is enabled
we now change the binlog_format to ROW, with a warning, instead of
aborting the server. If this change happens a warning will be printed to
the error log.
- Print a warning if Galera or FLASHBACK changes the binlog_format
to ROW. Before it was done silently.
Reviewed by: Sergei Golubchik <serg@mariadb.com>,
Kristian Nielsen <knielsen@knielsen-hq.org>
