bzr push into mysql-5.1 branch (marko.makela:3603 to 3605) Bug#12948130

[marko.makela]

 3605 Marko Mäkelä	2011-09-08
      Bug#12948130 UNNECESSARY X-LOCKING OF ADAPTIVE HASH INDEX (BTR_SEARCH_LATCH)
      
      InnoDB acquires an x-latch on btr_search_latch for certain in-place updates
      that do affect the adaptive hash index. These operations do not really need
      to be protected by the btr_search_latch:
      
      * updating DB_TRX_ID
      * updating DB_ROLL_PTR
      * updating PAGE_MAX_TRX_ID
      * updating the delete-mark flag
      
      rb:750 approved by Sunny Bains

    modified:
      storage/innodb_plugin/ChangeLog
      storage/innodb_plugin/btr/btr0cur.c
      storage/innodb_plugin/include/page0page.h
      storage/innodb_plugin/include/row0upd.ic
 3604 Marko Mäkelä	2011-09-08
      Bug#11766591 59733: POSSIBLE DEADLOCK WHEN BUFFERED CHANGES ARE DISCARDED
      
      Tweak the faulty UNIV_SYNC_DEBUG diagnostics a little bit more.
      
      ibuf_add_free_page(): Lower the latching order of the newly allocated page
      only after acquiring the ibuf_mutex.

    modified:
      storage/innobase/ibuf/ibuf0ibuf.c
      storage/innodb_plugin/ibuf/ibuf0ibuf.c
 3603 Vasil Dimov	2011-09-07
      Use cursors for seeking records in SYS_FOREIGN and SYS_INDEXES from
      DROP_TABLE_PROC().
      
      With this change I observe a speedup from 6.2s to 0.1s when executing
      DROP_TABLE_PROC() during DROP TABLE with 512 foreign keys, like what
      is being done in innodb_bug56143.test
      
      This fixes "Bug#11765460 DROP TABLE USES INEFFICIENT METHODS TO REMOVE
      FKS/INDEXES FROM INNODB SYS TABLES"
      
      Reviewed by:	Marko

    modified:
      storage/innodb_plugin/row/row0mysql.c
