Skip to content

Commit a4fa28b

Browse files
nacarvalhonacarvalho
committed
BUG#16223835: UNEXPECTED EMPTY RESULTS OUT OF SHOW COMMANDS AFTER DUP GTID_NEXT TRANSACTION
When a transaction was skipped due to its GTID was already logged, until GTID_NEXT was set to a different GTID, all the following executed transactions were incorrectly skipped. To avoid this incorrect behaviour, all executed transactions and skipped transactions on AUTOCOMMIT=0 mode or which implicit commit are marked as undefined (GTID_NEXT.type = UNDEFINED_GROUP) when them commit or rollback. So when a second transaction is executed for the same SET @@SESSION.GTID_NEXT command a error is thrown.
1 parent c5f1729 commit a4fa28b

8 files changed

Lines changed: 173 additions & 59 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,13 @@ SET @@SESSION.GTID_NEXT = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:13';
187187
SET @@SESSION.GTID_NEXT = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:14';
188188
ERROR HY000: @@SESSION.GTID_NEXT cannot be changed by a client that owns a GTID. The client owns aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:13. Ownership is released on COMMIT or ROLLBACK.
189189
SET @@SESSION.GTID_NEXT = 'AUTOMATIC';
190+
ROLLBACK;
191+
# can't set while owning a GTID
192+
SET @@SESSION.GTID_NEXT = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:13';
193+
SET @@SESSION.GTID_NEXT = 'AUTOMATIC';
190194
ERROR HY000: @@SESSION.GTID_NEXT cannot be changed by a client that owns a GTID. The client owns aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:13. Ownership is released on COMMIT or ROLLBACK.
191195
ROLLBACK;
196+
SET @@SESSION.GTID_NEXT = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:14';
192197
# can't do implicit commit while gtid_next=SID:GNO
193198
BEGIN;
194199
CREATE TABLE t2 (a INT);

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,17 @@ ROLLBACK;
219219
eval SET @@SESSION.GTID_NEXT = '$ida:13';
220220
--error ER_CANT_SET_GTID_NEXT_WHEN_OWNING_GTID
221221
eval SET @@SESSION.GTID_NEXT = '$ida:14';
222+
# the above error will clean owned_gtid
223+
eval SET @@SESSION.GTID_NEXT = 'AUTOMATIC';
224+
ROLLBACK;
225+
226+
--echo # can't set while owning a GTID
227+
eval SET @@SESSION.GTID_NEXT = '$ida:13';
222228
--error ER_CANT_SET_GTID_NEXT_WHEN_OWNING_GTID
223229
eval SET @@SESSION.GTID_NEXT = 'AUTOMATIC';
224230
ROLLBACK;
225231

232+
eval SET @@SESSION.GTID_NEXT = '$ida:14';
226233
--echo # can't do implicit commit while gtid_next=SID:GNO
227234
BEGIN;
228235
--error ER_CANT_DO_IMPLICIT_COMMIT_IN_TRX_WHEN_GTID_NEXT_IS_SET

‎sql/binlog.cc‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,15 +1457,15 @@ int MYSQL_BIN_LOG::rollback(THD *thd, bool all)
14571457
rollback.
14581458
*/
14591459
if (thd->lex->sql_command != SQLCOM_ROLLBACK_TO_SAVEPOINT)
1460-
if (int error= ha_rollback_low(thd, all))
1461-
DBUG_RETURN(error);
1460+
if ((error= ha_rollback_low(thd, all)))
1461+
goto end;
14621462

14631463
/*
14641464
If there is no cache manager, or if there is nothing in the
14651465
caches, there are no caches to roll back, so we're trivially done.
14661466
*/
14671467
if (cache_mngr == NULL || cache_mngr->is_binlog_empty())
1468-
DBUG_RETURN(0);
1468+
goto end;
14691469

14701470
DBUG_PRINT("debug",
14711471
("all.cannot_safely_rollback(): %s, trx_cache_empty: %s",
@@ -1487,8 +1487,8 @@ int MYSQL_BIN_LOG::rollback(THD *thd, bool all)
14871487
}
14881488
else if (!cache_mngr->stmt_cache.is_binlog_empty())
14891489
{
1490-
if (int error= cache_mngr->stmt_cache.finalize(thd))
1491-
DBUG_RETURN(error);
1490+
if ((error= cache_mngr->stmt_cache.finalize(thd)))
1491+
goto end;
14921492
stuff_logged= true;
14931493
}
14941494

@@ -1577,9 +1577,16 @@ int MYSQL_BIN_LOG::rollback(THD *thd, bool all)
15771577
a cache and need to be rolled back.
15781578
*/
15791579
error |= cache_mngr->trx_cache.truncate(thd, all);
1580-
DBUG_RETURN(error);
15811580
}
15821581

