revision-id: fd1eb737e709a2fc9e2b2e93ac8994b7ce3bbd96 (mariadb-10.5.4-375-gfd1eb737e70) parent(s): aa0e3805681552cff5dced141f695c96a4da872f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-12-05 01:16:16 +0300 message: MDEV-24351: S3, same-backend replication: Dropping a table on master... ..causes error on slave. Cause: if the master doesn't have the frm file for the table, DROP TABLE code will call ha_delete_table_force() to drop the table in all available storage engines. The issue was that this code path didn't check for HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE flag for the storage engine, and so did not add "... IF EXISTS" to the statement that's written to the binary log. This can cause error on the slave when it tries to drop a table that's already gone. --- mysql-test/suite/s3/replication.inc | 20 ++++++++++++++++++++ mysql-test/suite/s3/replication_mixed.result | 20 ++++++++++++++++++++ mysql-test/suite/s3/replication_stmt.result | 20 ++++++++++++++++++++ sql/handler.cc | 10 +++++++++- sql/handler.h | 5 ++++- sql/sql_table.cc | 9 ++++++++- 6 files changed, 81 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/s3/replication.inc b/mysql-test/suite/s3/replication.inc index d790c70f221..d30733a4396 100644 --- a/mysql-test/suite/s3/replication.inc +++ b/mysql-test/suite/s3/replication.inc @@ -179,6 +179,26 @@ sync_slave_with_master; --source include/show_binlog_events.inc connection master; +--echo # +--echo # MDEV-24351: S3, same-backend replication: Dropping a table on master +--echo # causes error on slave +--echo # +show variables like 's3_replicate_alter_as_create_select'; + +connection slave; +create table t3 (a int, b int) engine=aria; +insert into t3 values (1,1),(2,2),(3,3); +alter table t3 engine=s3; + +connection master; +let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1); +drop table t3; +--echo # Must show "DROP TABLE IF EXISTS t3", not just "DROP TABLE t3" +--source include/show_binlog_events.inc + +sync_slave_with_master; +connection master; + --echo # --echo # clean up --echo # diff --git a/mysql-test/suite/s3/replication_mixed.result b/mysql-test/suite/s3/replication_mixed.result index 077ae2bf1f9..66ed24d2626 100644 --- a/mysql-test/suite/s3/replication_mixed.result +++ b/mysql-test/suite/s3/replication_mixed.result @@ -272,6 +272,26 @@ slave-bin.000001 # Gtid # # GTID #-#-# slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1`,`t2` /* generated by server */ connection master; # +# MDEV-24351: S3, same-backend replication: Dropping a table on master +# causes error on slave +# +show variables like 's3_replicate_alter_as_create_select'; +Variable_name Value +s3_replicate_alter_as_create_select ON +connection slave; +create table t3 (a int, b int) engine=aria; +insert into t3 values (1,1),(2,2),(3,3); +alter table t3 engine=s3; +connection master; +drop table t3; +# Must show "DROP TABLE IF EXISTS t3", not just "DROP TABLE t3" +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t3` /* generated by server */ +connection slave; +connection master; +# # clean up # connection slave; diff --git a/mysql-test/suite/s3/replication_stmt.result b/mysql-test/suite/s3/replication_stmt.result index 8284c053cac..077029e2d7d 100644 --- a/mysql-test/suite/s3/replication_stmt.result +++ b/mysql-test/suite/s3/replication_stmt.result @@ -272,6 +272,26 @@ slave-bin.000001 # Gtid # # GTID #-#-# slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1`,`t2` /* generated by server */ connection master; # +# MDEV-24351: S3, same-backend replication: Dropping a table on master +# causes error on slave +# +show variables like 's3_replicate_alter_as_create_select'; +Variable_name Value +s3_replicate_alter_as_create_select ON +connection slave; +create table t3 (a int, b int) engine=aria; +insert into t3 values (1,1),(2,2),(3,3); +alter table t3 engine=s3; +connection master; +drop table t3; +# Must show "DROP TABLE IF EXISTS t3", not just "DROP TABLE t3" +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t3` /* generated by server */ +connection slave; +connection master; +# # clean up # connection slave; diff --git a/sql/handler.cc b/sql/handler.cc index 314fe7acb8c..e3d731bae10 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -4996,6 +4996,8 @@ struct st_force_drop_table_params const LEX_CSTRING *alias; int error; bool discovering; + + std::function<void(handlerton*)> callback; }; @@ -5020,6 +5022,7 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) param->error= error; if (error == 0) { + param->callback(hton); param->error= 0; return TRUE; // Table was deleted } @@ -5031,6 +5034,9 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) @brief Traverse all plugins to delete table when .frm file is missing. + @param callback Called with the handlerton of the plugin that we have + found the table in. + @return -1 Table was not found in any engine @return 0 Table was found in some engine and delete succeded @return # Error from first engine that had a table but didn't succeed to @@ -5040,7 +5046,8 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) */ int ha_delete_table_force(THD *thd, const char *path, const LEX_CSTRING *db, - const LEX_CSTRING *alias) + const LEX_CSTRING *alias, + std::function<void(handlerton*)> callback) { st_force_drop_table_params param; Table_exists_error_handler no_such_table_handler; @@ -5051,6 +5058,7 @@ int ha_delete_table_force(THD *thd, const char *path, const LEX_CSTRING *db, param.alias= alias; param.error= -1; // Table not found param.discovering= true; + param.callback= callback; thd->push_internal_handler(&no_such_table_handler); if (plugin_foreach(thd, delete_table_force, MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/sql/handler.h b/sql/handler.h index 4e1e3f0413f..038a6f0e507 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -36,6 +36,8 @@ #include "mdl.h" #include "vers_string.h" +#include <functional> + #include "sql_analyze_stmt.h" // for Exec_time_tracker #include <my_compare.h> @@ -5109,7 +5111,8 @@ int ha_delete_table(THD *thd, handlerton *db_type, const char *path, const LEX_CSTRING *db, const LEX_CSTRING *alias, bool generate_warning); int ha_delete_table_force(THD *thd, const char *path, const LEX_CSTRING *db, - const LEX_CSTRING *alias); + const LEX_CSTRING *alias, + std::function<void(handlerton*)> callback); void ha_prepare_for_backup(); void ha_end_backup(); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b8c51f05f77..2c0c337c830 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2548,7 +2548,14 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, /* Remove extension for delete */ *path_end= '\0'; - ferror= ha_delete_table_force(thd, path, &db, &table_name); + + ferror= ha_delete_table_force(thd, path, &db, &table_name, + [&](handlerton *h) + { + if (h->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) + log_if_exists= 1; + }); + if (!ferror) { /* Table existed and was deleted */