Commit 78cc9c9e authored by Sergey Vojtovich's avatar Sergey Vojtovich

Pre-MDEV-742 InnoDB fixes

1. Refactored innobase_close_connection(). Transaction must've already
been rolled back by this time. We should expect only transactions in the
PREPARED state when MDEV-742 is done.

2. Added missing put_pins() to trx_disconnect_prepared(). Missing
put_pins() wasn't a problem because trx_disconnect_prepared() is a dead
code. But it will get revived in the main MDEV-742 patch.

3. Fixed missing reset of trx->mysql_log_file_name when RW transaction
didn't emit any log records (zero-modification RW). The problem was
detected by ASAN when disconnected XA transaction was trying to make
use of inherited mysql_log_file_name pointing into binlog data of
detached THD.

This missing reset also had user-visible side effect, when
trx_sys_print_mysql_binlog_offset() would report binlog position
not of the most recently committed transaction.

One of possible scenarios that is expected to misbehave is as following:

  thr1> CREATE TABLE t1(a INT) ENGINE=InnoDB;
  thr1> INSERT INTO t1 VALUES(1);
  thr1> BEGIN;
  thr1> UPDATE t1 SET a=1
  thr1> COMMIT; -- zero-modification, misses to reset mysql_log_file_name

  thr2> BEGIN;
  thr2> INSERT INTO t1 VALUES(2);
  thr2> COMMIT;

  thr1> BEGIN;
  thr1> do-some-real-changes;
  thr1> ROLLBACK; -- will store binlog pos from previous COMMIT in thr1?

In this case it means if binlog is replayed from position reported by
trx_sys_print_mysql_binlog_offset(), t1 will end up with two records
containing '2'.

