From 48070b5332081554abb52efce9b4509ac5ec4d93 Mon Sep 17 00:00:00 2001 From: marko <> Date: Thu, 12 Mar 2009 08:26:40 +0000 Subject: [PATCH] branches/zip: Merge revisions 4359:4400 from branches/5.1: ------------------------------------------------------------------------ r4399 | marko | 2009-03-12 09:38:05 +0200 (Thu, 12 Mar 2009) | 2 lines branches/5.1: row_sel_get_clust_rec_for_mysql(): Store the cursor position also for unlock_row(). (Bug #39320) ------------------------------------------------------------------------ r4400 | marko | 2009-03-12 10:06:44 +0200 (Thu, 12 Mar 2009) | 5 lines branches/5.1: Fix a bug in multi-table semi-consistent reads. Remember the acquired record locks per table handle (row_prebuilt_t) rather than per transaction (trx_t), so that unlock_row should successfully unlock all non-matching rows in multi-table operations. This deficiency was found while investigating Bug #39320. ------------------------------------------------------------------------ These were submitted as rb://94 and rb://96 and approved by Heikki Tuuri. --- include/row0mysql.h | 15 ++++++ include/trx0trx.h | 42 ----------------- include/trx0trx.ic | 58 ------------------------ lock/lock0lock.c | 21 --------- mysql-test/innodb-semi-consistent.result | 7 +++ mysql-test/innodb-semi-consistent.test | 13 ++++++ row/row0mysql.c | 21 +-------- row/row0sel.c | 43 ++++++++++-------- trx/trx0trx.c | 2 - 9 files changed, 62 insertions(+), 160 deletions(-) diff --git a/include/row0mysql.h b/include/row0mysql.h index c1e11124a5d..8e42c316209 100644 --- a/include/row0mysql.h +++ b/include/row0mysql.h @@ -695,6 +695,21 @@ struct row_prebuilt_struct { This eliminates lock waits in some cases; note that this breaks serializability. */ + ulint new_rec_locks; /* normally 0; if + srv_locks_unsafe_for_binlog is + TRUE or session is using READ + COMMITTED isolation level, in a + cursor search, if we set a new + record lock on an index, this is + incremented; this is used in + releasing the locks under the + cursors if we are performing an + UPDATE and we determine after + retrieving the row that it does + not need to be locked; thus, + these can be used to implement a + 'mini-rollback' that releases + the latest record locks */ ulint mysql_prefix_len;/* byte offset of the end of the last requested column */ ulint mysql_row_len; /* length in bytes of a row in the diff --git a/include/trx0trx.h b/include/trx0trx.h index 8d5803ab353..e6ec2110bc1 100644 --- a/include/trx0trx.h +++ b/include/trx0trx.h @@ -43,34 +43,6 @@ extern sess_t* trx_dummy_sess; the kernel mutex */ extern ulint trx_n_mysql_transactions; -/***************************************************************** -Resets the new record lock info in a transaction struct. */ -UNIV_INLINE -void -trx_reset_new_rec_lock_info( -/*========================*/ - trx_t* trx); /* in: transaction struct */ -/***************************************************************** -Registers that we have set a new record lock on an index. We only have space -to store 2 indexes! If this is called to store more than 2 indexes after -trx_reset_new_rec_lock_info(), then this function does nothing. */ -UNIV_INLINE -void -trx_register_new_rec_lock( -/*======================*/ - trx_t* trx, /* in: transaction struct */ - dict_index_t* index); /* in: trx sets a new record lock on this - index */ -/***************************************************************** -Checks if trx has set a new record lock on an index. */ -UNIV_INLINE -ibool -trx_new_rec_locks_contain( -/*======================*/ - /* out: TRUE if trx has set a new record lock - on index */ - trx_t* trx, /* in: transaction struct */ - dict_index_t* index); /* in: index */ /************************************************************************ Releases the search latch if trx has reserved it. */ UNIV_INTERN @@ -609,20 +581,6 @@ struct trx_struct{ to srv_conc_innodb_enter, if the value here is > 0, we decrement this by 1 */ /*------------------------------*/ - dict_index_t* new_rec_locks[2];/* these are normally NULL; if - srv_locks_unsafe_for_binlog is TRUE - or session is using READ COMMITTED - isolation level, - in a cursor search, if we set a new - record lock on an index, this is set - to point to the index; this is - used in releasing the locks under the - cursors if we are performing an UPDATE - and we determine after retrieving - the row that it does not need to be - locked; thus, these can be used to - implement a 'mini-rollback' that - releases the latest record locks */ UT_LIST_NODE_T(trx_t) trx_list; /* list of transactions */ UT_LIST_NODE_T(trx_t) diff --git a/include/trx0trx.ic b/include/trx0trx.ic index 6da89f002fe..51212539c09 100644 --- a/include/trx0trx.ic +++ b/include/trx0trx.ic @@ -55,64 +55,6 @@ trx_start_if_not_started_low( } } -/***************************************************************** -Resets the new record lock info in a transaction struct. */ -UNIV_INLINE -void -trx_reset_new_rec_lock_info( -/*========================*/ - trx_t* trx) /* in: transaction struct */ -{ - trx->new_rec_locks[0] = NULL; - trx->new_rec_locks[1] = NULL; -} - -/***************************************************************** -Registers that we have set a new record lock on an index. We only have space -to store 2 indexes! If this is called to store more than 2 indexes after -trx_reset_new_rec_lock_info(), then this function does nothing. */ -UNIV_INLINE -void -trx_register_new_rec_lock( -/*======================*/ - trx_t* trx, /* in: transaction struct */ - dict_index_t* index) /* in: trx sets a new record lock on this - index */ -{ - if (trx->new_rec_locks[0] == NULL) { - trx->new_rec_locks[0] = index; - - return; - } - - if (trx->new_rec_locks[0] == index) { - - return; - } - - if (trx->new_rec_locks[1] != NULL) { - - return; - } - - trx->new_rec_locks[1] = index; -} - -/***************************************************************** -Checks if trx has set a new record lock on an index. */ -UNIV_INLINE -ibool -trx_new_rec_locks_contain( -/*======================*/ - /* out: TRUE if trx has set a new record lock - on index */ - trx_t* trx, /* in: transaction struct */ - dict_index_t* index) /* in: index */ -{ - return(trx->new_rec_locks[0] == index - || trx->new_rec_locks[1] == index); -} - /******************************************************************** Retrieves the error_info field from a trx. */ UNIV_INLINE diff --git a/lock/lock0lock.c b/lock/lock0lock.c index 8368fe584ee..11f839c1f56 100644 --- a/lock/lock0lock.c +++ b/lock/lock0lock.c @@ -1976,12 +1976,6 @@ lock_rec_lock_fast( if (lock == NULL) { if (!impl) { lock_rec_create(mode, block, heap_no, index, trx); - - if (srv_locks_unsafe_for_binlog - || trx->isolation_level - == TRX_ISO_READ_COMMITTED) { - trx_register_new_rec_lock(trx, index); - } } return(TRUE); @@ -2005,11 +1999,6 @@ lock_rec_lock_fast( if (!lock_rec_get_nth_bit(lock, heap_no)) { lock_rec_set_nth_bit(lock, heap_no); - if (srv_locks_unsafe_for_binlog - || trx->isolation_level - == TRX_ISO_READ_COMMITTED) { - trx_register_new_rec_lock(trx, index); - } } } @@ -2069,22 +2058,12 @@ lock_rec_lock_slow( err = lock_rec_enqueue_waiting(mode, block, heap_no, index, thr); - - if (srv_locks_unsafe_for_binlog - || trx->isolation_level == TRX_ISO_READ_COMMITTED) { - trx_register_new_rec_lock(trx, index); - } } else { if (!impl) { /* Set the requested lock on the record */ lock_rec_add_to_queue(LOCK_REC | mode, block, heap_no, index, trx); - if (srv_locks_unsafe_for_binlog - || trx->isolation_level - == TRX_ISO_READ_COMMITTED) { - trx_register_new_rec_lock(trx, index); - } } err = DB_SUCCESS; diff --git a/mysql-test/innodb-semi-consistent.result b/mysql-test/innodb-semi-consistent.result index 55e3cb5c7b4..ca0e362ef80 100644 --- a/mysql-test/innodb-semi-consistent.result +++ b/mysql-test/innodb-semi-consistent.result @@ -38,3 +38,10 @@ a 11 7 drop table t1; +create table t1 (a int, b int) engine=myisam; +create table t2 (c int, d int, key (c)) engine=innodb; +insert into t1 values (1,1); +insert into t2 values (1,2); +set session transaction isolation level read committed; +delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1); +drop table t1, t2; diff --git a/mysql-test/innodb-semi-consistent.test b/mysql-test/innodb-semi-consistent.test index 6d3020bb560..61ad7815ca9 100644 --- a/mysql-test/innodb-semi-consistent.test +++ b/mysql-test/innodb-semi-consistent.test @@ -53,3 +53,16 @@ drop table t1; connection default; disconnect a; disconnect b; + +# Bug 39320 +create table t1 (a int, b int) engine=myisam; +create table t2 (c int, d int, key (c)) engine=innodb; +insert into t1 values (1,1); +insert into t2 values (1,2); +connect (a,localhost,root,,); +connection a; +set session transaction isolation level read committed; +delete from t1 using t1 join t2 on t1.a = t2.c where t2.d in (1); +connection default; +disconnect a; +drop table t1, t2; diff --git a/row/row0mysql.c b/row/row0mysql.c index 8204285cc38..ce50aefbd24 100644 --- a/row/row0mysql.c +++ b/row/row0mysql.c @@ -1449,7 +1449,6 @@ row_unlock_for_mysql( and clust_pcur, and we do not need to reposition the cursors. */ { - dict_index_t* index; btr_pcur_t* pcur = prebuilt->pcur; btr_pcur_t* clust_pcur = prebuilt->clust_pcur; trx_t* trx = prebuilt->trx; @@ -1474,9 +1473,7 @@ row_unlock_for_mysql( trx->op_info = "unlock_row"; - index = btr_pcur_get_btr_cur(pcur)->index; - - if (index != NULL && trx_new_rec_locks_contain(trx, index)) { + if (prebuilt->new_rec_locks >= 1) { mtr_start(&mtr); @@ -1492,22 +1489,9 @@ row_unlock_for_mysql( rec, prebuilt->select_lock_type); mtr_commit(&mtr); - - /* If the search was done through the clustered index, then - we have not used clust_pcur at all, and we must NOT try to - reset locks on clust_pcur. The values in clust_pcur may be - garbage! */ - - if (dict_index_is_clust(index)) { - - goto func_exit; - } } - index = btr_pcur_get_btr_cur(clust_pcur)->index; - - if (index != NULL && trx_new_rec_locks_contain(trx, index)) { - + if (prebuilt->new_rec_locks >= 2) { mtr_start(&mtr); /* Restore the cursor position and find the record */ @@ -1525,7 +1509,6 @@ row_unlock_for_mysql( mtr_commit(&mtr); } -func_exit: trx->op_info = ""; return(DB_SUCCESS); diff --git a/row/row0sel.c b/row/row0sel.c index ebd7bf4a2ce..fb1523d3370 100644 --- a/row/row0sel.c +++ b/row/row0sel.c @@ -3002,8 +3002,9 @@ row_sel_get_clust_rec_for_mysql( func_exit: *out_rec = clust_rec; - if (prebuilt->select_lock_type == LOCK_X) { - /* We may use the cursor in update: store its position */ + if (prebuilt->select_lock_type != LOCK_NONE) { + /* We may use the cursor in update or in unlock_row(): + store its position */ btr_pcur_store_position(prebuilt->clust_pcur, mtr); } @@ -3405,13 +3406,7 @@ row_search_for_mysql( is set or session is using a READ COMMITED isolation level. Then we are able to remove the record locks set here on an individual row. */ - - if ((srv_locks_unsafe_for_binlog - || trx->isolation_level == TRX_ISO_READ_COMMITTED) - && prebuilt->select_lock_type != LOCK_NONE) { - - trx_reset_new_rec_lock_info(trx); - } + prebuilt->new_rec_locks = 0; /*-------------------------------------------------------------*/ /* PHASE 1: Try to pop the row from the prefetch cache */ @@ -4056,6 +4051,12 @@ no_gap_lock: switch (err) { const rec_t* old_vers; case DB_SUCCESS: + if (srv_locks_unsafe_for_binlog + || trx->isolation_level == TRX_ISO_READ_COMMITTED) { + /* Note that a record of + prebuilt->index was locked. */ + prebuilt->new_rec_locks = 1; + } break; case DB_LOCK_WAIT: if (UNIV_LIKELY(prebuilt->row_read_type @@ -4086,7 +4087,7 @@ no_gap_lock: if (UNIV_LIKELY(trx->wait_lock != NULL)) { lock_cancel_waiting_and_release( trx->wait_lock); - trx_reset_new_rec_lock_info(trx); + prebuilt->new_rec_locks = 0; } else { mutex_exit(&kernel_mutex); @@ -4098,6 +4099,9 @@ no_gap_lock: ULINT_UNDEFINED, &heap); err = DB_SUCCESS; + /* Note that a record of + prebuilt->index was locked. */ + prebuilt->new_rec_locks = 1; break; } mutex_exit(&kernel_mutex); @@ -4246,6 +4250,15 @@ requires_clust_rec: goto next_rec; } + if ((srv_locks_unsafe_for_binlog + || trx->isolation_level == TRX_ISO_READ_COMMITTED) + && prebuilt->select_lock_type != LOCK_NONE) { + /* Note that both the secondary index record + and the clustered index record were locked. */ + ut_ad(prebuilt->new_rec_locks == 1); + prebuilt->new_rec_locks = 2; + } + if (UNIV_UNLIKELY(rec_get_deleted_flag(clust_rec, comp))) { /* The record is delete marked: we can skip it */ @@ -4375,13 +4388,7 @@ next_rec: prebuilt->row_read_type = ROW_READ_TRY_SEMI_CONSISTENT; } did_semi_consistent_read = FALSE; - - if (UNIV_UNLIKELY(srv_locks_unsafe_for_binlog - || trx->isolation_level == TRX_ISO_READ_COMMITTED) - && prebuilt->select_lock_type != LOCK_NONE) { - - trx_reset_new_rec_lock_info(trx); - } + prebuilt->new_rec_locks = 0; /*-------------------------------------------------------------*/ /* PHASE 5: Move the cursor to the next index record */ @@ -4487,7 +4494,7 @@ lock_wait_or_error: rec_loop we will again try to set a lock, and new_rec_lock_info in trx will be right at the end. */ - trx_reset_new_rec_lock_info(trx); + prebuilt->new_rec_locks = 0; } mode = pcur->search_mode; diff --git a/trx/trx0trx.c b/trx/trx0trx.c index 3cf21000f9b..fa58e6d3576 100644 --- a/trx/trx0trx.c +++ b/trx/trx0trx.c @@ -183,8 +183,6 @@ trx_create( trx->autoinc_locks = ib_vector_create( mem_heap_create(sizeof(ib_vector_t) + sizeof(void*) * 4), 4); - trx_reset_new_rec_lock_info(trx); - return(trx); }