1582+
end:
1583+
/*
1584+
When a statement errors out on auto-commit mode it is rollback
1585+
implicitly, so the same should happen to its GTID.
1586+
*/
1587+
if (!thd->in_active_multi_stmt_transaction())
1588+
gtid_rollback(thd);
1589+
15831590
DBUG_PRINT("return", ("error: %d", error));
15841591
DBUG_RETURN(error);
15851592
}
@@ -5635,7 +5642,6 @@ bool MYSQL_BIN_LOG::write_cache(THD *thd, binlog_cache_data *cache_data)
56355642
goto err;
56365643
}
56375644

5638-
thd->variables.gtid_next.set_undefined();
56395645
global_sid_lock->rdlock();
56405646
if (gtid_state->update_on_flush(thd) != RETURN_STATUS_OK)
56415647
{
@@ -6559,7 +6565,6 @@ MYSQL_BIN_LOG::finish_commit(THD *thd)
65596565
else if (thd->transaction.flags.xid_written)
65606566
dec_prep_xids(thd);
65616567

6562-
thd->variables.gtid_next.set_undefined();
65636568
/*
65646569
Remove committed GTID from owned_gtids, it was already logged on
65656570
MYSQL_BIN_LOG::write_cache().

‎sql/rpl_gtid.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or
44
modify it under the terms of the GNU General Public License as
@@ -2753,6 +2753,14 @@ gtid_before_statement(THD *thd, Group_cache *gsc, Group_cache *gtc);
27532753
*/
27542754
enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd);
27552755

2756+
/**
2757+
Check if the current statement terminates a transaction, and if so
2758+
set GTID_NEXT.type to UNDEFINED_GROUP.
2759+
2760+
@param thd THD object for the session.
2761+
*/
2762+
void gtid_post_statement_checks(THD *thd);
2763+
27562764
/**
27572765
When a transaction is rolled back, this function releases ownership
27582766
of any GTIDs that the transaction owns.

‎sql/rpl_gtid_execution.cc‎

Lines changed: 119 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,83 @@ int gtid_acquire_ownership_multiple(THD *thd)
217217
#endif
218218

219219

220+
/**
221+
Check if current transaction should be skipped, that is, if GTID_NEXT
222+
was already logged.
223+
224+
@param thd The calling thread.
225+
226+
@retval true Transaction was already logged.
227+
@retval false Transaction must be executed.
228+
*/
229+
static inline bool is_already_logged_transaction(const THD *thd)
230+
{
231+
DBUG_ENTER("is_already_logged_transaction");
232+
233+
const Gtid_specification *gtid_next= &thd->variables.gtid_next;
234+
const Gtid_set *gtid_next_list= thd->get_gtid_next_list_const();
235+
236+
if (gtid_next_list == NULL)
237+
{
238+
if (gtid_next->type == GTID_GROUP)
239+
{
240+
if (thd->owned_gtid.sidno == 0)
241+
DBUG_RETURN(true);
242+
else
243+
DBUG_ASSERT(thd->owned_gtid.equals(gtid_next->gtid));
244+
}
245+
else
246+
DBUG_ASSERT(thd->owned_gtid.sidno == 0);
247+
}
248+
else
249+
{
250+
#ifdef HAVE_GTID_NEXT_LIST
251+
if (gtid_next->type == GTID_GROUP)
252+
{
253+
DBUG_ASSERT(gtid_next_list->contains_gtid(gtid_next->gtid));
254+
if (!thd->owned_gtid_set.contains_gtid(gtid_next->gtid))
255+
DBUG_RETURN(true);
256+
}
257+
#else
258+
DBUG_ASSERT(0);/*NOTREACHED*/
259+
#endif
260+
}
261+
262+
DBUG_RETURN(false);
263+
}
264+
265+
266+
/**
267+
Debug code executed when a transaction is skipped.
268+
269+
@param thd The calling thread.
270+
271+
@retval GTID_STATEMENT_SKIP Indicate that statement should be
272+
skipped by caller.
273+
*/
274+
static inline enum_gtid_statement_status skip_statement(const THD *thd)
275+
{
276+
DBUG_ENTER("skip_statement");
277+
278+
DBUG_PRINT("info", ("skipping statement '%s'. "
279+
"gtid_next->type=%d sql_command=%d "
280+
"thd->thread_id=%lu",
281+
thd->query(),
282+
thd->variables.gtid_next.type,
283+
thd->lex->sql_command,
284+
thd->thread_id));
285+
286+
#ifndef DBUG_OFF
287+
const Gtid_set* logged_gtids= gtid_state->get_logged_gtids();
288+
global_sid_lock->rdlock();
289+
DBUG_ASSERT(logged_gtids->contains_gtid(thd->variables.gtid_next.gtid));
290+
global_sid_lock->unlock();
291+
#endif
292+
293+
DBUG_RETURN(GTID_STATEMENT_SKIP);
294+
}
295+
296+
220297
enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd)
221298
{
222299
DBUG_ENTER("gtid_pre_statement_checks");
@@ -249,7 +326,7 @@ enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd)
249326
if (sql_command == SQLCOM_COMMIT || sql_command == SQLCOM_BEGIN ||
250327
sql_command == SQLCOM_ROLLBACK ||
251328
((sql_command == SQLCOM_SELECT ||
252-
sql_command == SQLCOM_SET_OPTION) &&
329+
(sql_command == SQLCOM_SET_OPTION && !thd->lex->is_set_password_sql)) &&
253330
!thd->lex->uses_stored_routines()))
254331
DBUG_RETURN(GTID_STATEMENT_EXECUTE);
255332

