Skip to content

Commit 2783fc7

Browse files
author
Alexey Botchkov
committed
MDEV-717 LP:1003679 - Wrong binlog order on concurrent DROP schema and CREATE function.
 The cause of the issue is when DROP DATABASE takes metadata lock and is in progress through it's execution, a concurrently running CREATE FUNCTION checks for the existence of database which it succeeds and then it waits on the metadata lock. Once DROP DATABASE writes to BINLOG and finally releases the metadata lock on schema object, the CREATE FUNCTION waiting on metadata lock gets in it's code path and succeeds and writes to binlog.
1 parent e4435b5 commit 2783fc7

File tree

6 files changed

+219
-82
lines changed

6 files changed

+219
-82
lines changed

‎mysql-test/suite/binlog/r/binlog_concurrent_drop_create.result

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
include/master-slave.inc
2+
[connection master]
3+
RESET MASTER;
4+
DROP DATABASE IF EXISTS db_717;
5+
DROP EVENT IF EXISTS test.e_x1;
6+
set @saved_global_binlog_format = @@global.binlog_format;
7+
set @saved_local_binlog_format = @@session.binlog_format;
8+
SET GLOBAL BINLOG_FORMAT = STATEMENT;
9+
SET SESSION BINLOG_FORMAT = STATEMENT;
10+
CREATE DATABASE db_717;
11+
CREATE FUNCTION db_717.f1() RETURNS INT RETURN 1;
12+
DROP DATABASE db_717;
13+
CREATE FUNCTION db_717.f2() RETURNS INT RETURN 1;
14+
CREATE DATABASE db_717;
15+
CREATE EVENT db_717.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
16+
DROP DATABASE db_717;
17+
CREATE EVENT db_717.e_x2 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
18+
CREATE DATABASE db_717;
19+
CREATE EVENT test.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
20+
DROP DATABASE db_717;
21+
ALTER EVENT test.e_x1 RENAME TO db_717.e_x2;
22+
DROP EVENT test.e_x1;
23+
include/show_binlog_events.inc
24+
Log_name Pos Event_type Server_id End_log_pos Info
25+
master-bin.000001 # Gtid # # GTID #-#-#
26+
master-bin.000001 # Query # # DROP DATABASE IF EXISTS db_717
27+
master-bin.000001 # Gtid # # GTID #-#-#
28+
master-bin.000001 # Query # # use `test`; DROP EVENT IF EXISTS test.e_x1
29+
master-bin.000001 # Gtid # # GTID #-#-#
30+
master-bin.000001 # Query # # CREATE DATABASE db_717
31+
master-bin.000001 # Gtid # # GTID #-#-#
32+
master-bin.000001 # Query # # use `test`; CREATE DEFINER=`root`@`localhost` FUNCTION `db_717`.`f1`() RETURNS int(11)
33+
RETURN 1
34+
master-bin.000001 # Gtid # # GTID #-#-#
35+
master-bin.000001 # Query # # DROP DATABASE db_717
36+
master-bin.000001 # Gtid # # GTID #-#-#
37+
master-bin.000001 # Query # # CREATE DATABASE db_717
38+
master-bin.000001 # Gtid # # GTID #-#-#
39+
master-bin.000001 # Query # # use `test`; CREATE DEFINER=`root`@`localhost` EVENT db_717.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int)
40+
master-bin.000001 # Gtid # # GTID #-#-#
41+
master-bin.000001 # Query # # DROP DATABASE db_717
42+
master-bin.000001 # Gtid # # GTID #-#-#
43+
master-bin.000001 # Query # # CREATE DATABASE db_717
44+
master-bin.000001 # Gtid # # GTID #-#-#
45+
master-bin.000001 # Query # # use `test`; CREATE DEFINER=`root`@`localhost` EVENT test.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int)
46+
master-bin.000001 # Gtid # # GTID #-#-#
47+
master-bin.000001 # Query # # DROP DATABASE db_717
48+
master-bin.000001 # Gtid # # GTID #-#-#
49+
master-bin.000001 # Query # # use `test`; DROP EVENT test.e_x1
50+
SET GLOBAL BINLOG_FORMAT = @saved_global_binlog_format;
51+
SET SESSION BINLOG_FORMAT = @saved_local_binlog_format;
52+
include/rpl_end.inc