Part of
MDEV-742 - XA PREPAREd transaction survive disconnect/server restart
parent 662d8a86
...@@ -4272,6 +4272,7 @@ innobase_commit_ordered_2( ...@@ -4272,6 +4272,7 @@ innobase_commit_ordered_2(
innobase_commit_low(trx); innobase_commit_low(trx);
if (!read_only) { if (!read_only) {
trx->mysql_log_file_name = NULL;
trx->flush_log_later = false; trx->flush_log_later = false;
if (innobase_commit_concurrency > 0) { if (innobase_commit_concurrency > 0) {
...@@ -4512,11 +4513,7 @@ innobase_rollback_trx( ...@@ -4512,11 +4513,7 @@ innobase_rollback_trx(
we come here to roll back the latest SQL statement) we we come here to roll back the latest SQL statement) we
release it now before a possibly lengthy rollback */ release it now before a possibly lengthy rollback */
lock_unlock_table_autoinc(trx); lock_unlock_table_autoinc(trx);
trx_deregister_from_2pc(trx);
if (!trx->has_logged()) {
trx->will_lock = 0;
DBUG_RETURN(0);
}
DBUG_RETURN(convert_error_code_to_mysql(trx_rollback_for_mysql(trx), DBUG_RETURN(convert_error_code_to_mysql(trx_rollback_for_mysql(trx),
0, trx->mysql_thd)); 0, trx->mysql_thd));
...@@ -4807,74 +4804,39 @@ innobase_savepoint( ...@@ -4807,74 +4804,39 @@ innobase_savepoint(
DBUG_RETURN(convert_error_code_to_mysql(error, 0, NULL)); DBUG_RETURN(convert_error_code_to_mysql(error, 0, NULL));
} }
/*****************************************************************//**
Frees a possible InnoDB trx object associated with the current THD.
@return 0 or error number */
static
int
innobase_close_connection(
/*======================*/
handlerton* hton, /*!< in: innobase handlerton */
THD* thd) /*!< in: handle to the MySQL thread of the user
whose resources should be free'd */
{
DBUG_ENTER("innobase_close_connection"); /**
DBUG_ASSERT(hton == innodb_hton_ptr); Frees a possible InnoDB trx object associated with the current THD.
trx_t* trx = thd_to_trx(thd);
/* During server initialization MySQL layer will try to open
some of the master-slave tables those residing in InnoDB.
After MySQL layer is done with needed checks these tables
are closed followed by invocation of close_connection on the
associated thd.
close_connection rolls back the trx and then frees it.
Once trx is freed thd should avoid maintaining reference to
it else it can be classified as stale reference.
Re-invocation of innodb_close_connection on same thd should
get trx as NULL. */
if (trx) {
if (!trx_is_registered_for_2pc(trx) && trx_is_started(trx)) { @param hton innobase handlerton
@param thd server thread descriptor, which resources should be free'd
sql_print_error("Transaction not registered for MariaDB 2PC, " @return 0 always
"but transaction is active"); */
} static int innobase_close_connection(handlerton *hton, THD *thd)
{
/* Disconnect causes rollback in the following cases: DBUG_ASSERT(hton == innodb_hton_ptr);
- trx is not started, or if (auto trx= thd_to_trx(thd))
- trx is in *not* in PREPARED state, or {
- trx has not updated any persistent data. if (trx->state == TRX_STATE_PREPARED)
TODO/FIXME: it does not make sense to initiate rollback {
in the 1st and 3rd case. */ if (trx->has_logged_persistent())
if (trx_is_started(trx)) { {
if (trx_state_eq(trx, TRX_STATE_PREPARED)) { trx_disconnect_prepared(trx);
if (trx->has_logged_persistent()) { return 0;
trx_disconnect_prepared(trx); }
} else { innobase_rollback_trx(trx);
trx_deregister_from_2pc(trx); }
goto rollback_and_free; /*
} in theory it may fire if preceding rollback failed,
} else { but what can we do about it?
sql_print_warning( */
"MariaDB is closing a connection that has an active " DBUG_ASSERT(!trx_is_started(trx));
"InnoDB transaction. " TRX_ID_FMT " row modifications " /* some bad guy missed to reset trx->will_lock somewhere, reset it here */
"will roll back.", trx->will_lock= 0;
trx->undo_no); trx_free(trx);
goto rollback_and_free; }
} return 0;
} else {
rollback_and_free:
innobase_rollback_trx(trx);
trx_free(trx);
}
}
DBUG_RETURN(0);
} }
UNIV_INTERN void lock_cancel_waiting_and_release(lock_t* lock); UNIV_INTERN void lock_cancel_waiting_and_release(lock_t* lock);
...@@ -17387,7 +17349,6 @@ innobase_rollback_by_xid( ...@@ -17387,7 +17349,6 @@ innobase_rollback_by_xid(
} }
#endif /* WITH_WSREP */ #endif /* WITH_WSREP */
int ret = innobase_rollback_trx(trx); int ret = innobase_rollback_trx(trx);
trx_deregister_from_2pc(trx);
ut_ad(!trx->will_lock); ut_ad(!trx->will_lock);
trx_free(trx); trx_free(trx);
......
...@@ -399,6 +399,7 @@ void trx_free(trx_t*& trx) ...@@ -399,6 +399,7 @@ void trx_free(trx_t*& trx)
ut_ad(!trx->n_mysql_tables_in_use); ut_ad(!trx->n_mysql_tables_in_use);
ut_ad(!trx->mysql_n_tables_locked); ut_ad(!trx->mysql_n_tables_locked);
ut_ad(!trx->internal); ut_ad(!trx->internal);
ut_ad(!trx->mysql_log_file_name);
if (trx->declared_to_be_inside_innodb) { if (trx->declared_to_be_inside_innodb) {
...@@ -437,7 +438,6 @@ void trx_free(trx_t*& trx) ...@@ -437,7 +438,6 @@ void trx_free(trx_t*& trx)
trx_sys.rw_trx_hash.put_pins(trx); trx_sys.rw_trx_hash.put_pins(trx);
trx->mysql_thd = 0; trx->mysql_thd = 0;
trx->mysql_log_file_name = 0;
// FIXME: We need to avoid this heap free/alloc for each commit. // FIXME: We need to avoid this heap free/alloc for each commit.
if (trx->autoinc_locks != NULL) { if (trx->autoinc_locks != NULL) {
...@@ -546,11 +546,13 @@ void trx_disconnect_prepared(trx_t *trx) ...@@ -546,11 +546,13 @@ void trx_disconnect_prepared(trx_t *trx)
{ {
ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED));
ut_ad(trx->mysql_thd); ut_ad(trx->mysql_thd);
ut_ad(!trx->mysql_log_file_name);
trx->read_view.close(); trx->read_view.close();
trx->is_recovered= true; trx->is_recovered= true;
trx->mysql_thd= NULL; trx->mysql_thd= NULL;
/* todo/fixme: suggest to do it at innodb prepare */ /* todo/fixme: suggest to do it at innodb prepare */
trx->will_lock= 0; trx->will_lock= 0;
trx_sys.rw_trx_hash.put_pins(trx);
} }
/****************************************************************//** /****************************************************************//**
...@@ -1121,8 +1123,6 @@ trx_write_serialisation_history( ...@@ -1121,8 +1123,6 @@ trx_write_serialisation_history(
mutex_exit(&rseg->mutex); mutex_exit(&rseg->mutex);
MONITOR_INC(MONITOR_TRX_COMMIT_UNDO); MONITOR_INC(MONITOR_TRX_COMMIT_UNDO);
trx->mysql_log_file_name = NULL;
} }
/******************************************************************** /********************************************************************
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment