Skip to content

Commit 00164ea

Browse files
committed
Merge branch 'gtid_table_garbage_rows_10.3' into 10.3
2 parents 2fd7706 + 3eb2c46 commit 00164ea

File tree

7 files changed

+167
-21
lines changed

7 files changed

+167
-21
lines changed

mysql-test/suite/rpl/r/rpl_parallel_optimistic.result

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,4 +648,12 @@ SET GLOBAL slave_parallel_threads=@old_parallel_threads;
648648
include/start_slave.inc
649649
connection server_1;
650650
DROP TABLE t1, t2, t3;
651+
include/save_master_gtid.inc
652+
connection server_2;
653+
include/sync_with_master_gtid.inc
654+
Check that no more than the expected last four GTIDs are in mysql.gtid_slave_pos
655+
select count(4) <= 4 from mysql.gtid_slave_pos order by domain_id, sub_id;
656+
count(4) <= 4
657+
1
658+
connection server_1;
651659
include/rpl_end.inc

mysql-test/suite/rpl/t/rpl_parallel_optimistic.test

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,5 +553,29 @@ SET GLOBAL slave_parallel_threads=@old_parallel_threads;
553553

554554
--connection server_1
555555
DROP TABLE t1, t2, t3;
556+
--source include/save_master_gtid.inc
556557

558+
--connection server_2
559+
--source include/sync_with_master_gtid.inc
560+
# Check for left-over rows in table mysql.gtid_slave_pos (MDEV-12147).
561+
#
562+
# There was a bug when a transaction got a conflict and was rolled back. It
563+
# might have also handled deletion of some old rows, and these deletions would
564+
# then also be rolled back. And since the deletes were never re-tried, old no
565+
# longer needed rows would accumulate in the table without limit.
566+
#
567+
# The earlier part of this test file have plenty of transactions being rolled
568+
# back. But the last DROP TABLE statement runs on its own and should never
569+
# conflict, thus at this point the mysql.gtid_slave_pos table should be clean.
570+
#
571+
# To support @@gtid_pos_auto_engines, when a row is inserted in the table, it
572+
# is associated with the engine of the table at insertion time, and it will
573+
# only be deleted during record_gtid from a table of the same engine. Since we
574+
# alter the table from MyISAM to InnoDB at the start of this test, we should
575+
# end up with 4 rows: two left-over from when the table was MyISAM, and two
576+
# left-over from the InnoDB part.
577+
--echo Check that no more than the expected last four GTIDs are in mysql.gtid_slave_pos
578+
select count(4) <= 4 from mysql.gtid_slave_pos order by domain_id, sub_id;
579+
580+
--connection server_1
557581
--source include/rpl_end.inc

sql/log_event.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5565,7 +5565,7 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi,
55655565
gtid= rgi->current_gtid;
55665566
if (unlikely(rpl_global_gtid_slave_state->record_gtid(thd, &gtid,
55675567
sub_id,
5568-
true, false,
5568+
rgi, false,
55695569
&hton)))
55705570
{
55715571
int errcode= thd->get_stmt_da()->sql_errno();
@@ -8362,7 +8362,7 @@ Gtid_list_log_event::do_apply_event(rpl_group_info *rgi)
83628362
{
83638363
if ((ret= rpl_global_gtid_slave_state->record_gtid(thd, &list[i],
83648364
sub_id_list[i],
8365-
false, false, &hton)))
8365+
NULL, false, &hton)))
83668366
return ret;
83678367
rpl_global_gtid_slave_state->update_state_hash(sub_id_list[i], &list[i],
83688368
hton, NULL);
@@ -8899,7 +8899,7 @@ int Xid_log_event::do_apply_event(rpl_group_info *rgi)
88998899
rgi->gtid_pending= false;
89008900

