Skip to content

Commit 60f08dd

Browse files
montywimontywi
committed
MDEV-22925 ALTER TABLE s3_table ENGINE=Aria can cause failure on slave
When converting a table (test.s3_table) from S3 to another engine, the following will be logged to the binary log: DROP TABLE IF EXISTS test.t1; CREATE OR REPLACE TABLE test.t1 (...) ENGINE=new_engine INSERT rows to test.t1 in binary-row-log-format The bug is that the above statements are logged one by one to the binary log. This means that a fast slave, configured to use the same S3 storage as the master, would be able to execute the DROP and CREATE from the binary log before the master has finished the ALTER TABLE. In this case the slave would ignore the DROP (as it's on a S3 table) but it will stop on CREATE of the local tale, as the table is still exists in S3. The REPLACE part will be ignored by the slave as it can't touch the S3 table. The fix is to ensure that all the above statements is written to binary log AFTER the table has been deleted from S3.
1 parent 6a0c05b commit 60f08dd

13 files changed

+48
-31
lines changed

‎mysql-test/suite/s3/replication.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
# Tests for S3 replication
66
#
77

8-
connection slave;
8+
sync_slave_with_master;
99
let $SLAVE_DATADIR= `select @@datadir`;
1010
connection master;
1111

‎mysql-test/suite/s3/replication_mixed.result

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,34 +194,30 @@ slave-bin.000001 # Gtid # # GTID #-#-#
194194
slave-bin.000001 # Query # # use `database`; flush tables
195195
slave-bin.000001 # Gtid # # GTID #-#-#
196196
slave-bin.000001 # Query # # use `database`; set @@sql_if_exists=1; alter table t1 add column e int, engine=s3
197-
slave-bin.000001 # Gtid # # GTID #-#-#
197+
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
198198
slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1` /* generated by server */
199-
slave-bin.000001 # Gtid # # GTID #-#-#
200-
slave-bin.000001 # Query # # use `database`; CREATE TABLE `t1` (
199+
slave-bin.000001 # Query # # use `database`; CREATE OR REPLACE TABLE `t1` (
201200
`a` int(11) DEFAULT NULL,
202201
`b` int(11) DEFAULT NULL,
203202
`c` int(11) DEFAULT NULL,
204203
`d` int(11) DEFAULT NULL,
205204
`e` int(11) DEFAULT NULL
206205
) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
207-
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
208206
slave-bin.000001 # Annotate_rows # # alter table t1 engine=aria
209207
slave-bin.000001 # Table_map # # table_id: # (database.t1)
210208
slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
211209
slave-bin.000001 # Query # # COMMIT
212210
slave-bin.000001 # Gtid # # GTID #-#-#
213211
slave-bin.000001 # Query # # use `database`; alter table t1 engine=s3
214-
slave-bin.000001 # Gtid # # GTID #-#-#
212+
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
215213
slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1` /* generated by server */
216-
slave-bin.000001 # Gtid # # GTID #-#-#
217-
slave-bin.000001 # Query # # use `database`; CREATE TABLE `t2` (
214+
slave-bin.000001 # Query # # use `database`; CREATE OR REPLACE TABLE `t2` (
218215
`a` int(11) DEFAULT NULL,
219216
`b` int(11) DEFAULT NULL,
220217
`c` int(11) DEFAULT NULL,
221218
`d` int(11) DEFAULT NULL,
222219
`e` int(11) DEFAULT NULL
223220
) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
224-
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
225221
slave-bin.000001 # Annotate_rows # # alter table t1 rename t2, engine=aria
226222
slave-bin.000001 # Table_map # # table_id: # (database.t2)
227223
slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F