@@ -294,33 +371,11 @@ enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd)
294371
thd->owned_gtid.gno,
295372
(ulong)thd->thread_id));
296373

374+
const bool skip_transaction= is_already_logged_transaction(thd);
297375
if (gtid_next_list == NULL)
298376
{
299-
if (gtid_next->type == GTID_GROUP)
300-
{
301-
if (thd->owned_gtid.sidno == 0)
302-
{
303-
/// @todo assert that statement is logged and not owned /sven
304-
DBUG_PRINT("info", ("skipping statement '%s'. "
305-
"gtid_next->type=%d sql_command=%d "
306-
"thd->thread_id=%lu",
307-
thd->query(),
308-
gtid_next->type, sql_command,
309-
thd->thread_id));
310-
/*
311-
@todo: tests fail when this is enabled. figure out why /sven
312-
char buf[Gtid::MAX_TEXT_LENGTH + 1];
313-
gtid_next->to_string(&global_sid_map, buf);
314-
push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
315-
ER_SKIPPING_LOGGED_TRANSACTION,
316-
ER(ER_SKIPPING_LOGGED_TRANSACTION), buf);
317-
*/
318-
DBUG_RETURN(GTID_STATEMENT_SKIP);
319-
}
320-
DBUG_ASSERT(thd->owned_gtid.equals(gtid_next->gtid));
321-
}
322-
else
323-
DBUG_ASSERT(thd->owned_gtid.sidno == 0);
377+
if (skip_transaction)
378+
DBUG_RETURN(skip_statement(thd));
324379
DBUG_RETURN(GTID_STATEMENT_EXECUTE);
325380
}
326381
else
@@ -333,26 +388,8 @@ enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd)
333388
MYF(0));
334389
DBUG_RETURN(GTID_STATEMENT_CANCEL);
335390
case GTID_GROUP:
336-
DBUG_ASSERT(gtid_next_list->contains_gtid(gtid_next->gtid));
337-
if (!thd->owned_gtid_set.contains_gtid(gtid_next->gtid))
338-
{
339-
/*
340-
@todo: tests fail when this is enabled. figure out why /sven
341-
char buf[Gtid::MAX_TEXT_LENGTH + 1];
342-
gtid_next->to_string(&global_sid_map, buf);
343-
push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
344-
ER_SKIPPING_LOGGED_TRANSACTION,
345-
ER(ER_SKIPPING_LOGGED_TRANSACTION), buf);
346-
*/
347-
/// @todo assert that statement is logged and not owned /sven
348-
DBUG_PRINT("info", ("skipping statement '%s'. "
349-
"gtid_next->type=%d sql_command=%d "
350-
"thd->thread_id=%lu",
351-
thd->query(),
352-
gtid_next->type, sql_command,
353-
thd->thread_id));
354-
DBUG_RETURN(GTID_STATEMENT_SKIP);
355-
}
391+
if (skip_transaction)
392+
DBUG_RETURN(skip_statement(thd));
356393
/*FALLTHROUGH*/
357394
case ANONYMOUS_GROUP:
358395
DBUG_RETURN(GTID_STATEMENT_EXECUTE);
@@ -368,6 +405,42 @@ enum_gtid_statement_status gtid_pre_statement_checks(const THD *thd)
368405
}
369406

370407

