List:Commits« Previous MessageNext Message »
From:marko.makela Date:May 4 2011 11:33am
Subject:bzr push into mysql-trunk-innodb branch (marko.makela:3618 to 3619)
Bug#12424846
View as plain text  
 3619 Marko Mäkelä	2011-05-04
      Bug #12424846 MULTI-VERSIONING UNNECESSARILY HOLDING PURGE_SYS->LATCH
      
      Multi-versioning concurrency control (MVCC) code acquires
      purge_sys->latch in S mode. This appears to be unnecessary. When MVCC
      is holding the clustered index page latch, other transactions or purge
      cannot modify that page. A rolling back transaction cannot remove its
      update_undo log before undoing all changes. A committing transaction
      will not remove its update_undo log (purge will). Purge cannot remove
      the update_undo log if there are transactions that could see the
      changes covered by it. MVCC stops when reaching the oldest version
      that it is allowed to see. Thus, there is no room for collision
      between MVCC and active transactions or purge.
      
      trx_undo_prev_version_build(): Assert that the purge_sys->latch is not
      being held by the caller. This is exactly the opposite from before.
      
      trx_undo_get_undo_rec(): Shortly acquire the purge_sys->latch for
      checking the purge_sys->view.
      
      trx_purge_update_undo_must_exist(): Replaced with inline calls to
      !read_view_sees_trx_id().
      
      row_vers_old_has_index_entry(), row_vers_build_for_consistent_read(),
      row_vers_build_for_semi_consistent_read(): No longer acquire
      purge_sys->latch.
      
      The purge_sys->latch seems to be necessary in some cases, and because
      S-lockers remain, it cannot be turned into a mutex.
      
      rb:654 approved by Sunny Bains

    modified:
      storage/innobase/include/trx0purge.h
      storage/innobase/include/trx0rec.h
      storage/innobase/row/row0vers.c
      storage/innobase/trx/trx0purge.c
      storage/innobase/trx/trx0rec.c
 3618 Jimmy Yang	2011-05-04 [merge]
      Merge from mysql-5.5-innodb to mysql-trunk-innodb

    modified:
      storage/innobase/os/os0file.c
=== modified file 'storage/innobase/include/trx0purge.h'
--- a/storage/innobase/include/trx0purge.h	revid:jimmy.yang@stripped-20110504072951-6hpqradjdn0r39ww
+++ b/storage/innobase/include/trx0purge.h	revid:marko.makela@stripped112929-3zqhpy29nlyl0sc0
@@ -52,17 +52,6 @@ trx_purge_get_log_from_hist(
 /*========================*/
 	fil_addr_t	node_addr);	/*!< in: file address of the history
 					list node of the log */
-/*****************************************************************//**
-Checks if trx_id is >= purge_view: then it is guaranteed that its update
-undo log still exists in the system.
-@return TRUE if is sure that it is preserved, also if the function
-returns FALSE, it is possible that the undo log still exists in the
-system */
-UNIV_INTERN
-ibool
-trx_purge_update_undo_must_exist(
-/*=============================*/
-	trx_id_t	trx_id);/*!< in: transaction id */
 /********************************************************************//**
 Creates the global purge system control structure and inits the history
 mutex. */

=== modified file 'storage/innobase/include/trx0rec.h'
--- a/storage/innobase/include/trx0rec.h	revid:jimmy.yang@strippeddn0r39ww
+++ b/storage/innobase/include/trx0rec.h	revid:marko.makela@stripped
@@ -240,10 +240,10 @@ trx_undo_get_undo_rec_low(
 	roll_ptr_t	roll_ptr,	/*!< in: roll pointer to record */
 	mem_heap_t*	heap);		/*!< in: memory heap where copied */
 /*******************************************************************//**
-Build a previous version of a clustered index record. This function checks
-that the caller has a latch on the index page of the clustered index record
-and an s-latch on the purge_view. This guarantees that the stack of versions
-is locked.
+Build a previous version of a clustered index record. The caller must
+hold a latch on the index page of the clustered index record, to
+guarantee that the stack of versions is locked all the way down to the
+purge_sys->view.
 @return DB_SUCCESS, or DB_MISSING_HISTORY if the previous version is
 earlier than purge_view, which means that it may have been removed */
 UNIV_INTERN

