Merge lp:~percona-dev/percona-server/bug677407 into lp:~percona-dev/percona-server/release-5.1.53

[Alexey Kopytov]

Fix for LP bug #677407.

[Vadim Tkachenko (vadim-tk)]

looks good, but requires docs entry

review: Needs Fixing

[Fred Linhoss (fred-linhoss)]

Will be in release notes for 5.1.53-11.7.

review: Approve (documentation)

[Vadim Tkachenko (vadim-tk)]

good

review: Approve

=== added file 'bug677407.patch'
--- bug677407.patch	1970-01-01 00:00:00 +0000
+++ bug677407.patch	2010-11-30 16:59:10 +0000
@@ -0,0 +1,138 @@
+# name       : bug677407
+# introduced : 13
+# maintainer : Alexey
+
+LP bug#677407 / MySQL Bug#48883: Stale data from INNODB_LOCKS table.
+
+The innodb.innodb_information_schema test could fail sporadically due to
+the flawed logic in the INFORMATION_SCHEMA.INNODB_LOCKS caching
+mechanism.
+
+The logic for how to check when to update the table cache for
+INNODB_LOCKS with real data was flawed. This could result in both not
+updating the cache often enough (when the table is queried repeatedly
+with less than 100 milliseconds in-between) resulting in stale data; as
+well as updating too often (when multiple queries against the table
+start at around the same time).
+
+Fixed by updating the "last updated" timestamp in the right place, when
+the cache is updated, not when it is read.
+
+I hereby grant Percona the following BSD license to this patch for
+inclusion in Percona Server:
+
+Copyright (c) 2010, Kristian Nielsen All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the
+      distribution.
+    * Neither the name of the author nor the names of its contributors
+      may be used to endorse or promote products derived from this
+      software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+--- a/storage/innodb_plugin/trx/trx0i_s.c	2010-11-03 16:39:54.000000000 +0300
++++ b/storage/innodb_plugin/trx/trx0i_s.c	2010-11-30 13:57:03.000000000 +0300
+@@ -157,10 +157,6 @@
+ 	ullint		last_read;	/*!< last time the cache was read;
+ 					measured in microseconds since
+ 					epoch */
+-	mutex_t		last_read_mutex;/*!< mutex protecting the
+-					last_read member - it is updated
+-					inside a shared lock of the
+-					rw_lock member */
+ 	i_s_table_cache_t innodb_trx;	/*!< innodb_trx table */
+ 	i_s_table_cache_t innodb_locks;	/*!< innodb_locks table */
+ 	i_s_table_cache_t innodb_lock_waits;/*!< innodb_lock_waits table */
+@@ -1101,13 +1097,6 @@
+ {
+ 	ullint	now;
+ 
+-	/* Here we read cache->last_read without acquiring its mutex
+-	because last_read is only updated when a shared rw lock on the
+-	whole cache is being held (see trx_i_s_cache_end_read()) and
+-	we are currently holding an exclusive rw lock on the cache.
+-	So it is not possible for last_read to be updated while we are
+-	reading it. */
+-
+ #ifdef UNIV_SYNC_DEBUG
+ 	ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX));
+ #endif
+@@ -1205,6 +1194,12 @@
+ /*===================================*/
+ 	trx_i_s_cache_t*	cache)	/*!< in/out: cache */
+ {
++	ullint	now;
++
++#ifdef UNIV_SYNC_DEBUG
++	ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX));
++#endif
++
+ 	if (!can_cache_be_updated(cache)) {
+ 
+ 		return(1);
+@@ -1217,6 +1212,10 @@
+ 
+ 	mutex_exit(&kernel_mutex);
+ 
++	/* update cache last read time */
++	now = ut_time_us(NULL);
++	cache->last_read = now;
++
+ 	return(0);
+ }
+ 
+@@ -1247,16 +1246,12 @@
+ 	release kernel_mutex
+ 	release trx_i_s_cache_t::rw_lock
+ 	acquire trx_i_s_cache_t::rw_lock, S
+-	acquire trx_i_s_cache_t::last_read_mutex
+-	release trx_i_s_cache_t::last_read_mutex
+ 	release trx_i_s_cache_t::rw_lock */
+ 
+ 	rw_lock_create(&cache->rw_lock, SYNC_TRX_I_S_RWLOCK);
+ 
+ 	cache->last_read = 0;
+ 
+-	mutex_create(&cache->last_read_mutex, SYNC_TRX_I_S_LAST_READ);
+-
+ 	table_cache_init(&cache->innodb_trx, sizeof(i_s_trx_row_t));
+ 	table_cache_init(&cache->innodb_locks, sizeof(i_s_locks_row_t));
+ 	table_cache_init(&cache->innodb_lock_waits,
+@@ -1307,18 +1302,10 @@
+ /*===================*/
+ 	trx_i_s_cache_t*	cache)	/*!< in: cache */
+ {
+-	ullint	now;
+-
+ #ifdef UNIV_SYNC_DEBUG
+ 	ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_SHARED));
+ #endif
+ 
+-	/* update cache last read time */
+-	now = ut_time_us(NULL);
+-	mutex_enter(&cache->last_read_mutex);
+-	cache->last_read = now;
+-	mutex_exit(&cache->last_read_mutex);
+-
+ 	rw_lock_s_unlock(&cache->rw_lock);
+ }
+ 

=== modified file 'series'
--- series	2010-11-17 15:52:39 +0000
+++ series	2010-11-30 16:59:10 +0000
@@ -50,3 +50,4 @@
 sql_no_fcache.patch
 show_slave_status_nolock.patch
 innodb_fast_shutdown.patch
+bug677407.patch