=== modified file 'storage/innobase/ibuf/ibuf0ibuf.c'
--- a/storage/innobase/ibuf/ibuf0ibuf.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/ibuf/ibuf0ibuf.c	revid:[e-mail]-20110908131024-5bekg27tq8i059wz
@@ -1683,14 +1683,14 @@ ibuf_add_free_page(
 
 	page = buf_page_get(space, page_no, RW_X_LATCH, &mtr);
 
-#ifdef UNIV_SYNC_DEBUG
-	buf_page_dbg_add_level(page, SYNC_TREE_NODE_NEW);
-#endif /* UNIV_SYNC_DEBUG */
-
 	ibuf_enter();
 
 	mutex_enter(&ibuf_mutex);
 
+#ifdef UNIV_SYNC_DEBUG
+	buf_page_dbg_add_level(page, SYNC_TREE_NODE_NEW);
+#endif /* UNIV_SYNC_DEBUG */
+
 	root = ibuf_tree_root_get(ibuf_data, space, &mtr);
 
 	/* Add the page to the free list and update the ibuf size data */

=== modified file 'storage/innodb_plugin/ChangeLog'
--- a/storage/innodb_plugin/ChangeLog	revid:vasil.dimov@strippedhic
+++ b/storage/innodb_plugin/ChangeLog	revid:marko.makela@stripped
@@ -1,3 +1,8 @@
[phone]	The InnoDB Team
+
+	* btr/btr0cur.c, include/page0page.h, include/row0upd.ic:
+	Fix Bug#12948130 UNNECESSARY X-LOCKING OF ADAPTIVE HASH INDEX
+
 2011-09-06	The InnoDB Team
 
 	* buf/buf0buddy.c:

=== modified file 'storage/innodb_plugin/btr/btr0cur.c'
--- a/storage/innodb_plugin/btr/btr0cur.c	revid:vasil.dimov@stripped0110907145810-v98kldmho23vhhic
+++ b/storage/innodb_plugin/btr/btr0cur.c	revid:marko.makela@stripped024-5bekg27tq8i059wz
@@ -1727,6 +1727,7 @@ btr_cur_update_in_place(
 	roll_ptr_t	roll_ptr	= ut_dulint_zero;
 	trx_t*		trx;
 	ulint		was_delete_marked;
+	ibool		is_hashed;
 	mem_heap_t*	heap		= NULL;
 	ulint		offsets_[REC_OFFS_NORMAL_SIZE];
 	ulint*		offsets		= offsets_;
@@ -1768,7 +1769,21 @@ btr_cur_update_in_place(
 		return(err);
 	}
 
-	if (block->is_hashed) {
+	if (!(flags & BTR_KEEP_SYS_FLAG)) {
+		row_upd_rec_sys_fields(rec, NULL,
+				       index, offsets, trx, roll_ptr);
+	}
+
+	was_delete_marked = rec_get_deleted_flag(
+		rec, page_is_comp(buf_block_get_frame(block)));
+
+	is_hashed = block->is_hashed;
+
+	if (is_hashed) {
+		/* TO DO: Can we skip this if none of the fields
+		index->search_info->curr_n_fields
+		are being updated? */
+
 		/* The function row_upd_changes_ord_field_binary works only
 		if the update vector was built for a clustered index, we must
 		NOT call it if index is secondary */
@@ -1784,17 +1799,9 @@ btr_cur_update_in_place(
 		rw_lock_x_lock(&btr_search_latch);
 	}
 
-	if (!(flags & BTR_KEEP_SYS_FLAG)) {
-		row_upd_rec_sys_fields(rec, NULL,
-				       index, offsets, trx, roll_ptr);
-	}
-
-	was_delete_marked = rec_get_deleted_flag(
-		rec, page_is_comp(buf_block_get_frame(block)));
-
 	row_upd_rec_in_place(rec, index, offsets, update, page_zip);
 
-	if (block->is_hashed) {
+	if (is_hashed) {
 		rw_lock_x_unlock(&btr_search_latch);
 	}
 
@@ -2520,7 +2527,8 @@ btr_cur_parse_del_mark_set_clust_rec(
 
 		/* We do not need to reserve btr_search_latch, as the page
 		is only being recovered, and there cannot be a hash index to
-		it. */
+		it. Besides, these fields are being updated in place
+		and the adaptive hash index does not depend on them. */
 
 		btr_rec_set_deleted_flag(rec, page_zip, val);
 
@@ -2600,9 +2608,9 @@ btr_cur_del_mark_set_clust_rec(
 		return(err);
 	}
 
-	if (block->is_hashed) {
-		rw_lock_x_lock(&btr_search_latch);
-	}
+	/* The btr_search_latch is not needed here, because
+	the adaptive hash index does not depend on the delete-mark
+	and the delete-mark is being updated in place. */
 
 	page_zip = buf_block_get_page_zip(block);
 
@@ -2616,10 +2624,6 @@ btr_cur_del_mark_set_clust_rec(
 				       index, offsets, trx, roll_ptr);
 	}
 
-	if (block->is_hashed) {
-		rw_lock_x_unlock(&btr_search_latch);
-	}
-
 	btr_cur_del_mark_set_clust_rec_log(flags, rec, index, val, trx,
 					   roll_ptr, mtr);
 
@@ -2695,7 +2699,8 @@ btr_cur_parse_del_mark_set_sec_rec(
 
 		/* We do not need to reserve btr_search_latch, as the page
 		is only being recovered, and there cannot be a hash index to
-		it. */
+		it. Besides, the delete-mark flag is being updated in place
+		and the adaptive hash index does not depend on it. */
 
 		btr_rec_set_deleted_flag(rec, page_zip, val);
 	}
@@ -2743,16 +2748,11 @@ btr_cur_del_mark_set_sec_rec(
 	ut_ad(!!page_rec_is_comp(rec)
 	      == dict_table_is_comp(cursor->index->table));
 
-	if (block->is_hashed) {
-		rw_lock_x_lock(&btr_search_latch);
-	}
-
+	/* We do not need to reserve btr_search_latch, as the
+	delete-mark flag is being updated in place and the adaptive
+	hash index does not depend on it. */
 	btr_rec_set_deleted_flag(rec, buf_block_get_page_zip(block), val);
 
-	if (block->is_hashed) {
-		rw_lock_x_unlock(&btr_search_latch);
-	}
-
 	btr_cur_del_mark_set_sec_rec_log(rec, val, mtr);
 
 	return(DB_SUCCESS);
@@ -2772,8 +2772,11 @@ btr_cur_del_unmark_for_ibuf(
 					uncompressed */
 	mtr_t*		mtr)		/*!< in: mtr */
 {
-	/* We do not need to reserve btr_search_latch, as the page has just
-	been read to the buffer pool and there cannot be a hash index to it. */
+	/* We do not need to reserve btr_search_latch, as the page
+	has just been read to the buffer pool and there cannot be
+	a hash index to it.  Besides, the delete-mark flag is being
+	updated in place and the adaptive hash index does not depend
+	on it. */
 
 	btr_rec_set_deleted_flag(rec, page_zip, FALSE);
 

=== modified file 'storage/innodb_plugin/ibuf/ibuf0ibuf.c'
--- a/storage/innodb_plugin/ibuf/ibuf0ibuf.c	revid:vasil.dimov@stripped-20110907145810-v98kldmho23vhhic
+++ b/storage/innodb_plugin/ibuf/ibuf0ibuf.c	revid:marko.makela@stripped08131024-5bekg27tq8i059wz
@@ -1766,16 +1766,15 @@ ibuf_add_free_page(void)
 		block = buf_page_get(
 			IBUF_SPACE_ID, 0, page_no, RW_X_LATCH, &mtr);
 
-		buf_block_dbg_add_level(block, SYNC_IBUF_TREE_NODE_NEW);
+		ibuf_enter();
+
+		mutex_enter(&ibuf_mutex);
 
+		buf_block_dbg_add_level(block, SYNC_IBUF_TREE_NODE_NEW);
 
 		page = buf_block_get_frame(block);
 	}
 
-	ibuf_enter();
-
-	mutex_enter(&ibuf_mutex);
-
 	root = ibuf_tree_root_get(&mtr);
 
 	/* Add the page to the free list and update the ibuf size data */

=== modified file 'storage/innodb_plugin/include/page0page.h'
--- a/storage/innodb_plugin/include/page0page.h	revid:vasil.dimov@strippedo23vhhic
+++ b/storage/innodb_plugin/include/page0page.h	revid:marko.makela@stripped59wz
@@ -68,10 +68,7 @@ typedef	byte		page_header_t;
 #define PAGE_MAX_TRX_ID	 18	/* highest id of a trx which may have modified
 				a record on the page; a dulint; defined only
 				in secondary indexes and in the insert buffer
-				tree; NOTE: this may be modified only
-				when the thread has an x-latch to the page,
-				and ALSO an x-latch to btr_search_latch
-				if there is a hash index to the page! */
+				tree */
 #define PAGE_HEADER_PRIV_END 26	/* end of private data structure of the page
 				header which are set in a page create */
 /*----*/

=== modified file 'storage/innodb_plugin/include/row0upd.ic'
--- a/storage/innodb_plugin/include/row0upd.ic	revid:vasil.dimov@stripped
+++ b/storage/innodb_plugin/include/row0upd.ic	revid:[e-mail]-20110908131024-5bekg27tq8i059wz
@@ -157,11 +157,6 @@ row_upd_rec_sys_fields(
 {
 	ut_ad(dict_index_is_clust(index));
 	ut_ad(rec_offs_validate(rec, index, offsets));
-#ifdef UNIV_SYNC_DEBUG
-	if (!rw_lock_own(&btr_search_latch, RW_LOCK_EX)) {
-		ut_ad(!buf_block_align(rec)->is_hashed);
-	}
-#endif /* UNIV_SYNC_DEBUG */
 
 	if (UNIV_LIKELY_NULL(page_zip)) {
 		ulint	pos = dict_index_get_sys_col_pos(index, DATA_TRX_ID);

No bundle (reason: useless for push emails).