=== modified file 'storage/innobase/row/row0vers.c'
--- a/storage/innobase/row/row0vers.c	revid:jimmy.yang@oracle.com-20110504072951-6hpqradjdn0r39ww
+++ b/storage/innobase/row/row0vers.c	revid:marko.makela@stripped110504112929-3zqhpy29nlyl0sc0
@@ -113,8 +113,6 @@ row_vers_impl_x_locked_low(
 	modify rec, and does not necessarily have an implicit x-lock
 	on rec. */
 
-	mtr_s_lock(&purge_sys->latch, mtr);
-
 	for (version = clust_rec;; version = prev_version) {
 		ulint		err;
 		row_ext_t*	ext;
@@ -189,9 +187,9 @@ row_vers_impl_x_locked_low(
 		would require rec to be in a different state. */
 
 		/* The previous version of clust_rec must be
-		accessible, because clust_rec was not a fresh insert
-		and we are holding purge_sys->latch. There is no
-		guarantee that the transaction is still active. */
+		accessible, because clust_rec was not a fresh insert.
+		There is no guarantee that the transaction is still
+		active. */
 
 		ut_ad(err == DB_SUCCESS);
 
@@ -285,8 +283,10 @@ row_vers_impl_x_locked(
 
 	/* Search for the clustered index record. The latch on the
 	page of clust_rec locks the top of the stack of versions. The
-	bottom of the version stack will be locked by
-	purge_sys->latch. */
+	bottom of the version stack is not locked; oldest versions may
+	disappear by the fact that transactions may be committed and
+	collected by the purge. This is not a problem, because we are
+	only interested in active transactions. */
 
 	clust_rec = row_get_clust_rec(
 		BTR_SEARCH_LEAF, rec, index, &clust_index, &mtr);
@@ -334,15 +334,7 @@ row_vers_must_preserve_del_marked(
 
 	mtr_s_lock(&(purge_sys->latch), mtr);
 
-	if (trx_purge_update_undo_must_exist(trx_id)) {
-
-		/* A purge operation is not yet allowed to remove this
-		delete marked record */
-
-		return(TRUE);
-	}
-
-	return(FALSE);
+	return(!read_view_sees_trx_id(purge_sys->view, trx_id));
 }
 
 /*****************************************************************//**
@@ -382,7 +374,6 @@ row_vers_old_has_index_entry(
 #ifdef UNIV_SYNC_DEBUG
 	ut_ad(!rw_lock_own(&(purge_sys->latch), RW_LOCK_SHARED));
 #endif /* UNIV_SYNC_DEBUG */
-	mtr_s_lock(&(purge_sys->latch), mtr);
 
 	clust_index = dict_table_get_first_index(index->table);
 
@@ -395,7 +386,11 @@ row_vers_old_has_index_entry(
 	if (also_curr && !rec_get_deleted_flag(rec, comp)) {
 		row_ext_t*	ext;
 
-		/* The stack of versions is locked by mtr.
+		/* The top of the stack of versions is locked by the
+		mtr holding a latch on the page containing the
+		clustered index record. The bottom of the stack is
+		locked by the fact that the purge_sys->view must
+		'overtake' any read view of an active transaction.
 		Thus, it is safe to fetch the prefixes for
 		externally stored columns. */
 		row = row_build(ROW_COPY_POINTERS, clust_index,
@@ -535,7 +530,6 @@ row_vers_build_for_consistent_read(
 
 	ut_ad(!read_view_sees_trx_id(view, trx_id));
 
-	rw_lock_s_lock(&(purge_sys->latch));
 	version = rec;
 
 	for (;;) {
@@ -615,7 +609,6 @@ row_vers_build_for_consistent_read(
 	}/* for (;;) */
 
 	mem_heap_free(heap);
-	rw_lock_s_unlock(&(purge_sys->latch));
 
 	return(err);
 }
@@ -661,12 +654,6 @@ row_vers_build_for_semi_consistent_read(
 
 	ut_ad(rec_offs_validate(rec, index, *offsets));
 
-	rw_lock_s_lock(&(purge_sys->latch));
-	/* The S-latch on purge_sys prevents the purge view from
-	changing.  Thus, if we have an uncommitted transaction at
-	this point, then purge cannot remove its undo log even if
-	the transaction could commit now. */
-
 	version = rec;
 
 	for (;;) {
@@ -759,7 +746,6 @@ row_vers_build_for_semi_consistent_read(
 	if (heap) {
 		mem_heap_free(heap);
 	}
-	rw_lock_s_unlock(&(purge_sys->latch));
 
 	return(err);
 }

=== modified file 'storage/innobase/trx/trx0purge.c'
--- a/storage/innobase/trx/trx0purge.c	revid:jimmy.yang@stripped
+++ b/storage/innobase/trx/trx0purge.c	revid:marko.makela@stripped20110504112929-3zqhpy29nlyl0sc0
@@ -80,25 +80,6 @@ trx_purge_fetch_next_rec(
 					handled */
 	mem_heap_t*	heap);		/*!< in: memory heap where copied */
 
-/*****************************************************************//**
-Checks if trx_id is >= purge_view: then it is guaranteed that its update
-undo log still exists in the system.
-@return TRUE if is sure that it is preserved, also if the function
-returns FALSE, it is possible that the undo log still exists in the
-system */
-UNIV_INTERN
-ibool
-trx_purge_update_undo_must_exist(
-/*=============================*/
-	trx_id_t	trx_id)	/*!< in: transaction id */
-{
-#ifdef UNIV_SYNC_DEBUG
-	ut_ad(rw_lock_own(&(purge_sys->latch), RW_LOCK_SHARED));
-#endif /* UNIV_SYNC_DEBUG */
-
-	return(!read_view_sees_trx_id(purge_sys->view, trx_id));
-}
-
 /****************************************************************//**
 Builds a purge 'query' graph. The actual purge is performed by executing
 this query graph.

=== modified file 'storage/innobase/trx/trx0rec.c'
--- a/storage/innobase/trx/trx0rec.c	revid:jimmy.yang@strippedom-20110504072951-6hpqradjdn0r39ww
+++ b/storage/innobase/trx/trx0rec.c	revid:marko.makela@stripped29-3zqhpy29nlyl0sc0
@@ -36,6 +36,7 @@ Created 3/26/1996 Heikki Tuuri
 #ifndef UNIV_HOTBACKUP
 #include "dict0dict.h"
 #include "ut0mem.h"
+#include "read0read.h"
 #include "row0ext.h"
 #include "row0upd.h"
 #include "que0que.h"
@@ -1383,11 +1384,13 @@ trx_undo_get_undo_rec(
 	trx_undo_rec_t** undo_rec,	/*!< out, own: copy of the record */
 	mem_heap_t*	heap)		/*!< in: memory heap where copied */
 {
-#ifdef UNIV_SYNC_DEBUG
-	ut_ad(rw_lock_own(&(purge_sys->latch), RW_LOCK_SHARED));
-#endif /* UNIV_SYNC_DEBUG */
+	ibool		missing_history;
+
+	rw_lock_s_lock(&purge_sys->latch);
+	missing_history = read_view_sees_trx_id(purge_sys->view, trx_id);
+	rw_lock_s_unlock(&purge_sys->latch);
 
-	if (!trx_purge_update_undo_must_exist(trx_id)) {
+	if (UNIV_UNLIKELY(missing_history)) {
 
 		/* It may be that the necessary undo log has already been
 		deleted */
@@ -1407,10 +1410,10 @@ trx_undo_get_undo_rec(
 #endif /* UNIV_DEBUG */
 
 /*******************************************************************//**
-Build a previous version of a clustered index record. This function checks
-that the caller has a latch on the index page of the clustered index record
-and an s-latch on the purge_view. This guarantees that the stack of versions
-is locked all the way down to the purge_view.
+Build a previous version of a clustered index record. The caller must
+hold a latch on the index page of the clustered index record, to
+guarantee that the stack of versions is locked all the way down to the
+purge_sys->view.
 @return DB_SUCCESS, or DB_MISSING_HISTORY if the previous version is
 earlier than purge_view, which means that it may have been removed */
 UNIV_INTERN
@@ -1451,7 +1454,7 @@ trx_undo_prev_version_build(
 	byte*		buf;
 	ulint		err;
 #ifdef UNIV_SYNC_DEBUG
-	ut_ad(rw_lock_own(&(purge_sys->latch), RW_LOCK_SHARED));
+	ut_ad(!rw_lock_own(&purge_sys->latch, RW_LOCK_SHARED));
 #endif /* UNIV_SYNC_DEBUG */
 	ut_ad(mtr_memo_contains_page(index_mtr, index_rec, MTR_MEMO_PAGE_S_FIX)
 	      || mtr_memo_contains_page(index_mtr, index_rec,

Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20110504112929-3zqhpy29nlyl0sc0.bundle
Thread
bzr push into mysql-trunk-innodb branch (marko.makela:3618 to 3619)Bug#12424846marko.makela4 May