Merge lp:~akopytov/percona-server/bug1170103-5.5 into lp:percona-server/5.5

[Alexey Kopytov]

Since no GCA merge is possible, the branch is based on release-5.5.30-30.2.

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/723/

[Laurynas Biveinis (laurynas-biveinis)]

The code is OK.

I am wondering about testing. I suspect that there should be existing MTR tests that would report this error under Valgrind. But as our awesome Jenkins setup Valgrind builds are broken since February 13th, I am waiting for some local tests to complete. Perhaps you have looked into this already? If there are none, perhaps we should add one?

review: Needs Information

[Laurynas Biveinis (laurynas-biveinis)]

I tried to grep the testsuite for any tests that have InnoDB and query cache enabled. I found three, out of which innodb.innodb, innodb.innodb_misc1 are Valgrind-clean with the bug present. innodb.innodb_mysql crashes.

[lp:~akopytov/percona-server/bug1170103-5.5]

503. By Alexey Kopytov on 2013-04-27

Bug #1170103: Memory leak @ read_view_open_now...

row_search_check_if_query_cache_permitted() did not use the prebuilt
read view and allocated a new one on each call instead.

Fixed by reusing the prebuilt view appropriately.

This revision also adds additional status variables, should we have to
diagnose similar problems in future.

[Alexey Kopytov (akopytov)]

Added a minimal test case. http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/724/

[Laurynas Biveinis (laurynas-biveinis)]

OK, thank you.

review: Approve

[Laurynas Biveinis (laurynas-biveinis)]

Just noticed that it's "work in progress." Please set to Approved yourself is that was the wrong status, or ping me if a re-review is needed at a later time.

=== added file 'Percona-Server/mysql-test/t/percona_bug1170103.test'
--- Percona-Server/mysql-test/t/percona_bug1170103.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/percona_bug1170103.test	2013-04-27 10:07:32 +0000
@@ -0,0 +1,23 @@
+########################################################################
+# Byg #1170103: Memory leak @ read_view_open_now...
+########################################################################
+
+--source include/have_query_cache.inc
+--source include/have_innodb.inc
+
+CREATE TABLE t(a INT) ENGINE=InnoDB;
+INSERT INTO t VALUES (1), (2), (3);
+
+SET @old_query_cache_size=@@query_cache_size;
+SET @old_query_cache_type=@@query_cache_type;
+
+SET GLOBAL query_cache_size=1024*1024;
+SET GLOBAL query_cache_type=1;
+
+SELECT * FROM t;
+SELECT * FROM t;
+
+SET GLOBAL query_cache_size=@old_query_cache_size;
+SET GLOBAL query_cache_type=@old_query_cache_type;
+
+DROP TABLE t;
 
=== modified file 'Percona-Server/storage/innobase/handler/ha_innodb.cc'
--- Percona-Server/storage/innobase/handler/ha_innodb.cc	2013-04-11 21:03:27 +0000
+++ Percona-Server/storage/innobase/handler/ha_innodb.cc	2013-04-27 10:07:32 +0000
@@ -865,6 +865,10 @@
   (char*) &export_vars.innodb_rows_read,		  SHOW_LONG},
   {"rows_updated",
   (char*) &export_vars.innodb_rows_updated,		  SHOW_LONG},
