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.
Changing the way how a the following conditions are evaluated:
WHERE timestamp_column=datetime_const_expr
(for all comparison operators: =, <=>, <, >, <=, >=, <> and for NULLIF)
Before the change it was always performed as DATETIME.
That was not efficient, as involved per-row TIMESTAMP->DATETIME conversion
for timestamp_column. For example, in case of the SYSTEM time zone
it involved a localtime_r() call, which is known to be slow.
After the change it's performed as TIMESTAMP in many cases.
This allows to avoid per-row conversion, as it works the other way around:
datetime_const_expr is converted to TIMESTAMP once before the execution stage.
Note, datetime_const_expr must be inside monotone continuous periods of
the current time zone, i.e. not near these anomalies:
- DST changes (spring forward, fall back)
- leap seconds
Problem:
sp_cache erroneously looked up fully qualified SP names (e.g. `DB`.`SP`),
in case insensitive style. It was wrong, because only the "name"
part is always case insensitive, while the "db" part should be compared
according to lower_case_table_names (case sensitively for 0,
case insensitively for 1 and 2).
Fix:
Adding a "casedn_name" parameter make_qname() to tell
if the name part should be lower cased:
`DB1`.`SP` -> "DB1.SP" (when casedn_name=false)
`DB1`.`SP` -> "DB1.sp" (when casedn_name=true)
and using make_qname() with casedn_name=true when creating
sp_cache hash lookup keys.
Details:
As a result, it now works as follows:
- sp_head::m_db is converted to lower case if lower_case_table_names>0
during the sp_name initialization phase. So when make_qname() is called,
sp_head::m_db is already normalized. There are no changes in here.
- The initialization phase of sp_head when creating sp_head::m_qname
now calls make_qname() with casedn_name=true,
so sp_head::m_name gets written to sp_head::m_qname in lower case.
- sp_cache_lookup() now also calls make_qname() with casedn_name=true,
so sp_head::m_name gets written to the temporary lookup key in lower case.
- sp_cache::m_hashtable now uses case sensitive comparison
Part#1 A non-functional change
Changing the signature of Identifier_chain2::make_qname() from
bool make_qname(MEM_ROOT *mem_root, LEX_CSTRING *dst) const;
to
LEX_CSTRING make_qname(MEM_ROOT *mem_root) const;
Now the result is returned as LEX_CSTRING from the function rather than
is passed as a parameter.
The return value {NULL,0} means "EOM".
This is a requirement step to fix and merge easier
MDEV-33019 The database part is not case sensitive in SP names
The original MDEV-31991 commit commend:
- Moving some of Database_qualified_name methods into a new class
Identifier_chain2.
- Changing the data type of the following variables from
Database_qualified_name to Identifier_chain2:
* q_pkg_proc in LEX::call_statement_start()
* q_pkg_func in LEX::make_item_func_call_generic()
Rationale:
The data type of Database_qualified_name::m_db will be changed
to Lex_ident_db soon. So Database_qualified_name won't be able
to store the `pkg.routine` part of `db.pkg.routine` any more,
because `pkg` must not depend on lower-case-table-names.
Fix old_mode flags conflict between OLD_MODE_NO_NULL_COLLATION_IDS
and OLD_MODE_LOCK_ALTER_TABLE_COPY.
Both flags used to be 1 << 6, now OLD_MODE_LOCK_ALTER_TABLE_COPY changed
to be 1 << 7
Connector/NET does not expect collation IDs returned by "show collations"
to be NULL, runs into an exception.
The fix is to determine connector/net using its connection attributes,
then make sure "show collations" does not output NULL IDs.
The patch introduces new old_mode NO_NULL_COLLATION_IDs, that is
automatically set, once MySQL Connector/NET connection is determined.
A test was added, that uses MySql.Data from powershell - only works
if MySql.Data is installed into GAC (i.e with C/NET MSI package)
The reason for this change are the following:
- If we call set_killed() from one thread to kill another thread with
a message, there may be concurrent usage of the MEM_ROOT which is
not supported (this could cause memory corruption).
We do not currently have code that does this, but the API allows this
and it is better to be fix the issue before it happens.
- The per thread memory tracking does not work if one thread uses
another threads MEM_ROOT.
- set_killed() can be called if a MEM_ROOT allocation fails. In this case
it is not good to try to allocate more memory from potentially the same
MEM_ROOT.
Fix is to use my_malloc() instead of mem_root for killed messages.
This patch is actually follow-up for the task
MDEV-23902: MariaDB crash on calling function
to use correct query arena for a statement. In case invocation of
a function is in progress use its call arena, else use current
query arena that can be either a statement or a regular query arena.
This patch provides a new mode FULL_NODUP to binlog_row_image system
variable. With FULL_NODUP mode, all columns are included in before
image, but only updated columns are included in after image for UPDATE.
While all columns are included in the after image for INSERT.
FULL_NODUP is for replacing FULL mode. It includes all data of
the before and after image as FULL mode, but it uses less storage
especially in the case that only a few columns are updated.
Note: It will binlog full before and after image for all modes if the
table has no primary key. FULL_NODUP follows the behavior.
and to related methods and their parameters:
- The return value of Spvar_definition::m_column_type_ref()
- The parameter of Spvar_definition::set_column_type_ref()
- The method Qualified_column_ident::resolve_type_ref()
- The parameter of LEX::sp_variable_declarations_column_type_finalize()
Checking for kill with thd_kill_level() or check_killed() runs apc
requests, which takes the LOCK_thd_kill mutex. But this is dangerous,
as checking for kill needs to be called while holding many different
mutexes, and can lead to cyclic mutex dependency and deadlock.
But running apc is only "best effort", so skip running the apc if the
LOCK_thd_kill is not available. The apc will then be run on next check
of kill signal.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.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.
Binary logging is now disabled for the queries run by SQL SERVICE.
The binlogging can be turned on with the 'SET SQL_LOG_BIN=On' query.
Conflicts:
sql/sql_prepare.cc
Conflicts:
sql/sql_prepare.cc
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.
