3455 Marko Mäkelä 2011-01-24
Fix some things after WL#5458.
lock_rec_validate_page(): Rename the misleading parameter
have_lock_trx_sys_mutex to locked_lock_trx_sys. There is no
trx_sys->mutex, it is trx_sys->lock.
lock_sys_create(): Remove some commented-out code.
lock_set_lock_and_trx_wait(): Add debug assertions. Assert that
trx->mutex is being held. Previously, this constraint was being
violated.
lock_rec_create(): Add debug assertions. Protect
lock_set_lock_and_trx_wait() with trx->mutex.
lock_rec_add_to_queue(): Add a debug assertion on caller_owns_trx_mutex.
lock_move_rec_list_end(): Remove unnecessary acquisition of trx->mutex
around lock_reset_lock_and_trx_wait(). A running transaction cannot
have lock waits; therefore the lock waits can safely be reset while
the transaction is not running, while holding just lock_sys->mutex.
lock_deadlock_recursive(): Protect
trx->lock.was_chosen_as_deadlock_victim with trx->mutex.
lock_rec_queue_validate(): Rename have_lock_trx_sys_mutex to
locked_lock_trx_sys. Add debug assertions. Remove some duplicated code.
modified:
storage/innobase/lock/lock0lock.c
3454 Marko Mäkelä 2011-01-24
Non-functional change, cleaning up after WL#5458.
Remove the trx_get_on_id() wrapper, because it is inherently unsafe.
Rename trx_get_on_id_low() to trx_get_on_id(). Adjust callers.
modified:
storage/innobase/include/trx0sys.h
storage/innobase/include/trx0sys.ic
storage/innobase/row/row0vers.c
storage/innobase/trx/trx0trx.c
3453 Marko Mäkelä 2011-01-24
Instead of checking if trx->state != some_expected_value, check
trx->state against all allowed values, to detect corruption.
trx_state_eq(trx, state): Test if trx->state == state. In debug builds,
check trx->state, trx->in_mysql_trx_list, and trx->in_trx_list for corruption.
trx_assert_started(): A debug predicate. Assert if trx is not in one
of the active states.
trx_in_trx_list(): Assert trx_was_started(). This allows us to remove
the trx->state assertions in the caller.
lock_table_queue_validate(), lock_rec_queue_validate(),
lock_rec_validate_page(): Remove unnecessary acquisition of trx->mutex.
Use trx_state_eq(), trx_was_started(), or trx_in_trx_list() for the
trx->state checks.
trx_is_active_low(), trx_recover_for_mysql(), trx_get_trx_by_xid(),
read_view_open_now_low(), read_cursor_view_create_for_mysql(),
row_vers_build_for_semi_consistent_read():
Use trx_state_eq().
trx_rollback_for_mysql(), trx_rollback_last_sql_stat_for_mysql(),
trx_rollback_to_savepoint_for_mysql(), trx_rollback_resurrected(),
trx_commit_or_rollback_prepare(), trx_mark_sql_stat_end():
Check trx->state against all allowed values. Committed transactions
must not be rolled back or committed. Partial rollback of prepared
transactions is forbidden. Committed or prepared transactions must not
execute statements.
trx_rollback_to_savepoint_low(): Refactored from
trx_rollback_to_savepoint_for_mysql(). Roll back to a savepoint
identified by trx_named_savept_t.
trx_release_savepoint_for_mysql(): Add debug assertions.
trx_sys_init_at_db_start(): Use trx_state_eq(trx, TRX_STATE_ACTIVE).
This might correct a bug where rows_to_undo would include the rows of
recovered committed transactions.
trx_list_insert_ordered(), trx_start_low(), trx_commit():
Add debug assertions.
rb://579 approved by Jimmy Yang
modified:
storage/innobase/include/trx0roll.h
storage/innobase/include/trx0sys.h
storage/innobase/include/trx0sys.ic
storage/innobase/include/trx0trx.h
storage/innobase/include/trx0trx.ic
storage/innobase/lock/lock0lock.c
storage/innobase/read/read0read.c
storage/innobase/row/row0vers.c
storage/innobase/trx/trx0roll.c
storage/innobase/trx/trx0sys.c
storage/innobase/trx/trx0trx.c
=== modified file 'storage/innobase/include/trx0sys.h'
--- a/storage/innobase/include/trx0sys.h revid:marko.makela@strippedom-20110124102828-1o7l37nibd18jwja
+++ b/storage/innobase/include/trx0sys.h revid:marko.makela@stripped125226-tecjgoda8urz5lkn
@@ -261,15 +261,10 @@ trx_read_trx_id(
const byte* ptr); /*!< in: pointer to memory from where to read */
/****************************************************************//**
Looks for the trx handle with the given id in trx_list.
-@return the trx handle or NULL if not found */
-UNIV_INLINE
-trx_t*
-trx_get_on_id_low(
-/*==============*/
- trx_id_t trx_id);/*!< in: trx id to search for */
-/****************************************************************//**
-Looks for the trx handle with the given id in trx_list.
-@return the trx handle or NULL if not found */
+The caller must be holding trx_sys->lock.
+@return the trx handle or NULL if not found;
+the pointer must not be dereferenced unless lock_sys->mutex was
+acquired before calling this function and is still being held */
UNIV_INLINE
trx_t*
trx_get_on_id(
=== modified file 'storage/innobase/include/trx0sys.ic'
--- a/storage/innobase/include/trx0sys.ic revid:marko.makela@stripped
+++ b/storage/innobase/include/trx0sys.ic revid:marko.makela@oracle.com-20110124125226-tecjgoda8urz5lkn
@@ -246,13 +246,15 @@ trx_read_trx_id(
}
/****************************************************************//**
-Looks for the trx handle with the given id in trx_list. Because the list
-is ordered on trx id in descending order, we try and speed things up a bit
-@return the trx handle or NULL if not found */
+Looks for the trx handle with the given id in trx_list.
+The caller must be holding trx_sys->lock.
+@return the trx handle or NULL if not found;
+the pointer must not be dereferenced unless lock_sys->mutex was
+acquired before calling this function and is still being held */
UNIV_INLINE
trx_t*
-trx_get_on_id_low(
-/*==============*/
+trx_get_on_id(
+/*==========*/
trx_id_t trx_id) /*!< in: trx id to search for */
{
trx_t* trx;
@@ -311,27 +313,7 @@ trx_get_on_id_low(
}
/****************************************************************//**
-Looks for the trx handle with the given id in trx_list.
-@return the trx handle or NULL if not found */
-UNIV_INLINE
-trx_t*
-trx_get_on_id(
-/*==========*/
- trx_id_t trx_id) /*!< in: trx id to search for */
-{
- trx_t* trx;
-
- rw_lock_s_lock(&trx_sys->lock);
-
- trx = trx_get_on_id_low(trx_id);
-
- rw_lock_s_unlock(&trx_sys->lock);
-
- return(trx);
-}
-
-/****************************************************************//**
-Returns the minumum trx id in trx list. This is the smallest id for which
+Returns the minimum trx id in trx list. This is the smallest id for which
the trx can possibly be active. (But, you must look at the trx->state
to find out if the minimum trx id transaction itself is active, or already
committed.). The caller must be holding the trx_sys_t::lock in shared mode.
@@ -412,7 +394,7 @@ trx_is_active_low(
*corrupt = TRUE;
}
} else {
- trx = trx_get_on_id_low(trx_id);
+ trx = trx_get_on_id(trx_id);
if (trx != NULL
&& trx_state_eq(trx, TRX_STATE_COMMITTED_IN_MEMORY)) {
=== modified file 'storage/innobase/lock/lock0lock.c'
--- a/storage/innobase/lock/lock0lock.c revid:marko.makela@oracle.com-20110124102828-1o7l37nibd18jwja
+++ b/storage/innobase/lock/lock0lock.c revid:marko.makela@stripped110124125226-tecjgoda8urz5lkn
@@ -368,14 +368,13 @@ static
ibool
lock_rec_validate_page(
/*===================*/
- ibool have_lock_trx_sys_mutex, /*!< in: if the caller holds
- both the lock mutex and
- trx_sys_t::lock. */
- ulint space, /*!< in: space id */
- ulint zip_size, /*!< in: compressed page size
- in bytes or 0 for uncompressed
- pages */
- ulint page_no); /*!< in: page number */
+ ibool locked_lock_trx_sys, /*!< in: if the caller holds
+ both the lock mutex and
+ trx_sys_t->lock. */
+ ulint space, /*!< in: space id */
+ ulint zip_size, /*!< in: compressed page size
+ in bytes or 0 for uncompressed pages */
+ ulint page_no); /*!< in: page number */
#endif /* UNIV_DEBUG */
/* The lock system */
@@ -587,10 +586,10 @@ lock_sys_create(
srv_n_lock_wait_time = 0;
srv_n_lock_max_wait_time = 0;
srv_lock_timeout_active = FALSE;
- srv_n_lock_wait_current_count = 0;
+ srv_n_lock_wait_current_count = 0;
- lock_sys_sz = sizeof(*lock_sys)
- + (OS_THREAD_MAX_N * sizeof(srv_slot_t));
+ lock_sys_sz = sizeof(*lock_sys)
+ + OS_THREAD_MAX_N * sizeof(srv_slot_t);
lock_sys = mem_zalloc(lock_sys_sz);
@@ -605,8 +604,6 @@ lock_sys_create(
lock_sys->rec_hash = hash_create(n_cells);
- /* hash_create_mutexes(lock_sys->rec_hash, 2, SYNC_REC_LOCK); */
-
lock_latest_err_file = os_file_create_tmpfile();
ut_a(lock_latest_err_file);
@@ -825,7 +822,10 @@ lock_set_lock_and_trx_wait(
trx_t* trx) /*!< in/out: trx */
{
ut_ad(lock);
+ ut_ad(lock->trx == trx);
ut_ad(trx->lock.wait_lock == NULL);
+ ut_ad(lock_mutex_own());
+ ut_ad(trx_mutex_own(trx));
trx->lock.wait_lock = lock;
lock->type_mode |= LOCK_WAIT;
@@ -1721,6 +1721,7 @@ lock_rec_create(
const page_t* page;
ut_ad(lock_mutex_own());
+ ut_ad(caller_owns_trx_mutex == trx_mutex_own(trx));
space = buf_block_get_space(block);
page_no = buf_block_get_page_no(block);
@@ -1768,15 +1769,16 @@ lock_rec_create(
HASH_INSERT(lock_t, hash, lock_sys->rec_hash,
lock_rec_fold(space, page_no), lock);
+ if (!caller_owns_trx_mutex) {
+ trx_mutex_enter(trx);
+ }
+ ut_ad(trx_mutex_own(trx));
+
if (UNIV_UNLIKELY(type_mode & LOCK_WAIT)) {
lock_set_lock_and_trx_wait(lock, trx);
}
- if (!caller_owns_trx_mutex) {
- trx_mutex_enter(trx);
- }
-
UT_LIST_ADD_LAST(trx_locks, trx->lock.trx_locks, lock);
if (!caller_owns_trx_mutex) {
@@ -1921,6 +1923,7 @@ lock_rec_add_to_queue(
lock_t* first_lock;
ut_ad(lock_mutex_own());
+ ut_ad(caller_owns_trx_mutex == trx_mutex_own(trx));
#ifdef UNIV_DEBUG
switch (type_mode & LOCK_MODE_MASK) {
case LOCK_X:
@@ -2832,9 +2835,7 @@ lock_move_rec_list_end(
lock_rec_reset_nth_bit(lock, heap_no);
if (UNIV_UNLIKELY(type_mode & LOCK_WAIT)) {
- trx_mutex_enter(lock->trx);
lock_reset_lock_and_trx_wait(lock);
- trx_mutex_exit(lock->trx);
}
if (comp) {
@@ -3718,17 +3719,11 @@ lock_deadlock_recursive(
lock_deadlock_fputs(
"*** WE ROLL BACK TRANSACTION (1)\n");
- wait_lock->trx->lock.was_chosen_as_deadlock_victim
- = TRUE;
-
- /* We need to release the transaction mutex,
- in case start is granted its locks once we
- release wait_lock. */
-
trx_mutex_enter(wait_lock->trx);
+ wait_lock->trx->lock
+ .was_chosen_as_deadlock_victim = TRUE;
lock_cancel_waiting_and_release(wait_lock);
-
trx_mutex_exit(wait_lock->trx);
trx_mutex_enter(start);
@@ -4954,9 +4949,10 @@ static
ibool
lock_rec_queue_validate(
/*====================*/
- ibool have_lock_trx_sys_mutex,
+ ibool locked_lock_trx_sys,
/*!< in: if the caller holds
- both the lock and trx sys mutexes. */
+ both the lock mutex and
+ trx_sys_t->lock. */
const buf_block_t* block, /*!< in: buffer block containing rec */
const rec_t* rec, /*!< in: record to look at */
dict_index_t* index, /*!< in: index, or NULL if not known */
@@ -4970,16 +4966,20 @@ lock_rec_queue_validate(
ut_a(block->frame == page_align(rec));
ut_ad(rec_offs_validate(rec, index, offsets));
ut_ad(!page_rec_is_comp(rec) == !rec_offs_comp(offsets));
+ ut_ad(lock_mutex_own() == locked_lock_trx_sys);
+#ifdef UNIV_SYNC_DEBUG
+ ut_ad(rw_lock_own(&trx_sys->lock, RW_LOCK_SHARED)
+ == locked_lock_trx_sys);
+#endif /* UNIV_SYNC_DEBUG */
heap_no = page_rec_get_heap_no(rec);
- if (!page_rec_is_user_rec(rec)) {
-
- if (!have_lock_trx_sys_mutex) {
- lock_mutex_enter();
+ if (!locked_lock_trx_sys) {
+ lock_mutex_enter();
+ rw_lock_s_lock(&trx_sys->lock);
+ }
- rw_lock_s_lock(&trx_sys->lock);
- }
+ if (!page_rec_is_user_rec(rec)) {
for (lock = lock_rec_get_first(block, heap_no);
lock != NULL;
@@ -4996,19 +4996,7 @@ lock_rec_queue_validate(
}
}
- if (!have_lock_trx_sys_mutex) {
- rw_lock_s_unlock(&trx_sys->lock);
-
- lock_mutex_exit();
- }
-
- return(TRUE);
- }
-
- if (!have_lock_trx_sys_mutex) {
- lock_mutex_enter();
-
- rw_lock_s_lock(&trx_sys->lock);
+ goto func_exit;
}
if (!index);
@@ -5062,11 +5050,10 @@ lock_rec_queue_validate(
}
}
- if (!have_lock_trx_sys_mutex) {
-
- rw_lock_s_unlock(&trx_sys->lock);
-
+func_exit:
+ if (!locked_lock_trx_sys) {
lock_mutex_exit();
+ rw_lock_s_unlock(&trx_sys->lock);
}
return(TRUE);
@@ -5079,14 +5066,13 @@ static
ibool
lock_rec_validate_page(
/*===================*/
- ibool have_lock_trx_sys_mutex, /*!< in: if the caller holds
- both the lock and trx sys
- mutexes. */
- ulint space, /*!< in: space id */
- ulint zip_size, /*!< in: compressed page size
- in bytes or 0 for uncompressed
- pages */
- ulint page_no) /*!< in: page number */
+ ibool locked_lock_trx_sys, /*!< in: if the caller holds
+ both the lock mutex and
+ trx_sys_t->lock. */
+ ulint space, /*!< in: space id */
+ ulint zip_size, /*!< in: compressed page size
+ in bytes or 0 for uncompressed pages */
+ ulint page_no) /*!< in: page number */
{
dict_index_t* index;
buf_block_t* block;
@@ -5103,10 +5089,9 @@ lock_rec_validate_page(
rec_offs_init(offsets_);
/* This is to preserve latching order. */
- if (have_lock_trx_sys_mutex) {
- rw_lock_s_unlock(&trx_sys->lock);
-
+ if (locked_lock_trx_sys) {
lock_mutex_exit();
+ rw_lock_s_unlock(&trx_sys->lock);
}
ut_ad(!lock_mutex_own());
@@ -5190,10 +5175,9 @@ function_exit:
mtr_commit(&mtr);
- if (!have_lock_trx_sys_mutex) {
- rw_lock_s_unlock(&trx_sys->lock);
-
+ if (!locked_lock_trx_sys) {
lock_mutex_exit();
+ rw_lock_s_unlock(&trx_sys->lock);
}
if (UNIV_LIKELY_NULL(heap)) {
=== modified file 'storage/innobase/row/row0vers.c'
--- a/storage/innobase/row/row0vers.c revid:marko.makela@stripped37nibd18jwja
+++ b/storage/innobase/row/row0vers.c revid:marko.makela@stripped
@@ -681,7 +681,7 @@ row_vers_build_for_semi_consistent_read(
}
rw_lock_s_lock(&trx_sys->lock);
- version_trx = trx_get_on_id_low(version_trx_id);
+ version_trx = trx_get_on_id(version_trx_id);
/* version_trx->state cannot change from or to
NOT_STARTED while we are holding the trx_sys->lock.
It may change from ACTIVE to PREPARED or COMMITTED. */
=== modified file 'storage/innobase/trx/trx0trx.c'
--- a/storage/innobase/trx/trx0trx.c revid:marko.makela@strippedja
+++ b/storage/innobase/trx/trx0trx.c revid:marko.makela@stripped
@@ -573,7 +573,9 @@ trx_lists_init_at_db_start(void)
ibool trx_created;
/* Check the trx_sys->trx_list first. */
+ rw_lock_s_lock(&trx_sys->lock);
trx = trx_get_on_id(undo->trx_id);
+ rw_lock_s_unlock(&trx_sys->lock);
if (trx == NULL) {
trx = trx_allocate_for_background();
Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20110124125226-tecjgoda8urz5lkn.bundle
| Thread |
|---|
| • bzr push into mysql-trunk-innodb branch (marko.makela:3453 to 3455) WL#5458 | marko.makela | 24 Jan |