+  {"read_views_memory",
+  (char*) &export_vars.innodb_read_views_memory,	  SHOW_LONG},
+  {"descriptors_memory",
+  (char*) &export_vars.innodb_descriptors_memory,	  SHOW_LONG},
   {"s_lock_os_waits",
   (char*) &export_vars.innodb_s_lock_os_waits,		  SHOW_LONGLONG},
   {"s_lock_spin_rounds",
 
=== modified file 'Percona-Server/storage/innobase/include/srv0srv.h'
--- Percona-Server/storage/innobase/include/srv0srv.h	2013-04-04 10:50:07 +0000
+++ Percona-Server/storage/innobase/include/srv0srv.h	2013-04-27 10:07:32 +0000
@@ -289,6 +289,9 @@
 extern ulint	srv_n_rows_deleted;
 extern ulint	srv_n_rows_read;
 
+extern ulint	srv_read_views_memory;
+extern ulint	srv_descriptors_memory;
+
 extern ibool	srv_print_innodb_monitor;
 extern ibool	srv_print_innodb_lock_monitor;
 extern ibool	srv_print_innodb_tablespace_monitor;
@@ -893,6 +896,8 @@
 	ulint innodb_rows_updated;		/*!< srv_n_rows_updated */
 	ulint innodb_rows_deleted;		/*!< srv_n_rows_deleted */
 	ulint innodb_truncated_status_writes;	/*!< srv_truncated_status_writes */
+	ulint innodb_read_views_memory;		/*!< srv_read_views_memory */
+	ulint innodb_descriptors_memory;	/*!< srv_descriptors_memory */
 	ib_int64_t innodb_s_lock_os_waits;
 	ib_int64_t innodb_s_lock_spin_rounds;
 	ib_int64_t innodb_s_lock_spin_waits;
 
=== modified file 'Percona-Server/storage/innobase/read/read0read.c'
--- Percona-Server/storage/innobase/read/read0read.c	2013-03-25 09:59:39 +0000
+++ Percona-Server/storage/innobase/read/read0read.c	2013-04-27 10:07:32 +0000
@@ -150,6 +150,7 @@
 {
 	if (view == NULL) {
 		view = ut_malloc(sizeof(read_view_t));
+		srv_read_views_memory += sizeof(read_view_t);
 		view->max_descr = 0;
 		view->descriptors = NULL;
 	}
@@ -159,6 +160,8 @@
 		/* avoid frequent re-allocations by extending the array to the
 		desired size + 10% */
 
+		srv_read_views_memory += (n + n / 10 - view->max_descr) *
+			sizeof(trx_id_t);
 		view->max_descr = n + n / 10;
 		view->descriptors = ut_realloc(view->descriptors,
 					       view->max_descr *
@@ -370,6 +373,9 @@
 {
 	ut_ad(mutex_own(&kernel_mutex));
 
+	srv_read_views_memory -= sizeof(read_view_t) +
+		view->max_descr * sizeof(trx_id_t);
+
 	if (view->descriptors != NULL) {
 		ut_free(view->descriptors);
 	}
 
=== modified file 'Percona-Server/storage/innobase/row/row0sel.c'
--- Percona-Server/storage/innobase/row/row0sel.c	2013-03-26 01:40:02 +0000
+++ Percona-Server/storage/innobase/row/row0sel.c	2013-04-27 10:07:32 +0000
@@ -4899,8 +4899,8 @@
 
 			trx->read_view =
 				read_view_open_now(trx->id,
-						   NULL, TRUE);
-
+						   trx->prebuilt_view, TRUE);
+			trx->prebuilt_view = trx->read_view;
 			trx->global_read_view = trx->read_view;
 		}
 	}
 
=== modified file 'Percona-Server/storage/innobase/srv/srv0srv.c'
--- Percona-Server/storage/innobase/srv/srv0srv.c	2013-04-04 10:50:07 +0000
+++ Percona-Server/storage/innobase/srv/srv0srv.c	2013-04-27 10:07:32 +0000
@@ -463,6 +463,9 @@
 UNIV_INTERN ulint		srv_n_rows_deleted CACHE_ALIGNED	= 0;
 UNIV_INTERN ulint		srv_n_rows_read CACHE_ALIGNED		= 0;
 
+UNIV_INTERN ulint		srv_read_views_memory CACHE_ALIGNED	= 0;
+UNIV_INTERN ulint		srv_descriptors_memory CACHE_ALIGNED	= 0;
+
 UNIV_INTERN ulint		srv_n_lock_deadlock_count CACHE_ALIGNED	= 0;
 UNIV_INTERN ulint		srv_n_lock_wait_count CACHE_ALIGNED	= 0;
 UNIV_INTERN ulint		srv_n_lock_wait_current_count CACHE_ALIGNED = 0;
@@ -2129,6 +2132,9 @@
 			"; in additional pool allocated " ULINTPF "\n",
 			ut_total_allocated_memory,
 			mem_pool_get_reserved(mem_comm_pool));