89018901
gtid= rgi->current_gtid;
8902-
err= rpl_global_gtid_slave_state->record_gtid(thd, &gtid, sub_id, true,
8902+
err= rpl_global_gtid_slave_state->record_gtid(thd, &gtid, sub_id, rgi,
89038903
false, &hton);
89048904
if (unlikely(err))
89058905
{

sql/rpl_gtid.cc

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ rpl_slave_state::record_and_update_gtid(THD *thd, rpl_group_info *rgi)
7979
rgi->gtid_pending= false;
8080
if (rgi->gtid_ignore_duplicate_state!=rpl_group_info::GTID_DUPLICATE_IGNORE)
8181
{
82-
if (record_gtid(thd, &rgi->current_gtid, sub_id, false, false, &hton))
82+
if (record_gtid(thd, &rgi->current_gtid, sub_id, NULL, false, &hton))
8383
DBUG_RETURN(1);
8484
update_state_hash(sub_id, &rgi->current_gtid, hton, rgi);
8585
}
@@ -331,6 +331,8 @@ rpl_slave_state::update(uint32 domain_id, uint32 server_id, uint64 sub_id,
331331
}
332332
}
333333
rgi->gtid_ignore_duplicate_state= rpl_group_info::GTID_DUPLICATE_NULL;
334+
335+
rgi->pending_gtid_deletes_clear();
334336
}
335337

336338
if (!(list_elem= (list_element *)my_malloc(sizeof(*list_elem), MYF(MY_WME))))
@@ -381,15 +383,24 @@ int
381383
rpl_slave_state::put_back_list(uint32 domain_id, list_element *list)
382384
{
383385
element *e;
386+
int err= 0;
387+
388+
mysql_mutex_lock(&LOCK_slave_state);
384389
if (!(e= (element *)my_hash_search(&hash, (const uchar *)&domain_id, 0)))
385-
return 1;
390+
{
391+
err= 1;
392+
goto end;
393+
}
386394
while (list)
387395
{
388396
list_element *next= list->next;
389397
e->add(list);
390398
list= next;
391399
}
392-
return 0;
400+
401+
end:
402+
mysql_mutex_unlock(&LOCK_slave_state);
403+
return err;
393404
}
394405

395406

@@ -559,20 +570,20 @@ rpl_slave_state::select_gtid_pos_table(THD *thd, LEX_CSTRING *out_tablename)
559570
/*
560571
Write a gtid to the replication slave state table.
561572
562-
Do it as part of the transaction, to get slave crash safety, or as a separate
563-
transaction if !in_transaction (eg. MyISAM or DDL).
564-
565573
gtid The global transaction id for this event group.
566574
sub_id Value allocated within the sub_id when the event group was
567575
read (sub_id must be consistent with commit order in master binlog).
576+
rgi rpl_group_info context, if we are recording the gtid transactionally
577+
as part of replicating a transactional event. NULL if called from
578+
outside of a replicated transaction.
568579
569580
Note that caller must later ensure that the new gtid and sub_id is inserted
570581
into the appropriate HASH element with rpl_slave_state.add(), so that it can
571582
be deleted later. But this must only be done after COMMIT if in transaction.
572583
*/
573584
int
574585
rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
575-
bool in_transaction, bool in_statement,
586+
rpl_group_info *rgi, bool in_statement,
576587
void **out_hton)
577588
{
578589
TABLE_LIST tlist;
@@ -671,7 +682,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
671682
thd->wsrep_ignore_table= true;
672683
#endif
673684

674-
if (!in_transaction)
685+
if (!rgi)
675686
{
676687
DBUG_PRINT("info", ("resetting OPTION_BEGIN"));
677688
thd->variables.option_bits&=
@@ -776,7 +787,8 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
776787
table->file->print_error(err, MYF(0));
777788
goto end;
778789
}
779-
while (delete_list)
790+
cur = delete_list;
791+
while (cur)
780792
{
781793
uchar key_buffer[4+8];
782794

@@ -786,9 +798,9 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
786798
/* `break' does not work inside DBUG_EXECUTE_IF */
787799
goto dbug_break; });
788800

789-
next= delete_list->next;
801+
next= cur->next;
790802