‎mysql-test/suite/binlog/t/binlog_concurrent_drop_create.test

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# MDEV-717
2+
# DROP DATABASE and CREATE PROCEDURE|FUNCTION|EVENT
3+
# statements can appear in wrong order in the binlog.
4+
#
5+
# Note - the test can be undeterministic.
6+
7+
--source include/master-slave.inc
8+
RESET MASTER;
9+
10+
--disable_warnings
11+
DROP DATABASE IF EXISTS db_717;
12+
DROP EVENT IF EXISTS test.e_x1;
13+
--enable_warnings
14+
15+
set @saved_global_binlog_format = @@global.binlog_format;
16+
set @saved_local_binlog_format = @@session.binlog_format;
17+
SET GLOBAL BINLOG_FORMAT = STATEMENT;
18+
SET SESSION BINLOG_FORMAT = STATEMENT;
19+
20+
# test function creation
21+
CREATE DATABASE db_717;
22+
23+
CREATE FUNCTION db_717.f1() RETURNS INT RETURN 1;
24+
25+
--send
26+
27+
DROP DATABASE db_717;
28+
29+
--connection master1
30+
31+
--error 0,ER_BAD_DB_ERROR
32+
33+
CREATE FUNCTION db_717.f2() RETURNS INT RETURN 1;
34+
35+
--connection master
36+
37+
--reap
38+
39+
# test event creation
40+
CREATE DATABASE db_717;
41+
42+
CREATE EVENT db_717.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
43+
44+
--send
45+
46+
DROP DATABASE db_717;
47+
48+
--connection master1
49+
50+
--error 0,ER_BAD_DB_ERROR
51+
52+
CREATE EVENT db_717.e_x2 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
53+
54+
--connection master
55+
56+
--reap
57+
58+
# test event modification
59+
CREATE DATABASE db_717;
60+
61+
CREATE EVENT test.e_x1 ON SCHEDULE EVERY 1000 HOUR DO CREATE TABLE t1(id int);
62+
63+
--send
64+
65+
DROP DATABASE db_717;
66+
67+
--connection master1
68+
69+
--error 0,ER_BAD_DB_ERROR
70+
71+
ALTER EVENT test.e_x1 RENAME TO db_717.e_x2;
72+
73+
--connection master
74+
75+
--reap
76+
77+
DROP EVENT test.e_x1;
78+
79+
source include/show_binlog_events.inc;
80+
81+
SET GLOBAL BINLOG_FORMAT = @saved_global_binlog_format;
82+
SET SESSION BINLOG_FORMAT = @saved_local_binlog_format;
83+
84+
--source include/rpl_end.inc
85+

‎sql/events.cc

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ Events::create_event(THD *thd, Event_parse_data *parse_data)
333333
if (check_access(thd, EVENT_ACL, parse_data->dbname.str, NULL, NULL, 0, 0))
334334
DBUG_RETURN(TRUE);
335335

