- developers - lists.mariadb.org

Re: dbe5c20b755: MDEV-31482: Lock wait timeout with INSERT-SELECT, autoinc, and statement-based replication
by Kristian Nielsen 03 Aug '23

03 Aug '23

Hi Marko, I'd like to ask your opinion/review of the below patch for MDEV-31482: https://github.com/MariaDB/server/commit/dbe5c20b755b87f67d87990c3baabc8666… https://jira.mariadb.org/browse/MDEV-31482 As you know, in-order parallel replication has InnoDB report lock waits. If T1 must replicate before T2, and InnoDB reports that T1 is waiting for a lock held by T2, then T2 will be killed (and retried later) so that T1 can replicate. As an optimisation, auto-increment lock waits where _not_ reported, but this turns out to be a bug. This patch just removes the exception so that such waits are reported like other waits. My only concern is the possible performance impact of this. I'm hoping it will not be very big, because in many cases the auto-increment lock is not taken for replicated transactions? My understanding is: - It is not taken for INSERT INTO t VALUES (...) (and maybe not for multi-row "INSERT ... VALUES (...), (...), ..." ?) - SELECT ... INSERT is not safe for statement-based replication, so will be logged as row in MIXED mode and give a warning in STATEMENT. But I wanted to hear you if you know any case and/or MariaDB version where a significant amount of auto-increment lock waits could be expected in parallel replication? And if so, maybe this fix should be pushed only to later MariaDB versions? - Kristian. Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: > revision-id: dbe5c20b755b87f67d87990c3baabc866667e41b (mariadb-10.4.30-2-gdbe5c20b755) > parent(s): a6114df595eeb7aeed4b050c9a3f4640c4320b5f > author: Kristian Nielsen > committer: Kristian Nielsen > timestamp: 2023-07-09 16:45:47 +0200 > message: > > MDEV-31482: Lock wait timeout with INSERT-SELECT, autoinc, and statement-based replication > > Remove the exception that InnoDB does not report auto-increment locks waits > to the parallel replication. > > There was an assumption that these waits could not cause conflicts with > in-order parallel replication and thus need not be reported. However, this > assumption is wrong and it is possible to get conflicts that lead to hangs > for the duration of --innodb-lock-wait-timeout. This can be seen with three > transactions: > > 1. T1 is waiting for T3 on an autoinc lock > 2. T2 is waiting for T1 to commit > 3. T3 is waiting on a normal row lock held by T2 > > Here, T3 needs to be deadlock killed on the wait by T1. > > Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> > > --- > mysql-test/suite/rpl/r/rpl_parallel_autoinc.result | 95 ++++++++++++++ > mysql-test/suite/rpl/t/rpl_parallel_autoinc.test | 140 +++++++++++++++++++++ > sql/sql_class.cc | 6 - > storage/innobase/lock/lock0lock.cc | 8 +- > 4 files changed, 236 insertions(+), 13 deletions(-) > > diff --git a/mysql-test/suite/rpl/r/rpl_parallel_autoinc.result b/mysql-test/suite/rpl/r/rpl_parallel_autoinc.result > new file mode 100644 > index 00000000000..c1829bafa1a > --- /dev/null > +++ b/mysql-test/suite/rpl/r/rpl_parallel_autoinc.result > @@ -0,0 +1,95 @@ > +include/master-slave.inc > +[connection master] > +MDEV-31482: Lock wait timeout with INSERT-SELECT, autoinc, and statement-based replication > +include/rpl_connect.inc [creating slave2] > +include/rpl_connect.inc [creating slave3] > +connection master; > +ALTER TABLE mysql.gtid_slave_pos ENGINE=InnoDB; > +CREATE TABLE t1 (a INT PRIMARY KEY AUTO_INCREMENT, b INT, c INT, INDEX (c)) ENGINE=InnoDB; > +INSERT INTO t1 (b,c) VALUES (0, 1), (0, 1), (0, 2), (0,3), (0, 5), (0, 7), (0, 8); > +CREATE TABLE t2 (a INT PRIMARY KEY, b INT) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (10,1), (20,2), (30,3), (40,4), (50,5); > +CREATE TABLE t3 (a VARCHAR(20) PRIMARY KEY, b INT) ENGINE=InnoDB; > +INSERT INTO t3 VALUES ('row for T1', 0), ('row for T2', 0), ('row for T3', 0); > +include/save_master_gtid.inc > +connection slave; > +include/sync_with_master_gtid.inc > +include/stop_slave.inc > +set @@global.slave_parallel_threads= 3; > +set @@global.slave_parallel_mode= OPTIMISTIC; > +set @@global.innodb_lock_wait_timeout= 20; > +connection master; > +BEGIN; > +UPDATE t3 SET b=b+1 where a="row for T1"; > +INSERT INTO t1(b, c) SELECT 1, t2.b FROM t2 WHERE a=10; > +Warnings: > +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statements writing to a table with an auto-increment column after selecting from another table are unsafe because the order in which rows are retrieved determines what (if any) rows will be written. This order cannot be predicted and may differ on master and the slave > +COMMIT; > +DELETE FROM t1 WHERE c >= 4 and c < 6; > +BEGIN; > +UPDATE t3 SET b=b+1 where a="row for T3"; > +INSERT INTO t1(b, c) SELECT 3, t2.b FROM t2 WHERE a >= 20 AND a <= 40; > +Warnings: > +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statements writing to a table with an auto-increment column after selecting from another table are unsafe because the order in which rows are retrieved determines what (if any) rows will be written. This order cannot be predicted and may differ on master and the slave > +COMMIT; > +include/save_master_gtid.inc > +connection slave1; > +BEGIN; > +SELECT * FROM t3 WHERE a="row for T1" FOR UPDATE; > +a b > +row for T1 0 > +connection slave2; > +BEGIN; > +SELECT * FROM t3 WHERE a="row for T3" FOR UPDATE; > +a b > +row for T3 0 > +connection slave3; > +BEGIN; > +DELETE FROM t2 WHERE a=30; > +connection slave; > +include/start_slave.inc > +connection slave2; > +ROLLBACK; > +connection slave1; > +ROLLBACK; > +connection slave3; > +ROLLBACK; > +connection slave; > +include/sync_with_master_gtid.inc > +SELECT * FROM t1 ORDER BY a; > +a b c > +1 0 1 > +2 0 1 > +3 0 2 > +4 0 3 > +6 0 7 > +7 0 8 > +8 1 1 > +9 3 2 > +10 3 3 > +11 3 4 > +SELECT * FROM t2 ORDER BY a; > +a b > +10 1 > +20 2 > +30 3 > +40 4 > +50 5 > +SELECT * FROM t3 ORDER BY a; > +a b > +row for T1 1 > +row for T2 0 > +row for T3 1 > +connection master; > +CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format"); > +DROP TABLE t1, t2, t3; > +connection slave; > +include/stop_slave.inc > +SET @@global.slave_parallel_threads= 0; > +SET @@global.slave_parallel_mode= conservative; > +SET @@global.innodb_lock_wait_timeout= 50; > +include/start_slave.inc > +SELECT @@GLOBAL.innodb_autoinc_lock_mode; > +@@GLOBAL.innodb_autoinc_lock_mode > +1 > +include/rpl_end.inc > diff --git a/mysql-test/suite/rpl/t/rpl_parallel_autoinc.test b/mysql-test/suite/rpl/t/rpl_parallel_autoinc.test > new file mode 100644 > index 00000000000..0e96b4dfb80 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_parallel_autoinc.test > @@ -0,0 +1,140 @@ > +--source include/have_binlog_format_statement.inc > +--source include/have_innodb.inc > +--source include/master-slave.inc > + > +--echo MDEV-31482: Lock wait timeout with INSERT-SELECT, autoinc, and statement-based replication > + > +# The scenario is transactions T1, T2, T3: > +# > +# T1 is waiting for T3 on an autoinc lock > +# T2 is waiting for T1 to commit > +# T3 is waiting on a normal row lock held by T2 > +# > +# This caused a hang until innodb_lock_wait_timeout, because autoinc > +# locks were not reported to the in-order parallel replication, so T3 > +# was not deadlock killed. > + > +--let $lock_wait_timeout=20 > + > +--let $rpl_connection_name= slave2 > +--let $rpl_server_number= 2 > +--source include/rpl_connect.inc > + > +--let $rpl_connection_name= slave3 > +--let $rpl_server_number= 2 > +--source include/rpl_connect.inc > + > +--connection master > +ALTER TABLE mysql.gtid_slave_pos ENGINE=InnoDB; > + > +# A table as destination for INSERT-SELECT > +CREATE TABLE t1 (a INT PRIMARY KEY AUTO_INCREMENT, b INT, c INT, INDEX (c)) ENGINE=InnoDB; > +INSERT INTO t1 (b,c) VALUES (0, 1), (0, 1), (0, 2), (0,3), (0, 5), (0, 7), (0, 8); > + > +# A table as source for INSERT-SELECT. > +CREATE TABLE t2 (a INT PRIMARY KEY, b INT) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (10,1), (20,2), (30,3), (40,4), (50,5); > + > +# A table to help order slave worker threads to setup the desired scenario. > +CREATE TABLE t3 (a VARCHAR(20) PRIMARY KEY, b INT) ENGINE=InnoDB; > +INSERT INTO t3 VALUES ('row for T1', 0), ('row for T2', 0), ('row for T3', 0); > +--source include/save_master_gtid.inc > + > +--connection slave > +--source include/sync_with_master_gtid.inc > +--source include/stop_slave.inc > +--let $save_innodb_lock_wait_timeout= `SELECT @@global.innodb_lock_wait_timeout` > +--let $save_slave_parallel_threads= `SELECT @@global.slave_parallel_threads` > +--let $save_slave_parallel_mode= `SELECT @@global.slave_parallel_mode` > +set @@global.slave_parallel_threads= 3; > +set @@global.slave_parallel_mode= OPTIMISTIC; > +eval set @@global.innodb_lock_wait_timeout= $lock_wait_timeout; > + > +--connection master > +# Transaction T1. > +BEGIN; > +UPDATE t3 SET b=b+1 where a="row for T1"; > +INSERT INTO t1(b, c) SELECT 1, t2.b FROM t2 WHERE a=10; > +COMMIT; > + > +# Transaction T2. > +DELETE FROM t1 WHERE c >= 4 and c < 6; > + > +# Transaction T3. > +BEGIN; > +UPDATE t3 SET b=b+1 where a="row for T3"; > +INSERT INTO t1(b, c) SELECT 3, t2.b FROM t2 WHERE a >= 20 AND a <= 40; > +COMMIT; > + > +--source include/save_master_gtid.inc > + > +--connection slave1 > +# Temporarily block T1 to create the scheduling that triggers the bug. > +BEGIN; > +SELECT * FROM t3 WHERE a="row for T1" FOR UPDATE; > + > +--connection slave2 > +# Temporarily block T3 from starting (so T2 can reach commit). > +BEGIN; > +SELECT * FROM t3 WHERE a="row for T3" FOR UPDATE; > + > +--connection slave3 > +# This critical step blocks T3 after it has inserted its first row, > +# and thus taken the auto-increment lock, but before it has reached > +# the point where it gets a row lock wait on T2. Even though > +# auto-increment lock waits were not reported due to the bug, > +# transitive lock waits (T1 waits on autoinc of T3 which waits on row > +# on T2) _were_ reported as T1 waiting on T2, and thus a deadlock kill > +# happened and the bug was not triggered. > +BEGIN; > +DELETE FROM t2 WHERE a=30; > + > +--connection slave > +--source include/start_slave.inc > + > +# First let T2 complete until it is waiting for T1 to commit. > +--let $wait_condition= SELECT count(*)=1 FROM information_schema.processlist WHERE state='Waiting for prior transaction to commit' and command LIKE 'Slave_worker'; > +--source include/wait_condition.inc > + > +# Then let T3 reach the point where it has obtained the autoinc lock, > +# but it is not yet waiting for a row lock held by T2. > +--connection slave2 > +ROLLBACK; > +--let $wait_condition= SELECT count(*)=1 FROM information_schema.processlist WHERE state='Sending data' and info LIKE 'INSERT INTO t1(b, c) SELECT 3, t2.b%' and time_ms > 500 and command LIKE 'Slave_worker'; > +--source include/wait_condition.inc > + > +# Now let T1 continue, while T3 is holding the autoinc lock but before > +# it is waiting for T2. Wait a short while to give the hang a chance to > +# happen; T1 needs to get to request the autoinc lock before we let T3 > +# continue. (There's a small chance the sleep will be too small, which will > +# let the test occasionally pass on non-fixed server). > +--connection slave1 > +ROLLBACK; > +--sleep 0.5 > + > +# Now let T3 continue; the bug was that this lead to an undetected > +# deadlock that remained until innodb lock wait timeout. > +--connection slave3 > +ROLLBACK; > + > +--connection slave > +--let $slave_timeout= `SELECT $lock_wait_timeout/2` > +--source include/sync_with_master_gtid.inc > +--let $slave_timeout= > +SELECT * FROM t1 ORDER BY a; > +SELECT * FROM t2 ORDER BY a; > +SELECT * FROM t3 ORDER BY a; > + > +# Cleanup. > +--connection master > +CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format"); > +DROP TABLE t1, t2, t3; > + > +--connection slave > +--source include/stop_slave.inc > +eval SET @@global.slave_parallel_threads= $save_slave_parallel_threads; > +eval SET @@global.slave_parallel_mode= $save_slave_parallel_mode; > +eval SET @@global.innodb_lock_wait_timeout= $save_innodb_lock_wait_timeout; > +--source include/start_slave.inc > +SELECT @@GLOBAL.innodb_autoinc_lock_mode; > +--source include/rpl_end.inc > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 73bb654080a..8ed3f8a9c5e 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -5119,12 +5119,6 @@ thd_need_wait_reports(const MYSQL_THD thd) > deadlock with the pre-determined commit order, we kill the later > transaction, and later re-try it, to resolve the deadlock. > > - This call need only receive reports about waits for locks that will remain > - until the holding transaction commits. InnoDB auto-increment locks, > - for example, are released earlier, and so need not be reported. (Such false > - positives are not harmful, but could lead to unnecessary kill and retry, so > - best avoided). > - > Returns 1 if the OTHER_THD will be killed to resolve deadlock, 0 if not. The > actual kill will happen later, asynchronously from another thread. The > caller does not need to take any actions on the return value if the > diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc > index 26388ad95e2..b1cf2152cd6 100644 > --- a/storage/innobase/lock/lock0lock.cc > +++ b/storage/innobase/lock/lock0lock.cc > @@ -6872,13 +6872,7 @@ DeadlockChecker::search() > return m_start; > } > > - /* We do not need to report autoinc locks to the upper > - layer. These locks are released before commit, so they > - can not cause deadlocks with binlog-fixed commit > - order. */ > - if (m_report_waiters > - && (lock_get_type_low(lock) != LOCK_TABLE > - || lock_get_mode(lock) != LOCK_AUTO_INC)) { > + if (m_report_waiters) { > thd_rpl_deadlock_check(m_start->mysql_thd, > lock->trx->mysql_thd); > }

2 1

Proposal: mariadbd should automatically upgrade system tables and data directory on startup
by Otto Kekäläinen 03 Aug '23

03 Aug '23

Hi! I frequently see users running into various problems after upgrading MariaDB and starting the upgraded mariadbd server: - Server might refuse to start, asking users to manually run mariadb-upgrade - Server might start, but misbehave as on-disk data format for regular tables or the system tables has not updated - On systems where mariadb-upgrade runs automatically on MariaDB service startup there might be unnecessary delay if mariadb-upgrade is run multiple times - If running mariadb-upgrade fails or is slow, users might end up in a situation where they re-restart the server and have two mariadb-upgrade scripts running and blocking each other - In some cases user might attempt to run mariadb-upgrade but it fails it the MariaDB server was not running, forcing user to figure out how to manually start the server for upgrades - Users might end up starting server in a mode that does not accept connections with the intent to only run mariadb-upgrade, but then don't understand they need to restart the server to have it serve connections again - Users planning upgrades get confused because because reading about Fedora based distros the DBA is requires to manually run mariadb-upgrade themselves, and reading about Debian based distros DBAs are not required to do this as it is automatic, but they might end up trying to run it manually anyway because of prior experience with Fedora Some of the above issues can be remediated by running 'mariadb_upgrade --check-if-upgrade-is-needed' before actually running mariadb_upgrade in service scripts, but it does not work if server is offline (https://jira.mariadb.org/browse/MDEV-27636) Making users aware that their database is running but mariadb-upgrade has not run can be done via https://github.com/MariaDB/server/pull/2464. However this are just duct tape to the original problem. All of the above could be permanently solved if mariadbd simply did the upgrade by itself automatically. Can we please have this? Can we agree that this would be a desired feature in next MariaDB 11.x version, and if somebody implements this, it would be timely reviewed and merged? There is already design outlined at https://jira.mariadb.org/browse/MDEV-30499

8 19

Problem Upgrading Database
by jashley＠deadacated.com 25 Jul '23

25 Jul '23

Greetings, MariaDB 10.11.4 on Server 2022 Upgrading from 10.11.4 to 11.0.2 or 11.1.1 fails on Phase 5/10: Waiting for startup to complete. Is this a known problem? Thank you, mux

3 2

Re: 64ac6ed3e35: MDEV-31742 incorrect examined rows in case of stored function usage
by Sergei Golubchik 21 Jul '23

21 Jul '23

Hi, Oleksandr, On Jul 21, Oleksandr Byelkin wrote: > revision-id: 64ac6ed3e35 (mariadb-10.4.30-31-g64ac6ed3e35) > parent(s): 55a27bac80c > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2023-07-19 16:14:39 +0200 > message: > > MDEV-31742 incorrect examined rows in case of stored function usage > > The counter is global so we do not need add backup to it > if we do not zero it after taking the backup. > > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index c9e86b6142c..7911c78b6e0 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -5568,7 +5568,6 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup) > The following is added to the old values as we are interested in the > total complexity of the query > */ > - inc_examined_row_count(backup->examined_row_count); may be backup->examined_row_count should be simply removed? Sub_statement_state::examined_row_count, I mean. Not saved, not restored? There's also void THD::add_slow_query_state(Sub_statement_state *backup) { affected_rows+= backup->affected_rows; bytes_sent_old= backup->bytes_sent_old; m_examined_row_count+= backup->examined_row_count; it's not even using inc_examined_row_count(). Also examined_row_count is mentioned in comments in sql_class.cc, if you'll be removing it from Sub_statement_state, don't forget to fix comments to match. > cuted_fields+= backup->cuted_fields; > DBUG_VOID_RETURN; > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: 08403d85b00: MDEV-31677 Assertion failed upon online ALTER with binlog_row_image=NOBLOB
by Sergei Golubchik 21 Jul '23

