Skip to content

M
mariadb
Commit 30d5e327 authored by Sunny Bains's avatar Sunny Bains
Browse files
Options

Bug #57243 Inconsistent use of trans_register_ha() API in InnoDB 

Remove trx_t::active_trans. Split into two separate fields with distinct
responsibilities. trx_t::is_registered and trx_t::owns_prepare_mutex.
There are wrapper functions for using this fields in ha_innodb.cc. The
wrapper functions check for invariants.

Fix some formatting to conform to InnoDB guidelines.

Remove innobase_register_stmt() and innobase_register_trx_and_stmt().

Add:
trx_is_started()
trx_deregister_from_2pc()
trx_register_for_2pc()
trx_is_registered_for_2pc()
trx_owns_prepare_commit_mutex_set()
trx_has_prepare_commit_mutex()

rb://479, Approved by Jimmy Yang.
parent 7ee0551f
Show whitespace changes
Inline Side-by-side
Showing with 185 additions and 166 deletions
storage/innobase/handler/ha_innodb.cc
View file @ 30d5e327
......@@ -32,7 +32,6 @@ Place, Suite 330, Boston, MA 02111-1307 USA
*****************************************************************************/
/* TODO list for the InnoDB handler in 5.0:
- Remove the flag trx->active_trans and look at trx->conc_state
- fix savepoint functions to use savepoint storage area
- Find out what kind of problems the OS X case-insensitivity causes to
table and database names; should we 'normalize' the names like we do
......@@ -1500,7 +1499,7 @@ Gets the InnoDB transaction handle for a MySQL handler object, creates
an InnoDB transaction struct if the corresponding MySQL thread struct still
lacks one.
@return InnoDB transaction handle */
static
static inline
trx_t*
check_trx_exists(
/*=============*/
......@@ -1522,6 +1521,77 @@ check_trx_exists(
return(trx);
}
/*********************************************************************//**
Note that a transaction has been registered with MySQL.
@return true if transaction is registered with MySQL 2PC coordinator */
static inline
bool
trx_is_registered_for_2pc(
/*=========================*/
const trx_t* trx) /* in: transaction */
{
return(trx->is_registered == 1);
}
/*********************************************************************//**
Note that a transaction owns the prepare_commit_mutex. */
static inline
void
trx_owns_prepare_commit_mutex_set(
/*==============================*/
trx_t* trx) /* in: transaction */
{
ut_a(trx_is_registered_for_2pc(trx));
trx->owns_prepare_mutex = 1;
}
/*********************************************************************//**
Note that a transaction has been registered with MySQL 2PC coordinator. */
static inline
void
trx_register_for_2pc(
/*==================*/
trx_t* trx) /* in: transaction */
{
trx->is_registered = 1;
ut_ad(trx->owns_prepare_mutex == 0);
}
/*********************************************************************//**
Note that a transaction has been deregistered. */
static inline
void
trx_deregister_from_2pc(
/*====================*/
trx_t* trx) /* in: transaction */
{
trx->is_registered = 0;
trx->owns_prepare_mutex = 0;
}
/*********************************************************************//**
Check whether atransaction owns the prepare_commit_mutex.
@return true if transaction owns the prepare commit mutex */
static inline
bool
trx_has_prepare_commit_mutex(
/*=========================*/
const trx_t* trx) /* in: transaction */
{
return(trx->owns_prepare_mutex == 1);
}
/*********************************************************************//**
Check if transaction is started.
@reutrn true if transaction is in state started */
static
bool
trx_is_started(
/*===========*/
trx_t* trx) /* in: transaction */
{
return(trx->conc_state != TRX_NOT_STARTED);
}
/*********************************************************************//**
Construct ha_innobase handler. */
......@@ -1585,46 +1655,29 @@ ha_innobase::update_thd()
}
/*********************************************************************//**
Registers that InnoDB takes part in an SQL statement, so that MySQL knows to
roll back the statement if the statement results in an error. This MUST be
called for every SQL statement that may be rolled back by MySQL. Calling this
several times to register the same statement is allowed, too. */
Registers an InnoDB transaction with the MySQL 2PC coordinator, so that
the MySQL XA code knows to call the InnoDB prepare and commit, or rollback
for the transaction. This MUST be called for every transaction for which
the user may call commit or rollback. Calling this several times to register
the same transaction is allowed, too. This function also registers the
current SQL statement. */
static inline
void
innobase_register_stmt(
/*===================*/
handlerton* hton, /*!< in: Innobase hton */
THD* thd) /*!< in: MySQL thd (connection) object */
innobase_register_trx(
/*==================*/
handlerton* hton, /* in: Innobase handlerton */
THD* thd, /* in: MySQL thd (connection) object */
trx_t* trx) /* in: transaction to register */
{
DBUG_ASSERT(hton == innodb_hton_ptr);
/* Register the statement */
trans_register_ha(thd, FALSE, hton);
}
/*********************************************************************//**
Registers an InnoDB transaction in MySQL, so that the MySQL XA code knows
to call the InnoDB prepare and commit, or rollback for the transaction. This
MUST be called for every transaction for which the user may call commit or
rollback. Calling this several times to register the same transaction is
allowed, too.
This function also registers the current SQL statement. */
static inline
void
innobase_register_trx_and_stmt(
/*===========================*/
handlerton *hton, /*!< in: Innobase handlerton */
THD* thd) /*!< in: MySQL thd (connection) object */
{
/* NOTE that actually innobase_register_stmt() registers also
the transaction in the AUTOCOMMIT=1 mode. */
innobase_register_stmt(hton, thd);
if (!trx_is_registered_for_2pc(trx)
&& thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
if (thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
/* No autocommit mode, register for a transaction */
trans_register_ha(thd, TRUE, hton);
}
trx_register_for_2pc(trx);
}
/* BACKGROUND INFO: HOW THE MYSQL QUERY CACHE WORKS WITH INNODB
......@@ -1772,14 +1825,8 @@ innobase_query_caching_of_table_permitted(
#ifdef __WIN__
innobase_casedn_str(norm_name);
#endif
/* The call of row_search_.. will start a new transaction if it is
not yet started */
if (trx->active_trans == 0) {
innobase_register_trx_and_stmt(innodb_hton_ptr, thd);
trx->active_trans = 1;
}
innobase_register_trx(innodb_hton_ptr, thd, trx);
if (row_search_check_if_query_cache_permitted(trx, norm_name)) {
......@@ -2046,14 +2093,7 @@ ha_innobase::init_table_handle_for_HANDLER(void)
trx_assign_read_view(prebuilt->trx);
/* Set the MySQL flag to mark that there is an active transaction */
if (prebuilt->trx->active_trans == 0) {
innobase_register_trx_and_stmt(ht, user_thd);
prebuilt->trx->active_trans = 1;
}
innobase_register_trx(ht, user_thd, prebuilt->trx);
/* We did the necessary inits in this function, no need to repeat them
in row_search_for_mysql */
......@@ -2548,12 +2588,10 @@ innobase_commit_low(
/*================*/
trx_t* trx) /*!< in: transaction handle */
{
if (trx->conc_state == TRX_NOT_STARTED) {
return;
}
if (trx_is_started(trx)) {
trx_commit_for_mysql(trx);
}
}
/*****************************************************************//**
......@@ -2595,10 +2633,7 @@ innobase_start_trx_and_assign_read_view(
/* Set the MySQL flag to mark that there is an active transaction */
if (trx->active_trans == 0) {
innobase_register_trx_and_stmt(hton, thd);
trx->active_trans = 1;
}
innobase_register_trx(hton, current_thd, trx);
DBUG_RETURN(0);
}
......@@ -2632,27 +2667,17 @@ innobase_commit(
trx_search_latch_release_if_reserved(trx);
}
/* The flag trx->active_trans is set to 1 in
1. ::external_lock(),
2. ::start_stmt(),
3. innobase_query_caching_of_table_permitted(),
4. innobase_savepoint(),
5. ::init_table_handle_for_HANDLER(),
6. innobase_start_trx_and_assign_read_view(),
7. ::transactional_table_lock()
/* Transaction is deregistered only in a commit or a rollback. If
it is deregistered we know there cannot be resources to be freed
and we could return immediately. For the time being, we play safe
and do the cleanup though there should be nothing to clean up. */
and it is only set to 0 in a commit or a rollback. If it is 0 we know
there cannot be resources to be freed and we could return immediately.
For the time being, we play safe and do the cleanup though there should
be nothing to clean up. */
if (!trx_is_registered_for_2pc(trx) && trx_is_started(trx)) {
if (trx->active_trans == 0
&& trx->conc_state != TRX_NOT_STARTED) {
sql_print_error("trx->active_trans == 0, but"
" trx->conc_state != TRX_NOT_STARTED");
sql_print_error("Transaction not registered for MySQL 2PC, "
"but transaction is active");
}
if (all
|| (!thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))) {
......@@ -2709,15 +2734,15 @@ retry:
mysql_mutex_unlock(&commit_cond_m);
}
if (trx->active_trans == 2) {
if (trx_has_prepare_commit_mutex(trx)) {
mysql_mutex_unlock(&prepare_commit_mutex);
}
trx_deregister_from_2pc(trx);
/* Now do a write + flush of logs. */
trx_commit_complete_for_mysql(trx);
trx->active_trans = 0;
} else {
/* We just mark the SQL statement ended and do not do a
transaction commit */
......@@ -2789,7 +2814,7 @@ innobase_rollback(
|| !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
error = trx_rollback_for_mysql(trx);
trx->active_trans = 0;
trx_deregister_from_2pc(trx);
} else {
error = trx_rollback_last_sql_stat_for_mysql(trx);
}
......@@ -2932,8 +2957,8 @@ innobase_savepoint(
innobase_release_stat_resources(trx);
/* cannot happen outside of transaction */
DBUG_ASSERT(trx->active_trans);
/* Cannot happen outside of transaction */
DBUG_ASSERT(trx_is_registered_for_2pc(trx));
/* TODO: use provided savepoint data area to store savepoint data */
char name[64];
......@@ -2963,16 +2988,15 @@ innobase_close_connection(
ut_a(trx);
if (trx->active_trans == 0
&& trx->conc_state != TRX_NOT_STARTED) {
if (!trx_is_registered_for_2pc(trx) && trx_is_started(trx)) {
sql_print_error("trx->active_trans == 0, but"
" trx->conc_state != TRX_NOT_STARTED");
sql_print_error("Transaction not registered for MySQL 2PC, "
"but transaction is active");
}
if (trx->conc_state != TRX_NOT_STARTED &&
global_system_variables.log_warnings) {
if (trx_is_started(trx) && global_system_variables.log_warnings) {
sql_print_warning(
"MySQL is closing a connection that has an active "
"InnoDB transaction. %llu row modifications will "
......@@ -4872,7 +4896,7 @@ no_commit:
/* Altering to InnoDB format */
innobase_commit(ht, user_thd, 1);
/* Note that this transaction is still active. */
prebuilt->trx->active_trans = 1;
trx_register_for_2pc(prebuilt->trx);
/* We will need an IX lock on the destination table. */
prebuilt->sql_stat_start = TRUE;
} else {
......@@ -4888,7 +4912,7 @@ no_commit:
locks, so they have to be acquired again. */
innobase_commit(ht, user_thd, 1);
/* Note that this transaction is still active. */
prebuilt->trx->active_trans = 1;
trx_register_for_2pc(prebuilt->trx);
/* Re-acquire the table lock on the source table. */
row_lock_table_for_mysql(prebuilt, src_table, mode);
/* We will need an IX lock on the destination table. */
......@@ -8700,8 +8724,8 @@ ha_innobase::start_stmt(
prepared for an update of a row */
prebuilt->select_lock_type = LOCK_X;
} else {
if (trx->isolation_level != TRX_ISO_SERIALIZABLE
} else if (trx->isolation_level != TRX_ISO_SERIALIZABLE
&& thd_sql_command(thd) == SQLCOM_SELECT
&& lock_type == TL_READ) {
......@@ -8718,21 +8742,12 @@ ha_innobase::start_stmt(
3) ::init_table_handle_for_HANDLER(), and
4) ::transactional_table_lock(). */
prebuilt->select_lock_type =
prebuilt->stored_select_lock_type;
}
prebuilt->select_lock_type = prebuilt->stored_select_lock_type;
}
trx->detailed_error[0] = '\0';
/* Set the MySQL flag to mark that there is an active transaction */
if (trx->active_trans == 0) {
*trx->detailed_error = 0;
innobase_register_trx_and_stmt(ht, thd);
trx->active_trans = 1;
} else {
innobase_register_stmt(ht, thd);
}
innobase_register_trx(ht, thd, trx);
return(0);
}
......@@ -8821,22 +8836,14 @@ ha_innobase::external_lock(
if (lock_type != F_UNLCK) {
/* MySQL is setting a new table lock */
trx->detailed_error[0] = '\0';
*trx->detailed_error = 0;
/* Set the MySQL flag to mark that there is an active
transaction */
if (trx->active_trans == 0) {
innobase_register_trx_and_stmt(ht, thd);
trx->active_trans = 1;
} else if (trx->n_mysql_tables_in_use == 0) {
innobase_register_stmt(ht, thd);
}
innobase_register_trx(ht, thd, trx);
if (trx->isolation_level == TRX_ISO_SERIALIZABLE
&& prebuilt->select_lock_type == LOCK_NONE
&& thd_test_options(thd,
OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
&& thd_test_options(
thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
/* To get serializable execution, we let InnoDB
conceptually add 'LOCK IN SHARE MODE' to all SELECTs
......@@ -8906,12 +8913,14 @@ ha_innobase::external_lock(
trx->mysql_n_tables_locked = 0;
prebuilt->used_in_HANDLER = FALSE;
if (!thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
if (trx->active_trans != 0) {
if (!thd_test_options(
thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
if (trx_is_started(trx)) {
innobase_commit(ht, thd, TRUE);
}
} else {
if (trx->isolation_level <= TRX_ISO_READ_COMMITTED
} else if (trx->isolation_level <= TRX_ISO_READ_COMMITTED
&& trx->global_read_view) {
/* At low transaction isolation levels we let
......@@ -8920,7 +8929,6 @@ ha_innobase::external_lock(
read_view_close_for_mysql(trx);
}
}
}
DBUG_RETURN(0);
}
......@@ -8987,12 +8995,7 @@ ha_innobase::transactional_table_lock(
/* MySQL is setting a new transactional table lock */
/* Set the MySQL flag to mark that there is an active transaction */
if (trx->active_trans == 0) {
innobase_register_trx_and_stmt(ht, thd);
trx->active_trans = 1;
}
innobase_register_trx(ht, thd, trx);
if (THDVAR(thd, table_locks) && thd_in_lock_tables(thd)) {
ulint error = DB_SUCCESS;
......@@ -9005,7 +9008,8 @@ ha_innobase::transactional_table_lock(
DBUG_RETURN((int) error);
}
if (thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
if (thd_test_options(
thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) {
/* Store the current undo_no of the transaction
so that we know where to roll back if we have
......@@ -10046,10 +10050,10 @@ innobase_xa_prepare(
innobase_release_stat_resources(trx);
if (trx->active_trans == 0 && trx->conc_state != TRX_NOT_STARTED) {
if (!trx_is_registered_for_2pc(trx) && trx_is_started(trx)) {
sql_print_error("trx->active_trans == 0, but trx->conc_state != "
"TRX_NOT_STARTED");
sql_print_error("Transaction not registered for MySQL 2PC, "
"but transaction is active");
}
if (all
......@@ -10058,7 +10062,7 @@ innobase_xa_prepare(
/* We were instructed to prepare the whole transaction, or
this is an SQL statement end and autocommit is on */
ut_ad(trx->active_trans);
ut_ad(trx_is_registered_for_2pc(trx));
error = (int) trx_prepare_for_mysql(trx);
} else {
......@@ -10082,9 +10086,10 @@ innobase_xa_prepare(
srv_active_wake_master_thread();
if (thd_sql_command(thd) != SQLCOM_XA_PREPARE &&
(all || !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)))
{
if (thd_sql_command(thd) != SQLCOM_XA_PREPARE
&& (all
|| !thd_test_options(
thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))) {
/* For ibbackup to work the order of transactions in binlog
and InnoDB must be the same. Consider the situation
......@@ -10105,7 +10110,7 @@ innobase_xa_prepare(
will be between XA PREPARE and XA COMMIT, and we don't want
to block for undefined period of time. */
mysql_mutex_lock(&prepare_commit_mutex);
trx->active_trans = 2;
trx_owns_prepare_commit_mutex_set(trx);
}
return(error);
......@@ -10140,7 +10145,7 @@ static
int
innobase_commit_by_xid(
/*===================*/
handlerton *hton,
handlerton* hton,
XID* xid) /*!< in: X/Open XA transaction identification */
{
trx_t* trx;
......
storage/innobase/include/trx0trx.h
View file @ 30d5e327
......@@ -470,6 +470,20 @@ struct trx_struct{
of view of concurrency control:
TRX_ACTIVE, TRX_COMMITTED_IN_MEMORY,
... */
/*------------------------------*/
/* MySQL has a transaction coordinator to coordinate two phase
commit between multiple storage engines and the binary log. When
an engine participates in a transaction, it's responsible for
registering itself using the trans_register_ha() API. */
unsigned is_registered:1;/* This flag is set to 1 after the
transaction has been registered with
the coordinator using the XA API, and
is set to 0 after commit or rollback. */
unsigned owns_prepare_mutex:1;/* 1 if owns prepare mutex, if
this is set to 1 then registered should
also be set to 1. This is used in the
XA code */
/*------------------------------*/
ulint isolation_level;/* TRX_ISO_REPEATABLE_READ, ... */
ulint check_foreigns; /* normally TRUE, but if the user
wants to suppress foreign key checks,
......@@ -500,9 +514,6 @@ struct trx_struct{
in that case we must flush the log
in trx_commit_complete_for_mysql() */
ulint duplicates; /*!< TRX_DUP_IGNORE | TRX_DUP_REPLACE */
ulint active_trans; /*!< 1 - if a transaction in MySQL
is active. 2 - if prepare_commit_mutex
was taken */
ulint has_search_latch;
/* TRUE if this trx has latched the
search system latch in S-mode */
......
storage/innobase/trx/trx0trx.c
View file @ 30d5e327
......@@ -105,7 +105,11 @@ trx_create(
trx->is_purge = 0;
trx->is_recovered = 0;
trx->conc_state = TRX_NOT_STARTED;
trx->start_time = time(NULL);
trx->is_registered = 0;
trx->owns_prepare_mutex = 0;
trx->start_time = ut_time();
trx->isolation_level = TRX_ISO_REPEATABLE_READ;
......@@ -124,7 +128,6 @@ trx_create(
trx->table_id = 0;
trx->mysql_thd = NULL;
trx->active_trans = 0;
trx->duplicates = 0;
trx->n_mysql_tables_in_use = 0;
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7