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.
- Fix to avoid mysqltest client getting killed abruptly during
mysql_shutdown(). When Galera replication is shutdown, wait for
THDs with `thd->stmt_da()->is_eof()` to disconnect (these are about
to disconnect anyway).
- Extract duplicate code from `wsrep_stop_replication()` and
`wsrep_shutdown_replication()` in a new function.
- No need to use a custom `shutdown_mysqld.inc` in galera
suite. Delete it, so that the one in `mysql-test/include/` is used.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
There were several races in the main.kill_processlist-6619 testcase:
- Lingering connections from a previous test case could be visible in SHOW
PROCESSLIST and cause .result diff.
- A sync point "dispatch_command_end" was ineffective, as it was consumed at
the end of the SET DEBUG command itself.
- The signal from sync point "before_execute_sql_command" could override an
earlier signal, causing DEBUG_SYNC timeout and test failure.
- The final SHOW PROCESSLIST could occasionally see a connection in state
"Busy" instead of the expected "Sleep".
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
In case there is a view that queried from a stored routine or
a prepared statement and this temporary table is dropped between
executions of SP/PS, then it leads to hitting an assertion
at the SELECT_LEX::fix_prepare_information. The fired assertion
was added by the commit 85f2e4f8e8
(MDEV-32466: Potential memory leak on executing of create view statement).
Firing of this assertion means memory leaking on execution of SP/PS.
Moreover, if the added assert be commented out, different result sets
can be produced by the statement SELECT * FROM the hidden table.
Both hitting the assertion and different result sets have the same root
cause. This cause is usage of temporary table's metadata after the table
itself has been dropped. To fix the issue, reload the cache of stored
routines. To do it cache of stored routines is reset at the end of
execution of the function dispatch_command(). Next time any stored routine
be called it will be loaded from the table mysql.proc. This happens inside
the method Sp_handler::sp_cache_routine where loading of a stored routine
is performed in case it missed in cache. Loading is performed unconditionally
while previously it was controlled by the parameter lookup_only. By that
reason the signature of the method Sroutine_hash_entry::sp_cache_routine
was changed by removing unused parameter lookup_only.
Clearing of sp caches affects the test main.lock_sync since it forces
opening and locking the table mysql.proc but the test assumes that each
statement locks its tables once during its execution. To keep this invariant
the debug sync points with names "before_lock_tables_takes_lock" and
"after_lock_tables_takes_lock" are not activated on handling the table
mysql.proc
This will makes it easier to find out what replication workers are
doing and what they are waiting for.
Things changed in processlist:
- Slave_SQL time was not consistent. Now time for state "Slave has
read all relay log; waiting for more updates" shows how long it has
waited for getting the next event.
- Slave_worker threads did often show "Closing tables" for a long
time. Now the state is reverted to the previous state after
"Closing tables" is done.
- Commit and Rollback states where not shown for replication (and some
other threads). Now Commit and Rollback states are always shown and
the state is reverted to previous state when the Commit/Rollback
have finished.
Code changes:
- Added thd->set_time_for_next_stage() for parallel replication when
when starting to wait for prior transactions to commit, group commit,
and FTWRL and for free space in thread pool.
Before we reset the time only after the above events.
- Moved THD_STAGE_INFO(stage_rollback) and THD_STAGE_INFO(stage_commit)
from sql_parse.cc to transaction.cc to ensure this is done for
all commits and not only 'normal connection queries'.
Test case changes:
- close_thread_tables() reverting stage to previous stage caused the
counter in performance_schema to be increased. In many case it is
the 'sql/starting' stage that was effected.
- We only change to "Commit" stage if there is a need for a commit.
This caused some "Commit" stages to disapper from perfschema reports.
TODO in 11.#:
- Slave_IO always showes "Waiting for master to send event" and the time is
from SLAVE START. We should in 11.# change this to be the time since
reading the last event.
Problem was that REPLACE was using consistency check that started
TOI and we tried to rollback it.
Do not use wsrep_before_rollback and wsrep_after_rollback if
we are runing consistency check because no writeset keys are
in that case added. Do not allow consistency check usage
if table storage for target table is not InnoDB, instead
give warning. REPLACE|SELECT INTO ... SELECT will use
now TOI if table storage for target table is not InnoDB
to maintain consistency between galera nodes.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
The old code collected a list of THD's, locked the THD's from getting
deleted by locking two mutex and then later in a separate loop
sent a kill signal to each THD.
The problem with this approach is that, as THD's can be reused,
the second time the THD is killed, the mutex can be taken in
different order, which signals failures in safe_mutex.
Fixed by sending the kill signal directly and not collect the THD's
in a list to be signaled later. This is the same approach we are using
in kill_zombie_dump_threads().
Other things:
- Reset safe_mutex_t->locked_mutex when freed (Safety fix)
rpl_semi_sync_slave_enabled_consistent.test and the first part of
the commit message comes from Brandon Nesterenko.
A test to show how to induce the "Read semi-sync reply magic number
error" message on a primary. In short, if semi-sync is turned on
during the hand-shake process between a primary and replica, but
later a user negates the rpl_semi_sync_slave_enabled variable while
the replica's IO thread is running; if the io thread exits, the
replica can skip a necessary call to kill_connection() in
repl_semisync_slave.slave_stop() due to its reliance on a global
variable. Then, the replica will send a COM_QUIT packet to the
primary on an active semi-sync connection, causing the magic number
error.
The test in this patch exits the IO thread by forcing an error;
though note a call to STOP SLAVE could also do this, but it ends up
needing more synchronization. That is, the STOP SLAVE command also
tries to kill the VIO of the replica, which makes a race with the IO
thread to try and send the COM_QUIT before this happens (which would
need more debug_sync to get around). See THD::awake_no_mutex for
details as to the killing of the replica’s vio.
Notes:
- The MariaDB documentation does not make it clear that when one
enables semi-sync replication it does not matter if one enables
it first in the master or slave. Any order works.
Changes done:
- The rpl_semi_sync_slave_enabled variable is now a default value for
when semisync is started. The variable does not anymore affect
semisync if it is already running. This fixes the original reported
bug. Internally we now use repl_semisync_slave.get_slave_enabled()
instead of rpl_semi_sync_slave_enabled. To check if semisync is
active on should check the @@rpl_semi_sync_slave_status variable (as
before).
- The semisync protocol conflicts in the way that the original
MySQL/MariaDB client-server protocol was designed (client-server
send and reply packets are strictly ordered and includes a packet
number to allow one to check if a packet is lost). When using
semi-sync the master and slave can send packets at 'any time', so
packet numbering does not work. The 'solution' has been that each
communication starts with packet number 1, but in some cases there
is still a chance that the packet number check can fail. Fixed by
adding a flag (pkt_nr_can_be_reset) in the NET struct that one can
use to signal that packet number checking should not be done. This
is flag is set when semi-sync is used.
- Added Master_info::semi_sync_reply_enabled to allow one to configure
some slaves with semisync and other other slaves without semisync.
Removed global variable semi_sync_need_reply that would not work
with multi-master.
- Repl_semi_sync_master::report_reply_packet() can now recognize
the COM_QUIT packet from semisync slave and not give a
"Read semi-sync reply magic number error" error for this case.
The slave will be removed from the Ack listener.
- On Windows, don't stop semisync Ack listener just because one
slave connection is using socket_id > FD_SETSIZE.
- Removed busy loop in Ack_receiver::run() by using
"Self-pipe trick" to signal new slave and stop Ack_receiver.
- Changed some Repl_semi_sync_slave functions that always returns 0
from int to void.
- Added Repl_semi_sync_slave::slave_reconnect().
- Removed dummy_function Repl_semi_sync_slave::reset_slave().
- Removed some duplicate semisync notes from the error log.
- Add test of "if (get_slave_enabled() && semi_sync_need_reply)"
before calling Repl_semi_sync_slave::slave_reply().
(Speeds up the code as we can skip all initializations).
- If epl_semisync_slave.slave_reply() fails, we disable semisync
for that connection.
- We do not call semisync.switch_off() if there are no active slaves.
Instead we check in Repl_semi_sync_master::commit_trx() if there are
no active threads. This simplices the code.
- Changed assert() to DBUG_ASSERT() to ensure that the DBUG log is
flushed in case of asserts.
- Removed the internal rpl_semi_sync_slave_status as it is not needed
anymore. The @@rpl_semi_sync_slave_status status variable is now
mapped to rpl_semi_sync_enabled.
- Removed rpl_semi_sync_slave_enabled as it is not needed anymore.
Repl_semi_sync_slave::get_slave_enabled() contains the active status.
- Added checking that we do not add a slave twice with
Ack_receiver::add_slave(). This could happen with old code.
- Removed Repl_semi_sync_master::check_and_switch() as it is not
needed anymore.
- Ensure that when we call Ack_receiver::remove_slave() that the slave
is removed from the listener before function returns.
- Call listener.listen_on_sockets() outside of mutex for better
performance and less contested mutex.
- Ensure that listening is ignoring newly added slaves when checking for
responses.
- Fixed the master ack_receiver listener is not killed if there are no
connected slaves (and thus stop semisync handling of future
connections). This could happen if all slaves sockets where would be
marked as unreliable.
- Added unlink() to base_ilist_iterator and remove() to
I_List_iterator. This enables us to remove 'dead' slaves in
Ack_recever::run().
- kill_zombie_dump_threads() now does killing of dump threads properly.
- It can now kill several threads (should be impossible but could
happen if IO slaves reconnects very fast).
- We now wait until the dump thread is done before starting the
dump.
- Added an error if kill_zombie_dump_threads() fails.
- Set thd->variables.server_id before calling
kill_zombie_dump_threads(). This simplies the code.
- Added a lot of comments both in code and tests.
- Removed DBUG_EVALUATE_IF "failed_slave_start" as it is not used.
Test changes:
- rpl.rpl_session_var2 added which runs rpl.rpl_session_var test with
semisync enabled.
- Some timings changed slight with startup of slave which caused
rpl_binlog_dump_slave_gtid_state_info.text to fail as it checked the
error log file before the slave had started properly. Fixed by
adding wait_for_pattern_in_file.inc that allows waiting for the
pattern to appear in the log file.
- Tests have been updated so that we first set
rpl_semi_sync_master_enabled on the master and then set
rpl_semi_sync_slave_enabled on the slaves (this is according to how
the MariaDB documentation document how to setup semi-sync).
- Error text "Master server does not have semi-sync enabled" has been
replaced with "Master server does not support semi-sync" for the
case when the master supports semi-sync but semi-sync is not
enabled.
Other things:
- Some trivial cleanups in Repl_semi_sync_master::update_sync_header().
- We should in 11.3 changed the default value for
rpl-semi-sync-master-wait-no-slave from TRUE to FALSE as the TRUE
does not make much sense as default. The main difference with using
FALSE is that we do not wait for semisync Ack if there are no slave
threads. In the case of TRUE we wait once, which did not bring any
notable benefits except slower startup of master configured for
using semisync.
Co-author: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This solves the problem reported in MDEV-32960 where a new
slave may not be registered in time and the master disables
semi sync because of that.
Other things:
- Added DBUG_EXECUTE_IF("print_allocated_thread_memory") at end of query
to easier find not freed memory allocated by THD
- Removed free_root() from plugin_init() that did nothing.
- Allow database level access via `LOCK TABLES` to execute statement
`BACKUP [un]LOCK <object>`
- `BACKUP UNLOCK` works only with `RELOAD` privilege.
In case there is `LOCK TABLES` privilege without `RELOAD` privilege,
we check if backup lock is taken before.
If it is not we raise an error of missing `RELOAD` privilege.
- We had to remove any error/warnings from calling functions because
`thd->get_stmt_da()->m_status` will be set to error and will break
`my_ok()`.
- Added missing test coverage of `RELOAD` privilege to `main.grant.test`
Reviewer: <daniel@mariadb.org>
In some cases "SHOW PROCESSLIST" could show "Reset for next command"
as State, even if the previous query had finished properly.
Fixed by clearing State after end of command and also setting the State
for the "Connect" command.
Other things:
- Changed usage of 'thd->set_command(COM_SLEEP)' to
'thd->mark_connection_idle()'.
- Changed thread_state_info() to return "" instead of NULL. This is
just a safety measurement and in line with the logic of the
rest of the function.
MDEV-32441 SENT_ROWS shows random wrong values when stored function
is selected.
MDEV-32281 EXAMINED_ROWS is not populated in
information_schema.processlist upon SELECT.
Added ROWS_SENT to information_schema.processlist
This is to have the same information as Percona server (SENT_ROWS)
To ensure that information_schema.processlist has correct values for
sent_rows and examined_rows I introduced two new variables to hold the
total counts so far. This was needed as stored functions and stored
procedures will reset the normal counters to be able to count rows for
each statement individually for slow query log.
Other things:
- Selects with functions shows in processlist the total examined_rows
and sent_rows by the main statement and all functions.
- Stored procedures shows in processlist examined_rows and sent_rows
per stored procedure statement.
- Fixed some double accounting for sent_rows and examined_rows.
- HANDLER operations now also supports send_rows and examined_rows.
- Display sizes for MEMORY_USED, MAX_MEMORY_USED, EXAMINED_ROWS and
QUERY_ID in information_schema.processlist changed to 10 characters.
- EXAMINED_ROWS and SENT_ROWS changed to bigint.
- INSERT RETURNING and DELETE RETURNING now updates SENT_ROWS.
- As thd is always up to date with examined_rows, we do not need
to handle examined row counting for unions or filesort.
- I renamed SORT_INFO::examined_rows to m_examined_rows to ensure that
we don't get bugs in merges that tries to use examined_rows.
- Removed calls of type "thd->set_examined_row_count(0)" as they are
not needed anymore.
- Removed JOIN::join_examined_rows
- Removed not used functions:
THD::set_examined_row_count()
- Made inline some functions that where called for each row.
standard table KEY_COLUMN_USAGE should only show keys where
a user has some privileges on every column of the key
standard table TABLE_CONSTRAINTS should show tables where
a user has any non-SELECT privilege on the table or on any column
of the table
standard table REFERENTIAL_CONSTRAINTS is defined in terms of
TABLE_CONSTRAINTS, so the same rule applies. If the user
has no rights to see the REFERENCED_TABLE_NAME value, it should be NULL
SHOW INDEX (and STATISTICS table) is non-standard, but it seems
reasonable to use the same logic as for KEY_COLUMN_USAGE.
The MDEV-29693 conflict resolution is from Monty, as well as is
a bug fix where ANALYZE TABLE wrongly built histograms for
single-column PRIMARY KEY.
Also includes a fix for safe_malloc error reporting.
Other things:
- Copied main.log_slow from 10.4 to avoid mtr issue
Disabled test:
- spider/bugfix.mdev_27239 because we started to get
+Error 1429 Unable to connect to foreign data source: localhost
-Error 1158 Got an error reading communication packets
- main.delayed
- Bug#54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED
This part is disabled for now as it fails randomly with different
warnings/errors (no corruption).
