Skip to content

Commit

Permalink
MDEV-25594: Improve debug checks
Browse files Browse the repository at this point in the history
trx_t::will_lock: Changed the type to bool.

trx_t::is_autocommit_non_locking(): Replaces
trx_is_autocommit_non_locking().

trx_is_ac_nl_ro(): Remove (replaced with equivalent assertion expressions).

assert_trx_nonlocking_or_in_list(): Remove.
Replaced with at least as strict checks in each place.

check_trx_state(): Moved to a static function; partially replaced with
individual debug assertions implementing equivalent or stricter checks.
  • Loading branch information
dr-m committed May 18, 2021
1 parent cc2651b commit 7b51d11
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 160 deletions.
48 changes: 16 additions & 32 deletions storage/innobase/handler/ha_innodb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4123,7 +4123,7 @@ innobase_commit_low(
if (trx_is_started(trx)) {
trx_commit_for_mysql(trx);
} else {
trx->will_lock = 0;
trx->will_lock = false;
#ifdef WITH_WSREP
trx->wsrep = false;
#endif /* WITH_WSREP */
Expand Down Expand Up @@ -7610,7 +7610,7 @@ ha_innobase::write_row(
ut_a(m_prebuilt->trx == trx);

if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

ins_mode_t vers_set_fields;
Expand Down Expand Up @@ -8369,7 +8369,7 @@ ha_innobase::update_row(
ib_senderrf(ha_thd(), IB_LOG_LEVEL_WARN, ER_READ_ONLY_MODE);
DBUG_RETURN(HA_ERR_TABLE_READONLY);
} else if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

if (m_upd_buf == NULL) {
Expand Down Expand Up @@ -8540,7 +8540,7 @@ ha_innobase::delete_row(
ib_senderrf(ha_thd(), IB_LOG_LEVEL_WARN, ER_READ_ONLY_MODE);
DBUG_RETURN(HA_ERR_TABLE_READONLY);
} else if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

if (!m_prebuilt->upd_node) {
Expand Down Expand Up @@ -9377,7 +9377,7 @@ ha_innobase::ft_init()
them as regular read only transactions for now. */

if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

DBUG_RETURN(rnd_init(false));
Expand Down Expand Up @@ -9443,7 +9443,7 @@ ha_innobase::ft_init_ext(
them as regular read only transactions for now. */

if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

dict_table_t* ft_table = m_prebuilt->table;
Expand Down Expand Up @@ -12972,7 +12972,7 @@ create_table_info_t::allocate_trx()
{
m_trx = innobase_trx_allocate(m_thd);

m_trx->will_lock++;
m_trx->will_lock = true;
m_trx->ddl = true;
}

Expand Down Expand Up @@ -13296,13 +13296,7 @@ inline int ha_innobase::delete_table(const char* name, enum_sql_command sqlcom)

ut_a(name_len < 1000);

/* Either the transaction is already flagged as a locking transaction
or it hasn't been started yet. */

ut_a(!trx_is_started(trx) || trx->will_lock > 0);

/* We are doing a DDL operation. */
++trx->will_lock;
trx->will_lock = true;

/* Drop the table in InnoDB */

Expand Down Expand Up @@ -13481,14 +13475,7 @@ innobase_drop_database(
#endif /* _WIN32 */

trx_t* trx = innobase_trx_allocate(thd);

/* Either the transaction is already flagged as a locking transaction
or it hasn't been started yet. */

ut_a(!trx_is_started(trx) || trx->will_lock > 0);

/* We are doing a DDL operation. */
++trx->will_lock;
trx->will_lock = true;

ulint dummy;

Expand Down Expand Up @@ -13532,7 +13519,7 @@ inline dberr_t innobase_rename_table(trx_t *trx, const char *from,
DEBUG_SYNC_C("innodb_rename_table_ready");

trx_start_if_not_started(trx, true);
ut_ad(trx->will_lock > 0);
ut_ad(trx->will_lock);

if (commit) {
/* Serialize data dictionary operations with dictionary mutex:
Expand Down Expand Up @@ -13642,8 +13629,7 @@ int ha_innobase::truncate()
heap, ib_table->name.m_name, ib_table->id);
const char* name = mem_heap_strdup(heap, ib_table->name.m_name);
trx_t* trx = innobase_trx_allocate(m_user_thd);

++trx->will_lock;
trx->will_lock = true;
trx_set_dict_operation(trx, TRX_DICT_OP_TABLE);
row_mysql_lock_data_dictionary(trx);
dict_stats_wait_bg_to_stop_using_table(ib_table, trx);
Expand Down Expand Up @@ -13728,9 +13714,7 @@ ha_innobase::rename_table(
}

trx_t* trx = innobase_trx_allocate(thd);

/* We are doing a DDL operation. */
++trx->will_lock;
trx->will_lock = true;
trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);

dberr_t error = innobase_rename_table(trx, from, to, true);
Expand Down Expand Up @@ -15602,7 +15586,7 @@ ha_innobase::start_stmt(
innobase_register_trx(ht, thd, trx);

if (!trx_is_started(trx)) {
++trx->will_lock;
trx->will_lock = true;
}

DBUG_RETURN(0);
Expand Down Expand Up @@ -15828,7 +15812,7 @@ ha_innobase::external_lock(
&& (m_prebuilt->select_lock_type != LOCK_NONE
|| m_prebuilt->stored_select_lock_type != LOCK_NONE)) {

++trx->will_lock;
trx->will_lock = true;
}

DBUG_RETURN(0);
Expand Down Expand Up @@ -15867,7 +15851,7 @@ ha_innobase::external_lock(
&& (m_prebuilt->select_lock_type != LOCK_NONE
|| m_prebuilt->stored_select_lock_type != LOCK_NONE)) {

++trx->will_lock;
trx->will_lock = true;
}

DBUG_RETURN(0);
Expand Down Expand Up @@ -16540,7 +16524,7 @@ ha_innobase::store_lock(
&& (m_prebuilt->select_lock_type != LOCK_NONE
|| m_prebuilt->stored_select_lock_type != LOCK_NONE)) {

++trx->will_lock;
trx->will_lock = true;
}

return(to);
Expand Down
3 changes: 1 addition & 2 deletions storage/innobase/handler/handler0alter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2352,7 +2352,7 @@ ha_innobase::check_if_supported_inplace_alter(
}
}

m_prebuilt->trx->will_lock++;
m_prebuilt->trx->will_lock = true;

/* When changing a NULL column to NOT NULL and specifying a
DEFAULT value, ensure that the DEFAULT expression is a constant.
Expand Down Expand Up @@ -11458,7 +11458,6 @@ ha_innobase::commit_inplace_alter_table(
m_prebuilt = ctx->prebuilt;
}
trx_start_if_not_started(user_trx, true);
user_trx->will_lock++;
m_prebuilt->trx = user_trx;
}
DBUG_INJECT_CRASH("ib_commit_inplace_crash",
Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/include/trx0i_s.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2015, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2020, MariaDB Corporation.
Copyright (c) 2017, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -151,7 +151,7 @@ struct i_s_trx_row_t {
bool trx_is_read_only;
/*!< trx_t::read_only */
bool trx_is_autocommit_non_locking;
/*!< trx_is_autocommit_non_locking(trx)
/*!< trx:t::is_autocommit_non_locking()
*/
};

Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/include/trx0sys.h
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ class rw_trx_hash_t
static void validate_element(trx_t *trx)
{
ut_ad(!trx->read_only || !trx->rsegs.m_redo.rseg);
ut_ad(!trx_is_autocommit_non_locking(trx));
ut_ad(!trx->is_autocommit_non_locking());
/* trx->state can be anything except TRX_STATE_NOT_STARTED */
mutex_enter(&trx->mutex);
ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE) ||
Expand Down
73 changes: 8 additions & 65 deletions storage/innobase/include/trx0trx.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,64 +393,6 @@ from innodb_lock_wait_timeout via trx_t::mysql_thd.
? thd_lock_wait_timeout((t)->mysql_thd) \
: 0)

/**
Determine if the transaction is a non-locking autocommit select
(implied read-only).
@param t transaction
@return true if non-locking autocommit select transaction. */
#define trx_is_autocommit_non_locking(t) \
((t)->auto_commit && (t)->will_lock == 0)

/**
Determine if the transaction is a non-locking autocommit select
with an explicit check for the read-only status.
@param t transaction
@return true if non-locking autocommit read-only transaction. */
#define trx_is_ac_nl_ro(t) \
((t)->read_only && trx_is_autocommit_non_locking((t)))

/**
Check transaction state */
#define check_trx_state(t) do { \
ut_ad(!trx_is_autocommit_non_locking((t))); \
switch ((t)->state) { \
case TRX_STATE_PREPARED: \
case TRX_STATE_PREPARED_RECOVERED: \
case TRX_STATE_ACTIVE: \
case TRX_STATE_COMMITTED_IN_MEMORY: \
continue; \
case TRX_STATE_NOT_STARTED: \
break; \
} \
ut_error; \
} while (0)

#ifdef UNIV_DEBUG
/*******************************************************************//**
Assert that an autocommit non-locking select cannot be in the
rw_trx_hash and that it is a read-only transaction.
The transaction must have mysql_thd assigned. */
# define assert_trx_nonlocking_or_in_list(t) \
do { \
if (trx_is_autocommit_non_locking(t)) { \
trx_state_t t_state = (t)->state; \
ut_ad((t)->read_only); \
ut_ad(!(t)->is_recovered); \
ut_ad((t)->mysql_thd); \
ut_ad(t_state == TRX_STATE_NOT_STARTED \
|| t_state == TRX_STATE_ACTIVE); \
} else { \
check_trx_state(t); \
} \
} while (0)
#else /* UNIV_DEBUG */
/*******************************************************************//**
Assert that an autocommit non-locking slect cannot be in the
rw_trx_hash and that it is a read-only transaction.
The transaction must have mysql_thd assigned. */
# define assert_trx_nonlocking_or_in_list(trx) ((void)0)
#endif /* UNIV_DEBUG */

typedef std::vector<ib_lock_t*, ut_allocator<ib_lock_t*> > lock_list;

/*******************************************************************//**
Expand Down Expand Up @@ -947,16 +889,15 @@ struct trx_t : ilist_node<> {
/*------------------------------*/
bool read_only; /*!< true if transaction is flagged
as a READ-ONLY transaction.
if auto_commit && will_lock == 0
if auto_commit && !will_lock
then it will be handled as a
AC-NL-RO-SELECT (Auto Commit Non-Locking
Read Only Select). A read only
transaction will not be assigned an
UNDO log. */
bool auto_commit; /*!< true if it is an autocommit */
ib_uint32_t will_lock; /*!< Will acquire some locks. Increment
each time we determine that a lock will
be acquired by the MySQL layer. */
bool will_lock; /*!< set to inform trx_start_low() that
the transaction may acquire locks */
/*------------------------------*/
fts_trx_t* fts_trx; /*!< FTS information, or NULL if
transaction hasn't modified tables
Expand Down Expand Up @@ -1103,11 +1044,13 @@ struct trx_t : ilist_node<> {
ut_ad(dict_operation == TRX_DICT_OP_NONE);
}

/** @return whether this is a non-locking autocommit transaction */
bool is_autocommit_non_locking() const { return auto_commit && !will_lock; }

private:
/** Assign a rollback segment for modifying temporary tables.
@return the assigned rollback segment */
trx_rseg_t* assign_temp_rseg();
/** Assign a rollback segment for modifying temporary tables.
@return the assigned rollback segment */
trx_rseg_t *assign_temp_rseg();
};

/**
Expand Down
10 changes: 7 additions & 3 deletions storage/innobase/include/trx0trx.ic
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************

Copyright (c) 1996, 2015, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2016, 2019, MariaDB Corporation.
Copyright (c) 2016, 2021, MariaDB Corporation.

This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -49,11 +49,15 @@ trx_state_eq(
case TRX_STATE_PREPARED:
case TRX_STATE_PREPARED_RECOVERED:
case TRX_STATE_COMMITTED_IN_MEMORY:
ut_ad(!trx_is_autocommit_non_locking(trx));
ut_ad(!trx->is_autocommit_non_locking());
return(trx->state == state);

case TRX_STATE_ACTIVE:
assert_trx_nonlocking_or_in_list(trx);
if (trx->is_autocommit_non_locking()) {
ut_ad(!trx->is_recovered);
ut_ad(trx->read_only);
ut_ad(trx->mysql_thd);
}
return(state == trx->state);

case TRX_STATE_NOT_STARTED:
Expand Down
Loading

0 comments on commit 7b51d11

Please sign in to comment.