336+
if (lock_object_name(thd, MDL_key::EVENT,
337+
parse_data->dbname.str, parse_data->name.str))
338+
DBUG_RETURN(TRUE);
339+
336340
if (check_db_dir_existence(parse_data->dbname.str))
337341
{
338342
my_error(ER_BAD_DB_ERROR, MYF(0), parse_data->dbname.str);
@@ -347,10 +351,6 @@ Events::create_event(THD *thd, Event_parse_data *parse_data)
347351
*/
348352
save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
349353

350-
if (lock_object_name(thd, MDL_key::EVENT,
351-
parse_data->dbname.str, parse_data->name.str))
352-
DBUG_RETURN(TRUE);
353-
354354
if (thd->lex->create_info.or_replace() && event_queue)
355355
event_queue->drop_event(thd, parse_data->dbname, parse_data->name);
356356

@@ -454,6 +454,16 @@ Events::update_event(THD *thd, Event_parse_data *parse_data,
454454

455455
if (check_access(thd, EVENT_ACL, parse_data->dbname.str, NULL, NULL, 0, 0))
456456
DBUG_RETURN(TRUE);
457+
if (lock_object_name(thd, MDL_key::EVENT,
458+
parse_data->dbname.str, parse_data->name.str))
459+
DBUG_RETURN(TRUE);
460+
461+
if (check_db_dir_existence(parse_data->dbname.str))
462+
{
463+
my_error(ER_BAD_DB_ERROR, MYF(0), parse_data->dbname.str);
464+
DBUG_RETURN(TRUE);
465+
}
466+
457467

458468
if (new_dbname) /* It's a rename */
459469
{
@@ -476,6 +486,13 @@ Events::update_event(THD *thd, Event_parse_data *parse_data,
476486
if (check_access(thd, EVENT_ACL, new_dbname->str, NULL, NULL, 0, 0))
477487
DBUG_RETURN(TRUE);
478488

489+
/*
490+
Acquire mdl exclusive lock on target database name.
491+
*/
492+
if (lock_object_name(thd, MDL_key::EVENT,
493+
new_dbname->str, new_name->str))
494+
DBUG_RETURN(TRUE);
495+
479496
/* Check that the target database exists */
480497
if (check_db_dir_existence(new_dbname->str))
481498
{
@@ -490,10 +507,6 @@ Events::update_event(THD *thd, Event_parse_data *parse_data,
490507
*/
491508
save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
492509

493-
if (lock_object_name(thd, MDL_key::EVENT,
494-
parse_data->dbname.str, parse_data->name.str))
495-
DBUG_RETURN(TRUE);
496-
497510
/* On error conditions my_error() is called so no need to handle here */
498511
if (!(ret= db_repository->update_event(thd, parse_data,
499512
new_dbname, new_name)))

‎sql/sp.cc

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434

3535
#include <my_user.h>
3636

37+
/* Used in error handling only */
38+
#define SP_TYPE_STRING(type) \
39+
(type == TYPE_ENUM_FUNCTION ? "FUNCTION" : "PROCEDURE")
40+
3741
static int
3842
db_load_routine(THD *thd, stored_procedure_type type, sp_name *name,
3943
sp_head **sphp,
@@ -1007,15 +1011,16 @@ sp_drop_routine_internal(THD *thd, stored_procedure_type type,
10071011
followed by an implicit grant (sp_grant_privileges())
10081012
and this subsequent call opens and closes mysql.procs_priv.
10091013
1010-
@return Error code. SP_OK is returned on success. Other
1011-
SP_ constants are used to indicate about errors.
1014+
@return Error status.
1015+
@retval FALSE on success
1016+
@retval TRUE on error
10121017
*/
10131018

1014-
int
1019+
bool
10151020
sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10161021
{
10171022
LEX *lex= thd->lex;
1018-
int ret;
1023+
bool ret= TRUE;
10191024
TABLE *table;
10201025
char definer_buf[USER_HOST_BUFF_SIZE];
10211026
LEX_STRING definer;
@@ -1040,7 +1045,22 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10401045

10411046
/* Grab an exclusive MDL lock. */
10421047
if (lock_object_name(thd, mdl_type, sp->m_db.str, sp->m_name.str))
1043-
DBUG_RETURN(SP_OPEN_TABLE_FAILED);
1048+
{
1049+
my_error(ER_BAD_DB_ERROR, MYF(0), sp->m_db.str);
1050+
DBUG_RETURN(TRUE);
1051+
}
1052+
1053+
/*
1054+
Check that a database directory with this name
1055+
exists. Design note: This won't work on virtual databases
1056+
like information_schema.
1057+
*/
1058+
if (check_db_dir_existence(sp->m_db.str))
1059+
{
1060+
my_error(ER_BAD_DB_ERROR, MYF(0), sp->m_db.str);
1061+
DBUG_RETURN(TRUE);
1062+
}
1063+
10441064

10451065
/* Reset sql_mode during data dictionary operations. */
10461066
thd->variables.sql_mode= 0;
@@ -1049,7 +1069,9 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10491069
thd->count_cuted_fields= CHECK_FIELD_WARN;
10501070

10511071
if (!(table= open_proc_table_for_update(thd)))
1052-
ret= SP_OPEN_TABLE_FAILED;
1072+
{
1073+
my_error(ER_SP_STORE_FAILED, MYF(0), SP_TYPE_STRING(type),sp->m_name.str);
1074+
}
10531075
else
10541076
{
10551077
/* Checking if the routine already exists */
@@ -1065,11 +1087,10 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10651087
push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
10661088
ER_SP_ALREADY_EXISTS,
10671089
ER_THD(thd, ER_SP_ALREADY_EXISTS),
1068-
type == TYPE_ENUM_FUNCTION ?
1069-
"FUNCTION" : "PROCEDURE",
1090+
SP_TYPE_STRING(type),
10701091
lex->spname->m_name.str);
10711092

1072-
ret= SP_OK;
1093+
ret= FALSE;
10731094

10741095
// Setting retstr as it is used for logging.
10751096
if (sp->m_type == TYPE_ENUM_FUNCTION)
@@ -1078,7 +1099,8 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10781099
}
10791100
else
10801101
{
1081-
ret= SP_WRITE_ROW_FAILED;
1102+
my_error(ER_SP_ALREADY_EXISTS, MYF(0),
1103+
SP_TYPE_STRING(type), sp->m_name.str);
10821104
goto done;
10831105
}
10841106
}
@@ -1090,7 +1112,8 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10901112

10911113
if (table->s->fields < MYSQL_PROC_FIELD_COUNT)
10921114
{
1093-
ret= SP_GET_FIELD_FAILED;
1115+
my_error(ER_SP_STORE_FAILED, MYF(0),
1116+
SP_TYPE_STRING(type), sp->m_name.str);
10941117
goto done;
10951118
}
10961119

@@ -1099,12 +1122,12 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
10991122
sp->m_name.str+sp->m_name.length) >
11001123
table->field[MYSQL_PROC_FIELD_NAME]->char_length())
11011124
{
1102-
ret= SP_BAD_IDENTIFIER;
1125+
my_error(ER_TOO_LONG_IDENT, MYF(0), sp->m_name.str);
11031126
goto done;
11041127
}
11051128
if (sp->m_body.length > table->field[MYSQL_PROC_FIELD_BODY]->field_length)
11061129
{
1107-
ret= SP_BODY_TOO_LONG;
1130+
my_error(ER_TOO_LONG_BODY, MYF(0), sp->m_name.str);
11081131
goto done;
11091132
}
11101133

@@ -1193,17 +1216,13 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
11931216
if (access == SP_CONTAINS_SQL ||
11941217
access == SP_MODIFIES_SQL_DATA)
11951218
{
1196-
my_message(ER_BINLOG_UNSAFE_ROUTINE,
1197-
ER_THD(thd, ER_BINLOG_UNSAFE_ROUTINE), MYF(0));
1198-
ret= SP_INTERNAL_ERROR;
1219+
my_error(ER_BINLOG_UNSAFE_ROUTINE, MYF(0));
11991220
goto done;
12001221
}
12011222
}
12021223
if (!(thd->security_ctx->master_access & SUPER_ACL))
12031224
{
1204-
my_message(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER,
1205-
ER_THD(thd, ER_BINLOG_CREATE_ROUTINE_NEED_SUPER), MYF(0));
1206-
ret= SP_INTERNAL_ERROR;
1225+
my_error(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER,MYF(0));
12071226
goto done;
12081227
}
12091228
}
@@ -1234,22 +1253,24 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
12341253

12351254
if (store_failed)
12361255
{
1237-
ret= SP_FLD_STORE_FAILED;
1256+
my_error(ER_CANT_CREATE_SROUTINE, MYF(0), sp->m_name.str);
12381257
goto done;
12391258
}
12401259

1241-
ret= SP_OK;
12421260
if (table->file->ha_write_row(table->record[0]))
1243-
ret= SP_WRITE_ROW_FAILED;
1261+
{
1262+
my_error(ER_SP_ALREADY_EXISTS, MYF(0),
1263+
SP_TYPE_STRING(type), sp->m_name.str);
1264+
goto done;
1265+
}
12441266
/* Make change permanent and avoid 'table is marked as crashed' errors */
12451267
table->file->extra(HA_EXTRA_FLUSH);
12461268

1247-
if (ret == SP_OK)
1248-
sp_cache_invalidate();
1269+
sp_cache_invalidate();
12491270
}
12501271

12511272
log:
1252-
if (ret == SP_OK && mysql_bin_log.is_open())
1273+
if (mysql_bin_log.is_open())
12531274
{
12541275
thd->clear_error();
12551276

@@ -1268,7 +1289,7 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
12681289
&(thd->lex->definer->host),
12691290
saved_mode))
12701291
{
1271-
ret= SP_INTERNAL_ERROR;
1292+
my_error(ER_OUT_OF_RESOURCES, MYF(0));
12721293
goto done;
12731294
}
12741295
/* restore sql_mode when binloging */
@@ -1277,9 +1298,13 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
12771298
if (thd->binlog_query(THD::STMT_QUERY_TYPE,
12781299
log_query.ptr(), log_query.length(),
12791300
FALSE, FALSE, FALSE, 0))
1280-
ret= SP_INTERNAL_ERROR;
1301+
{
1302+
my_error(ER_ERROR_ON_WRITE, MYF(MY_WME), "binary log", -1);
1303+
goto done;
1304+
}
12811305
thd->variables.sql_mode= 0;
12821306
}
1307+
ret= FALSE;
12831308

12841309
done:
12851310
thd->count_cuted_fields= saved_count_cuted_fields;

0 commit comments

Comments
 (0)