791-
table->field[1]->store(delete_list->sub_id, true);
803+
table->field[1]->store(cur->sub_id, true);
792804
/* domain_id is already set in table->record[0] from write_row() above. */
793805
key_copy(key_buffer, table->record[0], &table->key_info[0], 0, false);
794806
if (table->file->ha_index_read_map(table->record[1], key_buffer,
@@ -802,8 +814,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
802814
not want to endlessly error on the same element in case of table
803815
corruption or such.
804816
*/
805-
my_free(delete_list);
806-
delete_list= next;
817+
cur= next;
807818
if (err)
808819
break;
809820
}
@@ -826,18 +837,31 @@ IF_DBUG(dbug_break:, )
826837
*/
827838
if (delete_list)
828839
{
829-
mysql_mutex_lock(&LOCK_slave_state);
830840
put_back_list(gtid->domain_id, delete_list);
831-
mysql_mutex_unlock(&LOCK_slave_state);
841+
delete_list = 0;
832842
}
833843

834844
ha_rollback_trans(thd, FALSE);
835845
}
836846
close_thread_tables(thd);
837-
if (in_transaction)
847+
if (rgi)
848+
{
838849
thd->mdl_context.release_statement_locks();
850+
/*
851+
Save the list of old gtid entries we deleted. If this transaction
852+
fails later for some reason and is rolled back, the deletion of those
853+
entries will be rolled back as well, and we will need to put them back
854+
on the to-be-deleted list so we can re-do the deletion. Otherwise
855+
redundant rows in mysql.gtid_slave_pos may accumulate if transactions
856+
are rolled back and retried after record_gtid().
857+
*/
858+
rgi->pending_gtid_deletes_save(gtid->domain_id, delete_list);
859+
}
839860
else
861+
{
840862
thd->mdl_context.release_transactional_locks();
863+
rpl_group_info::pending_gtid_deletes_free(delete_list);
864+
}
841865
}
842866
thd->lex->restore_backup_query_tables_list(&lex_backup);
843867
thd->variables.option_bits= thd_saved_option;
@@ -1221,7 +1245,7 @@ rpl_slave_state::load(THD *thd, const char *state_from_master, size_t len,
12211245

12221246
if (gtid_parser_helper(&state_from_master, end, &gtid) ||
12231247
!(sub_id= next_sub_id(gtid.domain_id)) ||
1224-
record_gtid(thd, &gtid, sub_id, false, in_statement, &hton) ||
1248+
record_gtid(thd, &gtid, sub_id, NULL, in_statement, &hton) ||
12251249
update(gtid.domain_id, gtid.server_id, sub_id, gtid.seq_no, hton, NULL))
12261250
return 1;
12271251
if (state_from_master == end)

sql/rpl_gtid.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ struct rpl_slave_state
233233
int truncate_state_table(THD *thd);
234234
void select_gtid_pos_table(THD *thd, LEX_CSTRING *out_tablename);
235235
int record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
236-
bool in_transaction, bool in_statement, void **out_hton);
236+
rpl_group_info *rgi, bool in_statement, void **out_hton);
237237
uint64 next_sub_id(uint32 domain_id);
238238
int iterate(int (*cb)(rpl_gtid *, void *), void *data,
239239
rpl_gtid *extra_gtids, uint32 num_extra,

sql/rpl_rli.cc

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,6 +2086,7 @@ rpl_group_info::reinit(Relay_log_info *rli)
20862086
long_find_row_note_printed= false;
20872087
did_mark_start_commit= false;
20882088
gtid_ev_flags2= 0;
2089+
pending_gtid_delete_list= NULL;
20892090
last_master_timestamp = 0;
20902091
gtid_ignore_duplicate_state= GTID_DUPLICATE_NULL;
20912092
speculation= SPECULATE_NO;
@@ -2216,6 +2217,12 @@ void rpl_group_info::cleanup_context(THD *thd, bool error)
22162217
erroneously update the GTID position.
22172218
*/
22182219
gtid_pending= false;
2220+
2221+
/*
2222+
Rollback will have undone any deletions of old rows we might have made
2223+
in mysql.gtid_slave_pos. Put those rows back on the list to be deleted.
2224+
*/
2225+
pending_gtid_deletes_put_back();
22192226
}
22202227
m_table_map.clear_tables();
22212228
slave_close_thread_tables(thd);
@@ -2441,6 +2448,78 @@ rpl_group_info::unmark_start_commit()
24412448
}
24422449