‎mysql-test/suite/s3/replication_partition.result

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,18 +225,16 @@ slave-bin.000001 # Gtid # # GTID #-#-#
225225
slave-bin.000001 # Query # # use `database`; set @@sql_if_exists=1; rename table t2 to t1
226226
slave-bin.000001 # Gtid # # GTID #-#-#
227227
slave-bin.000001 # Query # # use `database`; set @@sql_if_exists=1; alter table t1 drop partition p1
228-
slave-bin.000001 # Gtid # # GTID #-#-#
228+
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
229229
slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1` /* generated by server */
230-
slave-bin.000001 # Gtid # # GTID #-#-#
231-
slave-bin.000001 # Query # # use `database`; CREATE TABLE `t1` (
230+
slave-bin.000001 # Query # # use `database`; CREATE OR REPLACE TABLE `t1` (
232231
`c1` int(11) NOT NULL,
233232
`c2` int(11) DEFAULT NULL,
234233
PRIMARY KEY (`c1`)
235234
) ENGINE=InnoDB DEFAULT CHARSET=latin1
236235
PARTITION BY RANGE (`c1`)
237236
(PARTITION `p2` VALUES LESS THAN (300) ENGINE = InnoDB,
238237
PARTITION `p3` VALUES LESS THAN (400) ENGINE = InnoDB)
239-
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
240238
slave-bin.000001 # Annotate_rows # # alter table t1 engine=innodb
241239
slave-bin.000001 # Table_map # # table_id: # (database.t1)
242240
slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F

‎mysql-test/suite/s3/replication_stmt.result

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,34 +194,30 @@ slave-bin.000001 # Gtid # # GTID #-#-#
194194
slave-bin.000001 # Query # # use `database`; flush tables
195195
slave-bin.000001 # Gtid # # GTID #-#-#
196196
slave-bin.000001 # Query # # use `database`; set @@sql_if_exists=1; alter table t1 add column e int, engine=s3
197-
slave-bin.000001 # Gtid # # GTID #-#-#
197+
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
198198
slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1` /* generated by server */
199-
slave-bin.000001 # Gtid # # GTID #-#-#
200-
slave-bin.000001 # Query # # use `database`; CREATE TABLE `t1` (
199+
slave-bin.000001 # Query # # use `database`; CREATE OR REPLACE TABLE `t1` (
201200
`a` int(11) DEFAULT NULL,
202201
`b` int(11) DEFAULT NULL,
203202
`c` int(11) DEFAULT NULL,
204203
`d` int(11) DEFAULT NULL,
205204
`e` int(11) DEFAULT NULL
206205
) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
207-
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
208206
slave-bin.000001 # Annotate_rows # # alter table t1 engine=aria
209207
slave-bin.000001 # Table_map # # table_id: # (database.t1)
210208
slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
211209
slave-bin.000001 # Query # # COMMIT
212210
slave-bin.000001 # Gtid # # GTID #-#-#
213211
slave-bin.000001 # Query # # use `database`; alter table t1 engine=s3
214-
slave-bin.000001 # Gtid # # GTID #-#-#
212+
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
215213
slave-bin.000001 # Query # # use `database`; DROP TABLE IF EXISTS `t1` /* generated by server */
216-
slave-bin.000001 # Gtid # # GTID #-#-#
217-
slave-bin.000001 # Query # # use `database`; CREATE TABLE `t2` (
214+
slave-bin.000001 # Query # # use `database`; CREATE OR REPLACE TABLE `t2` (
218215
`a` int(11) DEFAULT NULL,
219216
`b` int(11) DEFAULT NULL,
220217
`c` int(11) DEFAULT NULL,
221218
`d` int(11) DEFAULT NULL,
222219
`e` int(11) DEFAULT NULL
223220
) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
224-
slave-bin.000001 # Gtid # # BEGIN GTID #-#-#
225221
slave-bin.000001 # Annotate_rows # # alter table t1 rename t2, engine=aria
226222
slave-bin.000001 # Table_map # # table_id: # (database.t2)
227223
slave-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F

‎sql/handler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,8 @@ given at all. */
627627

628628
/* Create a sequence */
629629
#define HA_CREATE_USED_SEQUENCE (1UL << 25)
630+
/* Tell binlog_show_create_table to print all engine options */
631+
#define HA_CREATE_PRINT_ALL_OPTIONS (1UL << 26)
630632

631633
typedef ulonglong alter_table_operations;
632634
typedef bool Log_func(THD*, TABLE*, bool, const uchar*, const uchar*);

‎sql/log.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6496,12 +6496,15 @@ bool MYSQL_BIN_LOG::write(Log_event *event_info, my_bool *with_annotate)
64966496
(WSREP(thd) && !(thd->variables.option_bits & OPTION_BIN_LOG))))
64976497
DBUG_RETURN(0);
64986498

6499-
if (thd->variables.option_bits & OPTION_GTID_BEGIN)
6499+
if (thd->variables.option_bits &
6500+
(OPTION_GTID_BEGIN | OPTION_BIN_COMMIT_OFF))
65006501
{
65016502
DBUG_PRINT("info", ("OPTION_GTID_BEGIN was set"));
65026503
/* Wait for commit from binary log before we commit */
65036504
direct= 0;
65046505
using_trans= 1;
6506+
/* Set cache_type to ensure we don't get checksums for this event */
6507+
event_info->cache_type= Log_event::EVENT_TRANSACTIONAL_CACHE;
65056508
}
65066509

65076510
if (thd->binlog_evt_union.do_union)

‎sql/log_event.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,7 @@ class Log_event
12321232
*/
12331233
uint16 flags;
12341234

1235-
uint16 cache_type;
1235+
enum_event_cache_type cache_type;
12361236

12371237
/**
12381238
A storage to cache the global system variable's value.

‎sql/log_event_server.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ my_bool Log_event::need_checksum()
744744
{
745745
my_bool ret;
746746
DBUG_ENTER("Log_event::need_checksum");
747+
747748
/*
748749
few callers of Log_event::write
749750
(incl FD::write, FD constructing code on the slave side, Rotate relay log

‎sql/sql_insert.cc

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777
#include "sql_audit.h"
7878
#include "sql_derived.h" // mysql_handle_derived
7979
#include "sql_prepare.h"
80-
#include "rpl_filter.h" // binlog_filter
8180
#include <my_bit.h>
8281

8382
#include "debug_sync.h"
@@ -4796,8 +4795,12 @@ static int binlog_show_create_table(THD *thd, TABLE *table,
47964795
to a not shared table.
47974796
*/
47984797