408+
void gtid_post_statement_checks(THD *thd)
409+
{
410+
DBUG_ENTER("gtid_post_statement_checks");
411+
const enum_sql_command sql_command= thd->lex->sql_command;
412+
413+
/*
414+
If transaction is terminated we set GTID_NEXT type to
415+
UNDEFINED_GROUP, to prevent that the same GTID is used for another
416+
transaction (same GTID here means that user only set
417+
GTID_NEXT= GTID_GROUP once for two transactions).
418+
419+
If the current statement:
420+
implict commits
421+
OR
422+
is SQLCOM_SET_OPTION AND is SET PASSWORD
423+
OR
424+
is commit
425+
OR
426+
is rollback
427+
that means the transaction is terminated and we set GTID_NEXT type
428+
to UNDEFINED_GROUP.
429+
430+
SET AUTOCOMMIT=1 statement is handled on Gtid_state::update_on_flush().
431+
*/
432+
if (thd->variables.gtid_next.type == GTID_GROUP &&
433+
(stmt_causes_implicit_commit(thd, CF_IMPLICIT_COMMIT_BEGIN) ||
434+
(thd->lex->sql_command == SQLCOM_SET_OPTION &&
435+
thd->lex->is_change_password) ||
436+
sql_command == SQLCOM_COMMIT ||
437+
sql_command == SQLCOM_ROLLBACK))
438+
thd->variables.gtid_next.set_undefined();
439+
440+
DBUG_VOID_RETURN;
441+
}
442+
443+
371444
int gtid_rollback(THD *thd)
372445
{
373446
DBUG_ENTER("gtid_rollback");
@@ -376,7 +449,5 @@ int gtid_rollback(THD *thd)
376449
gtid_state->update_on_rollback(thd);
377450
global_sid_lock->unlock();
378451

379-
thd->variables.gtid_next.set_undefined();
380-
381452
DBUG_RETURN(0);
382453
}

‎sql/rpl_gtid_state.cc‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or
44
modify it under the terms of the GNU General Public License as
@@ -159,6 +159,15 @@ enum_return_status Gtid_state::update_on_flush(THD *thd)
159159
ret= logged_gtids._add_gtid(thd->owned_gtid);
160160
}
161161

162+
/*
163+
There may be commands that cause implicit commits, e.g.
164+
SET AUTOCOMMIT=1 may cause the previous statements to commit
165+
without executing a COMMIT command or be on auto-commit mode.
166+
Although we set GTID_NEXT type to UNDEFINED on
167+
Gtid_state::update_on_commit(), we also set it here to do it
168+
as soon as possible.
169+
*/
170+
thd->variables.gtid_next.set_undefined();
162171
broadcast_owned_sidnos(thd);
163172
unlock_owned_sidnos(thd);
164173

@@ -213,6 +222,12 @@ void Gtid_state::update_owned_gtids_impl(THD *thd, bool is_commit)
213222
owned_gtids.remove_gtid(thd->owned_gtid);
214223
}
215224

225+
/*
226+
There may be commands that cause implicit commits, e.g.
227+
SET AUTOCOMMIT=1 may cause the previous statements to commit
228+
without executing a COMMIT command or be on auto-commit mode.
229+
*/
230+
thd->variables.gtid_next.set_undefined();
216231
if (!is_commit)
217232
broadcast_owned_sidnos(thd);
218233
unlock_owned_sidnos(thd);

‎sql/share/errmsg-utf8.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6938,7 +6938,7 @@ ER_READ_ONLY_MODE
69386938
eng "Running in read-only mode"
69396939

69406940
ER_GTID_NEXT_TYPE_UNDEFINED_GROUP
6941-
eng "When @@SESSION.GTID_NEXT is set to a GTID, you must explicitly set it again after a COMMIT or ROLLBACK. If you see this error message in the slave SQL thread, it means that a table in the current transaction is transactional on the master and non-transactional on the slave. In a client connection, it means that you executed SET @@SESSION.GTID_NEXT before a transaction and forgot to set @@SESSION.GTID_NEXT to a different identifier or to 'AUTOMATIC' after COMMIT or ROLLBACK. Current @@SESSION.GTID_NEXT is '%s'."
6941+
eng "When @@SESSION.GTID_NEXT is set to a GTID, you must explicitly set it to a different value after a COMMIT or ROLLBACK. Please check GTID_NEXT variable manual page for detailed explanation. Current @@SESSION.GTID_NEXT is '%s'."
69426942

69436943
ER_VARIABLE_NOT_SETTABLE_IN_SP
69446944
eng "The system variable %.200s cannot be set in stored procedures."

‎sql/sql_class.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,6 +1969,9 @@ void THD::cleanup_after_query()
19691969
rand_used= 0;
19701970
binlog_accessed_db_names= NULL;
19711971
m_trans_fixed_log_file= NULL;
1972+
1973+
if (gtid_mode > 0)
1974+
gtid_post_statement_checks(this);
19721975
#ifndef EMBEDDED_LIBRARY
19731976
/*
19741977
Clean possible unused INSERT_ID events by current statement.

0 commit comments

Comments
 (0)