21 Jul '23

Hi, Nikita, ok to push after making few small changes, see inline: On Jul 20, Nikita Malyavin wrote: > revision-id: 08403d85b00 (mariadb-11.0.1-147-g08403d85b00) > parent(s): a31b2a52119 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2023-07-19 19:08:51 +0400 > message: > > MDEV-31677 Assertion failed upon online ALTER with binlog_row_image=NOBLOB > > Make binlog_prepare_row_images accept image type as an argument. > > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index d4ad0efbf3a..bb2453af6a0 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -7070,7 +7071,7 @@ int THD::binlog_update_row(TABLE* table, Event_log *bin_log, > not needed for binlogging. This is done according to the: > binlog-row-image option. the comment is outdated, should say This is done according to the row_image argument > */ > - binlog_prepare_row_images(table); > + binlog_prepare_row_images(table, row_image); > > size_t const before_maxlen= max_row_length(table, table->read_set, > before_record); > @@ -7194,7 +7195,7 @@ void THD::binlog_prepare_row_images(TABLE *table) > and the handler involved supports this. > */ > if (table->s->primary_key < MAX_KEY && > - (thd->variables.binlog_row_image < BINLOG_ROW_IMAGE_FULL) && > + row_image < BINLOG_ROW_IMAGE_FULL && > !ha_check_storage_engine_flag(table->s->db_type(), > HTON_NO_BINLOG_ROW_OPT)) > { please also repace thd->variables.binlog_row_image in the switch() below. doesn't make any practical difference, just for clarity Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Debugging MDEV-28776, the rare rpl.rpl_mark_optimize_tbl_ddl test failure
by Kristian Nielsen 21 Jul '23

21 Jul '23

Hi Brandon, The test failure in https://jira.mariadb.org/browse/MDEV-28776 (the one with 10 failed retries after deadlock, not the freebsd failures) seems like it could be something serious. But it has been very difficult to track down. I believe you have tried different things, and I also tried hard to reproduce it, so far without success. But it's still failing occasionally in actual buildbot runs. What do you think about pushing the below to the different branches? It makes the test case log InnoDB information about deadlocks encountered. This way, when it fails the next time in buildbot we can see 1) what kind of InnoDB deadlock is triggering the problem, if any; and 2) if it actually gets a deadlock inside InnoDB 10 times in a row, or if it is something else. - Kristian. ----------------------------------------------------------------------- Author: Kristian Nielsen <knielsen(a)knielsen-hq.org> Date: Sun Jul 9 15:18:03 2023 +0200 MDEV-28776: rpl.rpl_mark_optimize_tbl_ddl fails with timeout on sync_with_master This commit just extends the testcase to include some more information when the test failure occurs. The failure has so far only been reproducible in buildbot runs. Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> ----------------------------------------------------------------------- diff --git a/mysql-test/suite/rpl/t/rpl_mark_optimize_tbl_ddl.opt b/mysql-test/suite/rpl/t/rpl_mark_optimize_tbl_ddl.opt new file mode 100644 index 00000000000..3c53d5257b4 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_mark_optimize_tbl_ddl.opt @@ -0,0 +1 @@ +--innodb-print-all-deadlocks=1

3 6

Re: 534031b77e0: MDEV-31400 Simple plugin dependency resolution
by Sergei Golubchik 20 Jul '23

20 Jul '23

Hi, Yuchen, Thanks. Conceptually all good, the approach is correct. See comments below - about the state and about user visible warnings, nothing major. On Jul 18, Yuchen Pei wrote: > revision-id: 534031b77e0 (mariadb-10.9.5-17-g534031b77e0) > parent(s): d8997f875e2 > author: Yuchen Pei > committer: Yuchen Pei > timestamp: 2023-07-03 17:54:56 +1000 > message: > > MDEV-31400 Simple plugin dependency resolution > > diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc > index 5a077a934ac..15717352040 100644 > --- a/sql/sql_plugin.cc > +++ b/sql/sql_plugin.cc > @@ -1435,6 +1435,47 @@ void plugin_unlock_list(THD *thd, plugin_ref *list, size_t count) > DBUG_VOID_RETURN; > } > > +static int plugin_do_initialize(struct st_plugin_int *plugin, uint &state) > +{ > + DBUG_ENTER("plugin_do_initialize"); > + mysql_mutex_assert_not_owner(&LOCK_plugin); > + plugin_type_init init= plugin_type_initialize[plugin->plugin->type]; > + if (!init) > + init= (plugin_type_init) plugin->plugin->init; > + if (init) > + if (int ret= init(plugin)) > + { > + /* Plugin init failed but requested a retry if possible */ > + if (unlikely(ret== HA_ERR_RETRY_INIT)) > + sql_print_warning("Plugin '%s' registration as a %s failed.", > + plugin->name.str, plugin_type_names[plugin->plugin->type].str); > + else > + sql_print_error("Plugin '%s' registration as a %s failed.", > + plugin->name.str, plugin_type_names[plugin->plugin->type].str); > + DBUG_RETURN(ret); > + } > + state= PLUGIN_IS_READY; // plugin->init() succeeded > + > + if (plugin->plugin->status_vars) > + { > + /* > + historical ndb behavior caused MySQL plugins to specify > + status var names in full, with the plugin name prefix. > + this was never fixed in MySQL. > + MariaDB fixes that but supports MySQL style too. > + */ > + SHOW_VAR *show_vars= plugin->plugin->status_vars; > + SHOW_VAR tmp_array[2]= {{plugin->plugin->name, > + (char *) plugin->plugin->status_vars, SHOW_ARRAY}, > + {0, 0, SHOW_UNDEF}}; > + if (strncasecmp(show_vars->name, plugin->name.str, plugin->name.length)) > + show_vars= tmp_array; > + > + if (add_status_vars(show_vars)) > + DBUG_RETURN(1); > + } > + DBUG_RETURN(0); > +} > > static int plugin_initialize(MEM_ROOT *tmp_root, struct st_plugin_int *plugin, > int *argc, char **argv, bool options_only) > @@ -1595,7 +1594,7 @@ int plugin_init(int *argc, char **argv, int flags) > size_t i; > struct st_maria_plugin **builtins; > struct st_maria_plugin *plugin; > - struct st_plugin_int tmp, *plugin_ptr, **reap; > + struct st_plugin_int tmp, *plugin_ptr, **reap, **retry_end, **retry_start; > MEM_ROOT tmp_root; > bool reaped_mandatory_plugin= false; > bool mandatory= true; > @@ -1737,11 +1736,16 @@ int plugin_init(int *argc, char **argv, int flags) > */ > > mysql_mutex_lock(&LOCK_plugin); > + /* List of plugins to reap */ > reap= (st_plugin_int **) my_alloca((plugin_array.elements+1) * sizeof(void*)); > *(reap++)= NULL; > + /* List of plugins to retry */ > + retry_start= retry_end= > + (st_plugin_int **) my_alloca((plugin_array.elements+1) * sizeof(void*)); > > for(;;) > { > + int error; > for (i=0; i < MYSQL_MAX_PLUGIN_TYPE_NUM; i++) > { > HASH *hash= plugin_hash + plugin_type_initialization_order[i]; > @@ -1753,16 +1757,46 @@ int plugin_init(int *argc, char **argv, int flags) > bool plugin_table_engine= lex_string_eq(&plugin_table_engine_name, > &plugin_ptr->name); > bool opts_only= flags & PLUGIN_INIT_SKIP_INITIALIZATION && > - (flags & PLUGIN_INIT_SKIP_PLUGIN_TABLE || > - !plugin_table_engine); > - if (plugin_initialize(&tmp_root, plugin_ptr, argc, argv, opts_only)) > + (flags & PLUGIN_INIT_SKIP_PLUGIN_TABLE || !plugin_table_engine); > + error= plugin_initialize(&tmp_root, plugin_ptr, argc, argv, > + opts_only); > + if (error) > { > plugin_ptr->state= PLUGIN_IS_DYING; > - *(reap++)= plugin_ptr; > + /* The plugin wants a retry of the initialisation, > + possibly due to dependency on other plugins */ > + if (unlikely(error == HA_ERR_RETRY_INIT)) > + *(retry_end++)= plugin_ptr; > + else > + *(reap++)= plugin_ptr; > } > } > } > } > + /* Retry plugins that asked for it */ > + while (retry_start < retry_end) > + { > + st_plugin_int **to_re_retry, **retrying; > + for (to_re_retry= retrying= retry_start; retrying < retry_end; retrying++) > + { > + plugin_ptr= *retrying; > + uint state= plugin_ptr->state; > + mysql_mutex_unlock(&LOCK_plugin); you might want to add here sql_print_information("plugin XXX registration is retried") otherwise it would be confusing to see "plugin XXX registration failed" and then see the plugin perferctly initialized and usable. Alternatively (preferable, even) don't print "plugin registration failed" above, print it when adding a plugin to the reap list. So that the user won't see fail/retry loop at all, this is the inner working of the plugin subsystem, no user serviceable parts inside. > + error= plugin_do_initialize(plugin_ptr, state); > + mysql_mutex_lock(&LOCK_plugin); > + plugin_ptr->state= state; why do you need to manipulate (save/restore) the state here? Is that even correct? You set the state to PLUGIN_IS_DYING earlier and plugin_do_initialize() an change it to PLUGIN_IS_READY on success. You wouldn't want to change PLUGIN_IS_READY to PLUGIN_IS_DYING, would you? If this is the case, then you don't need to save the state at all, instead you need something like if (error) plugin_ptr->state= PLUGIN_IS_DYING; > + if (error == HA_ERR_RETRY_INIT) > + *(to_re_retry++)= plugin_ptr; > + else if (error) > + *(reap++)= plugin_ptr; > + } > + /* If the retry list has not changed, i.e. if all retry attempts > + result in another retry request, empty the retry list */ > + if (to_re_retry == retry_end) > + while (to_re_retry > retry_start) > + *(reap++)= *(--to_re_retry); > + retry_end= to_re_retry; > + } > > /* load and init plugins from the plugin table (unless done already) */ > if (flags & PLUGIN_INIT_SKIP_PLUGIN_TABLE) > @@ -1775,7 +1809,8 @@ int plugin_init(int *argc, char **argv, int flags) > } > > /* > - Check if any plugins have to be reaped > + Merge the retry list to the reap list, then reap the failed > + plugins. Note that during the merge we reverse the order in retry seems like this comment is obsolete > */ > while ((plugin_ptr= *(--reap))) > { > @@ -1788,6 +1823,7 @@ int plugin_init(int *argc, char **argv, int flags) > } > > mysql_mutex_unlock(&LOCK_plugin); > + my_afree(retry_start); > my_afree(reap); > if (reaped_mandatory_plugin && !opt_help) > goto err; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: Client-server protocol improvement proposal: Connection redirection
by Andrey Sidorov 19 Jul '23

19 Jul '23

Hi Otto, thanks for bringing this to my attention. Happy to add redirect support to mysql2, looks like its relatively straightforward and sits on top of the existing protocol Andrey On Wed, 19 Jul 2023 at 15:44, Otto Kekäläinen <otto(a)kekalainen.net> wrote: > Hi Andrey! > > I am reaching out to you because you maintain > https://github.com/sidorares/node-mysql2 > > Have you heard about this proposal on the MariaDB developers list? > > It suggests adding a new feature to the MariaDB/MySQL protocol where > the server would be able to tell clients "No accepting connections - > connect to this server instead". The implementation would be work by > sending a new error type ERR_REDIRECT along with a client the > IP/hostname of a new server, similar to how HTTP 301 redirect works- > > Full thread visible at > > https://lists.mariadb.org/hyperkitty/list/developers@lists.mariadb.org/thre… > > If you want to participate in discussion, just reply to this message > and change recipient to developers(a)lists.mariadb.org. > > Thanks! > > > ---------- Forwarded message --------- > From: Lenski, Daniel via developers <developers(a)lists.mariadb.org> > Date: Wed, 28 Jun 2023 at 15:24 > Subject: [MariaDB developers] Client-server protocol improvement > proposal: Connection redirection > To: developers(a)lists.mariadb.org <developers(a)lists.mariadb.org> > > > Hi! > > I would like to propose a new feature in the MariaDB client-server > protocol: application-layer redirection of client connections. > > We want the MariaDB server to be able to tell clients connecting to > it, “Sorry, this server is unavailable. Connect to an alternate server > instead.” This mechanism is inspired by HTTP 302 (“temporary > redirect”) mechanism familiar to all developers of web applications, > and is intended to have similar semantics and security properties, > since these have now been widely deployed and tested for decades. > > I have submitted a minimal but viable implementation of this at: > https://github.com/MariaDB/server/pull/2681, server-side implementation > https://github.com/mariadb-corporation/mariadb-connector-c/pull/226, > MariaDB Connector/C implementation > > > I am seeking advice and opinions on these, and will proceed to > finalize this in the following 1-2 months. > > In its current form, this implementation allows the MariaDB server > administrator to set two variables: > > SET GLOBAL SERVER_REDIRECT_MODE={ON,OFF} (the default is OFF) > SET GLOBAL SERVER_REDIRECT_TARGET='my-new-server.example.com' (or > 192.168.0.123:3307, or new-server.com:3308, etc) > > When SERVER_REDIRECT_MODE is set to on, the server will stop accepting > new TCP-based connections (unless they are to the extra port, which is > intended for emergency administrative use), and it will instead > respond to all clients with an error packet: > > $ mariadb --host my-server.example.com > ERROR 4189 (HY000): |Server is redirecting clients to > 'my-new-server.example.com:3307'|my-new-server.example.com:3307 > > When an appropriately-updated client receives this error packet (error > code 4196, message formatted as |Human readable message|<value of > SERVER_REDIRECT_TARGET>), it will parse the redirection target out > from the error message and attempt a new connection: > > $ updated-mariadb --host my-server.example.com -e 'select @@hostname' > Got server redirect to 'my-new-server.example.com' (port 3307) > +---------------------------+ > | @@hostname | > +---------------------------+ > | my-new-server.example.com | > +---------------------------+ > > The feature is gracefully backwards-compatible. Old clients that do > not support redirection will fail and show the message in a form from > which humans can deduce what happened. New clients that do support it > will follow it to the new server. > > [...] >

1 0

Re: 845fbe9ce01: MDEV-30820 slow log Rows_examined out of range
by Sergei Golubchik 18 Jul '23

18 Jul '23

Hi, Sanja, The patch is ok, but the test case is rather weird. What does it do? How long does it run? On Jul 17, Oleksandr Byelkin wrote: > revision-id: 845fbe9ce01 (mariadb-10.4.30-30-g845fbe9ce01) > parent(s): 02cd3675c4d > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2023-07-17 10:22:54 +0200 > message: > > MDEV-30820 slow log Rows_examined out of range > > Fix row counters to be able to get any possible value. > > diff --git a/mysql-test/main/log_slow.test b/mysql-test/main/log_slow.test > --- a/mysql-test/main/log_slow.test > +++ b/mysql-test/main/log_slow.test > @@ -119,3 +119,98 @@ drop table t; > --echo # > --echo # End of 10.3 tests > --echo # > + > + > +--echo # > +--echo # MDEV-30820: slow log Rows_examined out of range > +--echo # > + > +set @log_output.save= @@global.log_output, @slow_log.save= @@global.slow_query_log; > +set global log_output= 'TABLE', slow_query_log= ON; > +set long_query_time= 0.000001; > +create database db; > +use db; > + > +--delimiter // > +CREATE OR REPLACE FUNCTION `get_id`(`INPUT_STRING` VARCHAR(50), `DECRYPT_KEY` CHAR(20)) RETURNS int(11) > +BEGIN > + DECLARE > + REQUEST_ID INT ; > + SET > + REQUEST_ID = > + IF( > + ( > + LOCATE('#$#$#$ LHP_ID_', INPUT_STRING) > + ) > 0 AND > + ( > + INPUT_STRING REGEXP DECRYPT_KEY > + )>0, > + ( > + SPLIT_STRING( > + SUBSTRING_INDEX( > + SUBSTRING_INDEX(INPUT_STRING, '#$#$#$ LHP_ID_', -1), > + '#$#$#$', > + 1 > + ), > + '-', > + 2 > + ) > + ), > + NULL > + ) ; > + SET > + REQUEST_ID=IF( > + ISNULL(REQUEST_ID)OR(REQUEST_ID='')OR(REQUEST_ID=0), > + NULL, > + REQUEST_ID > + > + ); > + RETURN(REQUEST_ID) ; > +END > +// > + > +CREATE OR REPLACE FUNCTION `SPLIT_STRING`(`str` VARCHAR(255), `delim` VARCHAR(12), `pos` INT) RETURNS varchar(255) CHARSET utf8mb4 > +RETURN REPLACE( > + SUBSTRING( > + SUBSTRING_INDEX(str , delim , pos) , > + CHAR_LENGTH( > + SUBSTRING_INDEX(str , delim , pos - 1) > + ) + 1 > + ) , > + delim , > + '' > +) > +// > + > +CREATE TABLE `tab1` ( > +`ID` int(11) NOT NULL AUTO_INCREMENT, > +`NAME_F` varchar(50) DEFAULT NULL, > + PRIMARY KEY (`ID`) > +) DEFAULT CHARSET=utf8mb4 // > + > + CREATE TABLE `tab2` ( > + `ID` int(11) NOT NULL AUTO_INCREMENT, > + `TAB1_ID` int(11) DEFAULT NULL, > + PRIMARY KEY (`id`) > +) DEFAULT CHARSET=utf8mb4 // > + > +for i in 1..100 do insert into tab1 values (null,'qwerty'); end for ; // > +for i in 1..1000 do insert into tab2 values (null,round(rand()*10000)); end for ; // > + > +--delimiter ; > + > +SELECT > + get_id(CAST(aes_decrypt(tab1.NAME_F,'V41iNM0n4') AS char),'KM_ID_PL') as get_string, > + (CASE WHEN (SELECT ID FROM tab2 where tab2.TAB1_ID = tab1.ID LIMIT 1) IS NULL THEN 0 ELSE 1 END) AS IS_ATT > +FROM > + tab1 > +ORDER BY 2 DESC > +LIMIT 2; > + > +set global log_output= @log_output.save, slow_query_log= @slow_log.save; > +drop database db; > + > +--echo # > +--echo # End of 10.4 tests > +--echo # Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 5

Re: 5102d86beae: MDEV-30164 System variable for default collations
by Sergei Golubchik 17 Jul '23

17 Jul '23

Hi, Alexander, Looks good, thanks. ok to push. a couple of comments below - but really, on remnants of the previous version of the commit. On Jul 17, Alexander Barkov wrote: > revision-id: 5102d86beae (mariadb-10.11.2-47-g5102d86beae) > parent(s): 3430767e006 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2023-07-10 13:17:45 +0400 > message: > > MDEV-30164 System variable for default collations > > This patch adds a way to override default collations > (or "character set collations") for desired character sets. > > diff --git a/mysql-test/main/mysqld--help.result b/mysql-test/main/mysqld--help.result > index 77df8536fa2..5c361e3da88 100644 > --- a/mysql-test/main/mysqld--help.result > +++ b/mysql-test/main/mysqld--help.result > @@ -144,6 +144,8 @@ The following specify which files/extra groups are read (specified before remain > Don't ignore client side character set value sent during > handshake. > (Defaults to on; use --skip-character-set-client-handshake to disable.) > + --character-set-collations=name > + Set default collations for character sets. shouldn't it say "overrides"? > --character-set-filesystem=name > Set the filesystem character set. > -C, --character-set-server=name > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 607877dfb83..d35869e7e39 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -6061,6 +6061,32 @@ mysql_execute_command(THD *thd, bool is_called_from_prepared_stmt) > } > thd->reset_kill_query(); > } > + > + /* > + Binary logging is now done. Unset the "used" flags to avoid > + flags leaking to the next event (and to the COMMIT statement > + in the end of the current event). > + > + Example: > + > + Suppose a non-default collation (in @@character_set_collations) > + was used during the statement, the mysqlbinlog output for > + the current statement will contain a sequence like this: > + > + SET character_set_collations='utf8mb3=utf8mb3_bin'; > + INSERT INTO t1 VALUES (_utf8mb3'test'); > + COMMIT; > + > + The statment (INSERT in this example) is already in binlog at this point, and the > + and the "SET character_set_collations" is written inside a > + Q_CHARACTER_SET_COLLATIONS chunk in its log entry header. > + The flag CHARACTER_SET_COLLATIONS_USED is not needed any more. > + The COMMIT can be printed without "SET character_set_collations". > + > + The same logic applies to the other _USED flags. That's a bit too verbose in my opinion. The example was good in the previous version of the patch, but now the first paragraph of the comment pretty much explains it all. but do it as you prefer. > + */ > + thd->used= 0; > + > if (unlikely(thd->is_error()) || > (thd->variables.option_bits & OPTION_MASTER_SQL_ERROR)) > { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0