4799-
bool binlog_create_table(THD *thd, TABLE *table)
4798+
bool binlog_create_table(THD *thd, TABLE *table, bool replace)
48004799
{
4800+
Table_specification_st create_info;
4801+
bool result;
4802+
ulonglong save_option_bits;
4803+
48014804
/* Don't log temporary tables in row format */
48024805
if (thd->variables.binlog_format == BINLOG_FORMAT_ROW &&
48034806
table->s->tmp_table)
@@ -4811,7 +4814,19 @@ bool binlog_create_table(THD *thd, TABLE *table)
48114814
*/
48124815
thd->set_current_stmt_binlog_format_row();
48134816
table->file->prepare_for_row_logging();
4814-
return binlog_show_create_table(thd, table, 0) != 0;
4817+
4818+
create_info.lex_start();
4819+
save_option_bits= thd->variables.option_bits;
4820+
if (replace)
4821+
create_info.set(DDL_options_st::OPT_OR_REPLACE);
4822+
/* Ensure we write ENGINE=xxx and CHARSET=... to binary log */
4823+
create_info.used_fields|= (HA_CREATE_USED_ENGINE |
4824+
HA_CREATE_USED_DEFAULT_CHARSET);
4825+
/* Ensure we write all engine options to binary log */
4826+
create_info.used_fields|= HA_CREATE_PRINT_ALL_OPTIONS;
4827+
result= binlog_show_create_table(thd, table, &create_info) != 0;
4828+
thd->variables.option_bits= save_option_bits;
4829+
return result;
48154830
}
48164831

48174832

‎sql/sql_insert.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ int check_duplic_insert_without_overlaps(THD *thd, TABLE *table,
4343
int write_record(THD *thd, TABLE *table, COPY_INFO *info,
4444
select_result *returning= NULL);
4545
void kill_delayed_threads(void);
46-
bool binlog_create_table(THD *thd, TABLE *table);
46+
bool binlog_create_table(THD *thd, TABLE *table, bool replace);
4747
bool binlog_drop_table(THD *thd, TABLE *table);
4848

4949
#ifdef EMBEDDED_LIBRARY

‎sql/sql_priv.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@
182182
#define SELECT_NO_UNLOCK (1ULL << 41) // SELECT, intern
183183
#define SELECT_NO_UNLOCK (1ULL << 41) // SELECT, intern
184184
#define OPTION_BIN_TMP_LOG_OFF (1ULL << 42) // disable binlog, intern
185+
/* Disable commit of binlog. Used to combine many DDL's and DML's as one */
186+
#define OPTION_BIN_COMMIT_OFF (1ULL << 43)
185187

186188
#define OPTION_LEX_FOUND_COMMENT (1ULL << 0) // intern, parser
187189

‎sql/sql_show.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1812,7 +1812,9 @@ static void add_table_options(THD *thd, TABLE *table,
18121812
handlerton *hton;
18131813
HA_CREATE_INFO create_info;
18141814
bool check_options= (!(sql_mode & MODE_IGNORE_BAD_TABLE_OPTIONS) &&
1815-
!create_info_arg);
1815+
(!create_info_arg ||
1816+
create_info_arg->used_fields &
1817+
HA_CREATE_PRINT_ALL_OPTIONS));
18161818

18171819
#ifdef WITH_PARTITION_STORAGE_ENGINE
18181820
if (table->part_info)

‎sql/sql_table.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10711,7 +10711,7 @@ do_continue:;
1071110711
thd->binlog_table_should_be_logged(&new_table->s->db))
1071210712
{
1071310713
/*
10714-
We new_table is marked as internal temp table, but we want to have
10714+
'new_table' is marked as internal temp table, but we want to have
1071510715
the logging based on the original table type
1071610716
*/
1071710717
bool res;
@@ -10720,9 +10720,11 @@ do_continue:;
1072010720

1072110721
/* Force row logging, even if the table was created as 'temporary' */
1072210722
new_table->s->can_do_row_logging= 1;
10723-
1072410723
thd->binlog_start_trans_and_stmt();
10725-
res= binlog_drop_table(thd, table) || binlog_create_table(thd, new_table);
10724+
thd->variables.option_bits|= OPTION_BIN_COMMIT_OFF;
10725+
res= (binlog_drop_table(thd, table) ||
10726+
binlog_create_table(thd, new_table, 1));
10727+
thd->variables.option_bits&= ~OPTION_BIN_COMMIT_OFF;
1072610728
new_table->s->tmp_table= org_tmp_table;
1072710729
if (res)
1072810730
goto err_new_table_cleanup;

0 commit comments

Comments
 (0)