24432450

2451+
/*
2452+
When record_gtid() has deleted any old rows from the table
2453+
mysql.gtid_slave_pos as part of a replicated transaction, save the list of
2454+
rows deleted here.
2455+
2456+
If later the transaction fails (eg. optimistic parallel replication), the
2457+
deletes will be undone when the transaction is rolled back. Then we can
2458+
put back the list of rows into the rpl_global_gtid_slave_state, so that
2459+
we can re-do the deletes and avoid accumulating old rows in the table.
2460+
*/
2461+
void
2462+
rpl_group_info::pending_gtid_deletes_save(uint32 domain_id,
2463+
rpl_slave_state::list_element *list)
2464+
{
2465+
/*
2466+
We should never get to a state where we try to save a new pending list of
2467+
gtid deletes while we still have an old one. But make sure we handle it
2468+
anyway just in case, so we avoid leaving stray entries in the
2469+
mysql.gtid_slave_pos table.
2470+
*/
2471+
DBUG_ASSERT(!pending_gtid_delete_list);
2472+
if (unlikely(pending_gtid_delete_list))
2473+
pending_gtid_deletes_put_back();
2474+
2475+
pending_gtid_delete_list= list;
2476+
pending_gtid_delete_list_domain= domain_id;
2477+
}
2478+
2479+
2480+
/*
2481+
Take the list recorded by pending_gtid_deletes_save() and put it back into
2482+
rpl_global_gtid_slave_state. This is needed if deletion of the rows was
2483+
rolled back due to transaction failure.
2484+
*/
2485+
void
2486+
rpl_group_info::pending_gtid_deletes_put_back()
2487+
{
2488+
if (pending_gtid_delete_list)
2489+
{
2490+
rpl_global_gtid_slave_state->put_back_list(pending_gtid_delete_list_domain,
2491+
pending_gtid_delete_list);
2492+
pending_gtid_delete_list= NULL;
2493+
}
2494+
}
2495+
2496+
2497+
/*
2498+
Free the list recorded by pending_gtid_deletes_save(). Done when the deletes
2499+
in the list have been permanently committed.
2500+
*/
2501+
void
2502+
rpl_group_info::pending_gtid_deletes_clear()
2503+
{
2504+
pending_gtid_deletes_free(pending_gtid_delete_list);
2505+
pending_gtid_delete_list= NULL;
2506+
}
2507+
2508+
2509+
void
2510+
rpl_group_info::pending_gtid_deletes_free(rpl_slave_state::list_element *list)
2511+
{
2512+
rpl_slave_state::list_element *next;
2513+
2514+
while (list)
2515+
{
2516+
next= list->next;
2517+
my_free(list);
2518+
list= next;
2519+
}
2520+
}
2521+
2522+
24442523
rpl_sql_thread_info::rpl_sql_thread_info(Rpl_filter *filter)
24452524
: rpl_filter(filter)
24462525
{

sql/rpl_rli.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,11 @@ struct rpl_group_info
757757
/* Needs room for "Gtid D-S-N\x00". */
758758
char gtid_info_buf[5+10+1+10+1+20+1];
759759

760+
/* List of not yet committed deletions in mysql.gtid_slave_pos. */
761+
rpl_slave_state::list_element *pending_gtid_delete_list;
762+
/* Domain associated with pending_gtid_delete_list. */
763+
uint32 pending_gtid_delete_list_domain;
764+
760765
/*
761766
The timestamp, from the master, of the commit event.
762767
Used to do delayed update of rli->last_master_timestamp, for getting
@@ -898,6 +903,12 @@ struct rpl_group_info
898903
char *gtid_info();
899904
void unmark_start_commit();
900905

906+
static void pending_gtid_deletes_free(rpl_slave_state::list_element *list);
907+
void pending_gtid_deletes_save(uint32 domain_id,
908+
rpl_slave_state::list_element *list);
909+
void pending_gtid_deletes_put_back();
910+
void pending_gtid_deletes_clear();
911+
901912
longlong get_row_stmt_start_timestamp()
902913
{
903914
return row_stmt_start_timestamp;

0 commit comments

Comments
 (0)