+	fprintf(file,
+		"Total memory allocated by read views " ULINTPF "\n",
+		srv_read_views_memory);
 	/* Calcurate reserved memories */
 	if (btr_search_sys && btr_search_sys->hash_index[0]->heap) {
 		btr_search_sys_subtotal = mem_heap_get_size(btr_search_sys->hash_index[0]->heap);
@@ -2215,6 +2221,12 @@
 	fprintf(file, "%lu read views open inside InnoDB\n",
 		UT_LIST_GET_LEN(trx_sys->view_list));
 
+	fprintf(file, "%lu transactions active inside InnoDB\n",
+		UT_LIST_GET_LEN(trx_sys->trx_list));
+
+	fprintf(file, "%lu out of %lu descriptors used\n",
+		trx_sys->descr_n_used, trx_sys->descr_n_max);
+
 	if (UT_LIST_GET_LEN(trx_sys->view_list)) {
 		read_view_t*	view = UT_LIST_GET_LAST(trx_sys->view_list);
 
@@ -2524,6 +2536,8 @@
 	export_vars.innodb_rows_updated = srv_n_rows_updated;
 	export_vars.innodb_rows_deleted = srv_n_rows_deleted;
 	export_vars.innodb_truncated_status_writes = srv_truncated_status_writes;
+	export_vars.innodb_read_views_memory = srv_read_views_memory;
+	export_vars.innodb_descriptors_memory = srv_descriptors_memory;
 
 #ifdef UNIV_DEBUG
 	if (trx_sys->max_trx_id < purge_sys->done_trx_no) {
 
=== modified file 'Percona-Server/storage/innobase/trx/trx0sys.c'
--- Percona-Server/storage/innobase/trx/trx0sys.c	2013-03-25 09:59:39 +0000
+++ Percona-Server/storage/innobase/trx/trx0sys.c	2013-04-27 10:07:32 +0000
@@ -1324,6 +1324,8 @@
 					 TRX_DESCR_ARRAY_INITIAL_SIZE);
 	trx_sys->descr_n_max = TRX_DESCR_ARRAY_INITIAL_SIZE;
 	trx_sys->descr_n_used = 0;
+	srv_descriptors_memory = TRX_DESCR_ARRAY_INITIAL_SIZE *
+		sizeof(trx_id_t);
 
 	sys_header = trx_sysf_get(&mtr);
 
 
=== modified file 'Percona-Server/storage/innobase/trx/trx0trx.c'
--- Percona-Server/storage/innobase/trx/trx0trx.c	2013-03-26 01:40:02 +0000
+++ Percona-Server/storage/innobase/trx/trx0trx.c	2013-04-27 10:07:32 +0000
@@ -135,6 +135,7 @@
 				   n_max * sizeof(trx_id_t));
 
 		trx_sys->descr_n_max = n_max;
+		srv_descriptors_memory = n_max * sizeof(trx_id_t);
 	}
 
 	descr = trx_sys->descriptors + n_used - 1;
 
=== modified file 'build/percona-server.spec'
--- build/percona-server.spec	2013-04-23 06:02:56 +0000
+++ build/percona-server.spec	2013-04-27 10:07:32 +0000
@@ -725,6 +725,17 @@
 
 if [ $1 -eq 1 ]; then
 # ----------------------------------------------------------------------
+# Create a MySQL user and group. Do not report any problems if it already
+# exists.
+# ----------------------------------------------------------------------
+groupadd -r %{mysqld_group} 2> /dev/null || true
+useradd -M -r -d $mysql_datadir -s /bin/bash -c "MySQL server" \
+  -g %{mysqld_group} %{mysqld_user} 2> /dev/null || true
+# The user may already exist, make sure it has the proper group nevertheless
+# (BUG#12823)
+usermod -g %{mysqld_group} %{mysqld_user} 2> /dev/null || true
+
+# ----------------------------------------------------------------------
 # Create data directory if needed, check whether upgrade or install
 # ----------------------------------------------------------------------
 if [ ! -d $mysql_datadir ] ; then mkdir -m 755 $mysql_datadir; fi
@@ -760,17 +771,6 @@
 fi
 
 # ----------------------------------------------------------------------
-# Create a MySQL user and group. Do not report any problems if it already
-# exists.
-# ----------------------------------------------------------------------
-groupadd -r %{mysqld_group} 2> /dev/null || true
-useradd -M -r -d $mysql_datadir -s /bin/bash -c "MySQL server" \
-  -g %{mysqld_group} %{mysqld_user} 2> /dev/null || true
-# The user may already exist, make sure it has the proper group nevertheless
-# (BUG#12823)
-usermod -g %{mysqld_group} %{mysqld_user} 2> /dev/null || true
-
-# ----------------------------------------------------------------------
 # Initiate databases if needed
 # ----------------------------------------------------------------------
 # ----------------------------------------------------------------------