List:Commits« Previous MessageNext Message »
From:vasil.dimov Date:June 22 2010 4:32pm
Subject:bzr commit into mysql-5.1-innodb branch (vasil.dimov:3517) Bug#47991
View as plain text  
#At file:///usr/local/devel/bzrroot/server/mysql-5.1-innodb/ based on revid:marko.makela@stripped

 3517 Vasil Dimov	2010-06-22
      Fix Bug#47991 InnoDB Dictionary Cache memory usage increases indefinitely
      when renaming tables
      
      Allocate the table name using ut_malloc() instead of table->heap because
      the latter cannot be freed.
      
      Adjust dict_sys->size calculations all over the code.
      
      Change dict_table_t::name from const char* to char* because we need to
      ut_malloc()/ut_free() it.
      
      Reviewed by:	Inaam, Marko, Heikki (rb://384)
      Approved by:	Heikki (rb://384)

    modified:
      storage/innodb_plugin/dict/dict0dict.c
      storage/innodb_plugin/dict/dict0mem.c
      storage/innodb_plugin/include/dict0mem.h
      storage/innodb_plugin/include/univ.i
      storage/innodb_plugin/page/page0zip.c
      storage/innodb_plugin/row/row0merge.c
=== modified file 'storage/innodb_plugin/dict/dict0dict.c'
--- a/storage/innodb_plugin/dict/dict0dict.c	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/dict/dict0dict.c	revid:vasil.dimov@stripped
@@ -850,7 +850,8 @@ dict_table_add_to_cache(
 	/* Add table to LRU list of tables */
 	UT_LIST_ADD_FIRST(table_LRU, dict_sys->table_LRU, table);
 
-	dict_sys->size += mem_heap_get_size(table->heap);
+	dict_sys->size += mem_heap_get_size(table->heap)
+		+ strlen(table->name) + 1;
 }
 
 /**********************************************************************//**
@@ -904,14 +905,21 @@ dict_table_rename_in_cache(
 	dict_foreign_t*	foreign;
 	dict_index_t*	index;
 	ulint		fold;
-	ulint		old_size;
-	const char*	old_name;
+	char		old_name[MAX_TABLE_NAME_LEN + 1];
 
 	ut_ad(table);
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 
-	old_size = mem_heap_get_size(table->heap);
-	old_name = table->name;
+	/* store the old/current name to an automatic variable */
+	if (strlen(table->name) + 1 <= sizeof(old_name)) {
+		memcpy(old_name, table->name, strlen(table->name) + 1);
+	} else {
+		ut_print_timestamp(stderr);
+		fprintf(stderr, "InnoDB: too long table name: '%s', "
+			"max length is %d\n", table->name,
+			MAX_TABLE_NAME_LEN);
+		ut_error;
+	}
 
 	fold = ut_fold_string(new_name);
 
@@ -957,12 +965,22 @@ dict_table_rename_in_cache(
 	/* Remove table from the hash tables of tables */
 	HASH_DELETE(dict_table_t, name_hash, dict_sys->table_hash,
 		    ut_fold_string(old_name), table);
-	table->name = mem_heap_strdup(table->heap, new_name);
+
+	if (strlen(new_name) > strlen(table->name)) {
+		/* We allocate MAX_TABLE_NAME_LEN+1 bytes here to avoid
+		memory fragmentation, we assume a repeated calls of
+		ut_realloc() with the same size do not cause fragmentation */
+		ut_a(strlen(new_name) <= MAX_TABLE_NAME_LEN);
+		table->name = ut_realloc(table->name, MAX_TABLE_NAME_LEN + 1);
+	}
+	memcpy(table->name, new_name, strlen(new_name) + 1);
 
 	/* Add table to hash table of tables */
 	HASH_INSERT(dict_table_t, name_hash, dict_sys->table_hash, fold,
 		    table);
-	dict_sys->size += (mem_heap_get_size(table->heap) - old_size);
+
+	dict_sys->size += strlen(new_name) - strlen(old_name);
+	ut_a(dict_sys->size > 0);
 
 	/* Update the table_name field in indexes */
 	index = dict_table_get_first_index(table);
@@ -1187,7 +1205,7 @@ dict_table_remove_from_cache(
 	/* Remove table from LRU list of tables */
 	UT_LIST_REMOVE(table_LRU, dict_sys->table_LRU, table);
 
-	size = mem_heap_get_size(table->heap);
+	size = mem_heap_get_size(table->heap) + strlen(table->name) + 1;
 
 	ut_ad(dict_sys->size >= size);
 

=== modified file 'storage/innodb_plugin/dict/dict0mem.c'
--- a/storage/innodb_plugin/dict/dict0mem.c	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/dict/dict0mem.c	revid:vasil.dimov@stripped
@@ -68,7 +68,8 @@ dict_mem_table_create(
 	table->heap = heap;
 
 	table->flags = (unsigned int) flags;
-	table->name = mem_heap_strdup(heap, name);
+	table->name = ut_malloc(strlen(name) + 1);
+	memcpy(table->name, name, strlen(name) + 1);
 	table->space = (unsigned int) space;
 	table->n_cols = (unsigned int) (n_cols + DATA_N_SYS_COLS);
 
@@ -106,6 +107,7 @@ dict_mem_table_free(
 #ifndef UNIV_HOTBACKUP
 	mutex_free(&(table->autoinc_mutex));
 #endif /* UNIV_HOTBACKUP */
+	ut_free(table->name);
 	mem_heap_free(table->heap);
 }
 

=== modified file 'storage/innodb_plugin/include/dict0mem.h'
--- a/storage/innodb_plugin/include/dict0mem.h	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/include/dict0mem.h	revid:vasil.dimov@stripped
@@ -382,7 +382,7 @@ initialized to 0, NULL or FALSE in dict_
 struct dict_table_struct{
 	dulint		id;	/*!< id of the table */
 	mem_heap_t*	heap;	/*!< memory heap */
-	const char*	name;	/*!< table name */
+	char*		name;	/*!< table name */
 	const char*	dir_path_of_temp_table;/*!< NULL or the directory path
 				where a TEMPORARY table that was explicitly
 				created by a user should be placed if

=== modified file 'storage/innodb_plugin/include/univ.i'
--- a/storage/innodb_plugin/include/univ.i	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/include/univ.i	revid:vasil.dimov@stripped
@@ -290,6 +290,12 @@ management to ensure correct alignment f
 /* Maximum number of parallel threads in a parallelized operation */
 #define UNIV_MAX_PARALLELISM	32
 
+/* The maximum length of a table name. This is the MySQL limit and is
+defined in mysql_com.h like NAME_CHAR_LEN*SYSTEM_CHARSET_MBMAXLEN, the
+number does not include a terminating '\0'. InnoDB probably can handle
+longer names internally */
+#define MAX_TABLE_NAME_LEN	192
+
 /*
 			UNIVERSAL TYPE DEFINITIONS
 			==========================

=== modified file 'storage/innodb_plugin/page/page0zip.c'
--- a/storage/innodb_plugin/page/page0zip.c	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/page/page0zip.c	revid:vasil.dimov@stripped
@@ -1464,6 +1464,7 @@ page_zip_fields_free(
 		dict_table_t*	table = index->table;
 		mem_heap_free(index->heap);
 		mutex_free(&(table->autoinc_mutex));
+		ut_free(table->name);
 		mem_heap_free(table->heap);
 	}
 }

=== modified file 'storage/innodb_plugin/row/row0merge.c'
--- a/storage/innodb_plugin/row/row0merge.c	revid:marko.makela@stripped
+++ b/storage/innodb_plugin/row/row0merge.c	revid:vasil.dimov@stripped
@@ -2336,7 +2336,7 @@ row_merge_rename_tables(
 {
 	ulint		err	= DB_ERROR;
 	pars_info_t*	info;
-	const char*	old_name= old_table->name;
+	char		old_name[MAX_TABLE_NAME_LEN + 1];
 
 	ut_ad(trx->mysql_thread_id == os_thread_get_curr_id());
 	ut_ad(old_table != new_table);
@@ -2344,6 +2344,17 @@ row_merge_rename_tables(
 
 	ut_a(trx->dict_operation_lock_mode == RW_X_LATCH);
 
+	/* store the old/current name to an automatic variable */
+	if (strlen(old_table->name) + 1 <= sizeof(old_name)) {
+		memcpy(old_name, old_table->name, strlen(old_table->name) + 1);
+	} else {
+		ut_print_timestamp(stderr);
+		fprintf(stderr, "InnoDB: too long table name: '%s', "
+			"max length is %d\n", old_table->name,
+			MAX_TABLE_NAME_LEN);
+		ut_error;
+	}
+
 	trx->op_info = "renaming tables";
 
 	/* We use the private SQL parser of Innobase to generate the query


Attachment: [text/bzr-bundle] bzr/vasil.dimov@oracle.com-20100622163043-dc0lxy0byg74viet.bundle
Thread
bzr commit into mysql-5.1-innodb branch (vasil.dimov:3517) Bug#47991vasil.dimov22 Jun