List:Commits« Previous MessageNext Message »
From:marko.makela Date:May 24 2010 11:04am
Subject:bzr commit into mysql-5.1-innodb branch (marko.makela:3479) Bug#53578
View as plain text  
#At file:///home/marko/innobase/dev/mysql2a/5.1-innodb/ based on revid:vasil.dimov@strippedq3pp0vsinac

 3479 Marko Mäkelä	2010-05-24
      Bug#53578: assert on invalid page access, in fil_io()
      
      Store the max_space_id in the data dictionary header in order to avoid
      space_id reuse.
      
      DICT_HDR_MIX_ID: Renamed to DICT_HDR_MAX_SPACE_ID, DICT_HDR_MIX_ID_LOW.
      
      dict_hdr_get_new_id(): Return table_id, index_id, space_id or a subset of them.
      
      fil_system_t: Add ibool space_id_reuse_warned.
      
      fil_create_new_single_table_tablespace(): Get the space_id from the caller.
      
      fil_space_create(): Issue a warning if the fil_system->max_assigned_id
      is exceeded.
      
      fil_assign_new_space_id(): Return TRUE/FALSE and take a pointer to the
      space_id as a parameter. Make the function public.
      
      fil_init(): Initialize all fil_system fields by mem_zalloc(). Remove
      explicit initializations of certain fields to 0 or NULL.

    modified:
      storage/innodb_plugin/dict/dict0boot.c
      storage/innodb_plugin/dict/dict0crea.c
      storage/innodb_plugin/fil/fil0fil.c
      storage/innodb_plugin/include/dict0boot.h
      storage/innodb_plugin/include/fil0fil.h
      storage/innodb_plugin/row/row0mysql.c
=== modified file 'storage/innodb_plugin/dict/dict0boot.c'
--- a/storage/innodb_plugin/dict/dict0boot.c	revid:vasil.dimov@strippedp0vsinac
+++ b/storage/innodb_plugin/dict/dict0boot.c	revid:marko.makela@stripped9
@@ -62,32 +62,47 @@ dict_hdr_get(
 }
 
 /**********************************************************************//**
-Returns a new table, index, or tree id.
-@return	the new id */
+Returns a new table, index, or space id. */
 UNIV_INTERN
-dulint
+void
 dict_hdr_get_new_id(
 /*================*/
-	ulint	type)	/*!< in: DICT_HDR_ROW_ID, ... */
+	dulint*	table_id,	/*!< out: table id (not assigned if NULL) */
+	dulint*	index_id,	/*!< out: index id (not assigned if NULL) */
+	ulint*	space_id)	/*!< out: space id (not assigned if NULL) */
 {
 	dict_hdr_t*	dict_hdr;
 	dulint		id;
 	mtr_t		mtr;
 
-	ut_ad((type == DICT_HDR_TABLE_ID) || (type == DICT_HDR_INDEX_ID));
-
 	mtr_start(&mtr);
 
 	dict_hdr = dict_hdr_get(&mtr);
 
-	id = mtr_read_dulint(dict_hdr + type, &mtr);
-	id = ut_dulint_add(id, 1);
-
-	mlog_write_dulint(dict_hdr + type, id, &mtr);
+	if (table_id) {
+		id = mtr_read_dulint(dict_hdr + DICT_HDR_TABLE_ID, &mtr);
+		id = ut_dulint_add(id, 1);
+		mlog_write_dulint(dict_hdr + DICT_HDR_TABLE_ID, id, &mtr);
+		*table_id = id;
+	}
+
+	if (index_id) {
+		id = mtr_read_dulint(dict_hdr + DICT_HDR_INDEX_ID, &mtr);
+		id = ut_dulint_add(id, 1);
+		mlog_write_dulint(dict_hdr + DICT_HDR_INDEX_ID, id, &mtr);
+		*index_id = id;
+	}
+
+	if (space_id) {
+		*space_id = mtr_read_ulint(dict_hdr + DICT_HDR_MAX_SPACE_ID,
+					   MLOG_4BYTES, &mtr);
+		if (fil_assign_new_space_id(space_id)) {
+			mlog_write_ulint(dict_hdr + DICT_HDR_MAX_SPACE_ID,
+					 *space_id, MLOG_4BYTES, &mtr);
+		}
+	}
 
 	mtr_commit(&mtr);
-
-	return(id);
 }
 
 /**********************************************************************//**
@@ -151,9 +166,12 @@ dict_hdr_create(
 	mlog_write_dulint(dict_header + DICT_HDR_INDEX_ID,
 			  ut_dulint_create(0, DICT_HDR_FIRST_ID), mtr);
 
-	/* Obsolete, but we must initialize it to 0 anyway. */
-	mlog_write_dulint(dict_header + DICT_HDR_MIX_ID,
-			  ut_dulint_create(0, DICT_HDR_FIRST_ID), mtr);
+	mlog_write_ulint(dict_header + DICT_HDR_MAX_SPACE_ID,
+			 0, MLOG_4BYTES, mtr);
+
+	/* Obsolete, but we must initialize it anyway. */
+	mlog_write_ulint(dict_header + DICT_HDR_MIX_ID_LOW,
+			 DICT_HDR_FIRST_ID, MLOG_4BYTES, mtr);
 
 	/* Create the B-tree roots for the clustered indexes of the basic
 	system tables */

=== modified file 'storage/innodb_plugin/dict/dict0crea.c'
--- a/storage/innodb_plugin/dict/dict0crea.c	revid:vasil.dimov@stripped-42uk5q3pp0vsinac
+++ b/storage/innodb_plugin/dict/dict0crea.c	revid:marko.makela@strippedrlmt07tzd9
@@ -239,16 +239,22 @@ dict_build_table_def_step(
 	const char*	path_or_name;
 	ibool		is_path;
 	mtr_t		mtr;
+	ulint		space = 0;
 
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 
 	table = node->table;
 
-	table->id = dict_hdr_get_new_id(DICT_HDR_TABLE_ID);
+	dict_hdr_get_new_id(&table->id, NULL,
+			    srv_file_per_table ? &space : NULL);
 
 	thr_get_trx(thr)->table_id = table->id;
 
 	if (srv_file_per_table) {
+		if (UNIV_UNLIKELY(space == ULINT_UNDEFINED)) {
+			return(DB_ERROR);
+		}
+
 		/* We create a new single-table tablespace for the table.
 		We initially let it be 4 pages:
 		- page 0 is the fsp header and an extent descriptor page,
@@ -257,8 +263,6 @@ dict_build_table_def_step(
 		- page 3 will contain the root of the clustered index of the
 		table we create here. */
 
-		ulint	space = 0;	/* reset to zero for the call below */
-
 		if (table->dir_path_of_temp_table) {
 			/* We place tables created with CREATE TEMPORARY
 			TABLE in the tmp dir of mysqld server */
@@ -276,7 +280,7 @@ dict_build_table_def_step(
 
 		flags = table->flags & ~(~0 << DICT_TF_BITS);
 		error = fil_create_new_single_table_tablespace(
-			&space, path_or_name, is_path,
+			space, path_or_name, is_path,
 			flags == DICT_TF_COMPACT ? 0 : flags,
 			FIL_IBD_FILE_INITIAL_SIZE);
 		table->space = (unsigned int) space;
@@ -561,7 +565,7 @@ dict_build_index_def_step(
 	ut_ad((UT_LIST_GET_LEN(table->indexes) > 0)
 	      || dict_index_is_clust(index));
 
-	index->id = dict_hdr_get_new_id(DICT_HDR_INDEX_ID);
+	dict_hdr_get_new_id(NULL, &index->id, NULL);
 
 	/* Inherit the space id from the table; we store all indexes of a
 	table in the same tablespace */

=== modified file 'storage/innodb_plugin/fil/fil0fil.c'
--- a/storage/innodb_plugin/fil/fil0fil.c	revid:vasil.dimov@strippedsinac
+++ b/storage/innodb_plugin/fil/fil0fil.c	revid:marko.makela@stripped
@@ -279,6 +279,10 @@ struct fil_system_struct {
 					request */
 	UT_LIST_BASE_NODE_T(fil_space_t) space_list;
 					/*!< list of all file spaces */
+	ibool		space_id_reuse_warned;
+					/* !< TRUE if fil_space_create()
+					has issued a warning about
+					potential space_id reuse */
 };
 
 /** The tablespace memory cache. This variable is NULL before the module is
@@ -1193,7 +1197,19 @@ try_again:
 	space->tablespace_version = fil_system->tablespace_version;
 	space->mark = FALSE;
 
-	if (purpose == FIL_TABLESPACE && id > fil_system->max_assigned_id) {
+	if (UNIV_LIKELY(purpose == FIL_TABLESPACE)
+	    && UNIV_UNLIKELY(id > fil_system->max_assigned_id)) {
+		if (!fil_system->space_id_reuse_warned) {
+			fil_system->space_id_reuse_warned = TRUE;
+
+			ut_print_timestamp(stderr);
+			fprintf(stderr,
+				"  InnoDB: Warning: allocated tablespace %lu,"
+				" old maximum was %lu\n",
+				(ulong) id,
+				(ulong) fil_system->max_assigned_id);
+		}
+
 		fil_system->max_assigned_id = id;
 	}
 
@@ -1231,19 +1247,25 @@ try_again:
 Assigns a new space id for a new single-table tablespace. This works simply by
 incrementing the global counter. If 4 billion id's is not enough, we may need
 to recycle id's.
-@return	new tablespace id; ULINT_UNDEFINED if could not assign an id */
-static
-ulint
-fil_assign_new_space_id(void)
-/*=========================*/
+@return	TRUE if assigned, FALSE if not */
+UNIV_INTERN
+ibool
+fil_assign_new_space_id(
+/*====================*/
+	ulint*	space_id)	/*!< in/out: space id */
 {
-	ulint		id;
+	ulint	id;
+	ibool	success;
 
 	mutex_enter(&fil_system->mutex);
 
-	fil_system->max_assigned_id++;
+	id = *space_id;
 
-	id = fil_system->max_assigned_id;
+	if (id < fil_system->max_assigned_id) {
+		id = fil_system->max_assigned_id;
+	}
+
+	id++;
 
 	if (id > (SRV_LOG_SPACE_FIRST_ID / 2) && (id % 1000000UL == 0)) {
 		ut_print_timestamp(stderr);
@@ -1259,7 +1281,11 @@ fil_assign_new_space_id(void)
 			(ulong) SRV_LOG_SPACE_FIRST_ID);
 	}
 
-	if (id >= SRV_LOG_SPACE_FIRST_ID) {
+	success = (id < SRV_LOG_SPACE_FIRST_ID);
+
+	if (success) {
+		*space_id = fil_system->max_assigned_id = id;
+	} else {
 		ut_print_timestamp(stderr);
 		fprintf(stderr,
 			"InnoDB: You have run out of single-table"
@@ -1269,14 +1295,12 @@ fil_assign_new_space_id(void)
 			" have to dump all your tables and\n"
 			"InnoDB: recreate the whole InnoDB installation.\n",
 			(ulong) id);
-		fil_system->max_assigned_id--;
-
-		id = ULINT_UNDEFINED;
+		*space_id = ULINT_UNDEFINED;
 	}
 
 	mutex_exit(&fil_system->mutex);
 
-	return(id);
+	return(success);
 }
 
 /*******************************************************************//**
@@ -1512,7 +1536,7 @@ fil_init(
 	ut_a(hash_size > 0);
 	ut_a(max_n_open > 0);
 
-	fil_system = mem_alloc(sizeof(fil_system_t));
+	fil_system = mem_zalloc(sizeof(fil_system_t));
 
 	mutex_create(&fil_system->mutex, SYNC_ANY_LATCH);
 
@@ -1521,16 +1545,7 @@ fil_init(
 
 	UT_LIST_INIT(fil_system->LRU);
 
-	fil_system->n_open = 0;
 	fil_system->max_n_open = max_n_open;
-
-	fil_system->modification_counter = 0;
-	fil_system->max_assigned_id = 0;
-
-	fil_system->tablespace_version = 0;
-
-	UT_LIST_INIT(fil_system->unflushed_spaces);
-	UT_LIST_INIT(fil_system->space_list);
 }
 
 /*******************************************************************//**
@@ -2115,7 +2130,7 @@ fil_op_log_parse_or_replay(
 			fil_create_directory_for_tablename(name);
 
 			if (fil_create_new_single_table_tablespace(
-				    &space_id, name, FALSE, flags,
+				    space_id, name, FALSE, flags,
 				    FIL_IBD_FILE_INITIAL_SIZE) != DB_SUCCESS) {
 				ut_error;
 			}
@@ -2562,9 +2577,7 @@ UNIV_INTERN
 ulint
 fil_create_new_single_table_tablespace(
 /*===================================*/
-	ulint*		space_id,	/*!< in/out: space id; if this is != 0,
-					then this is an input parameter,
-					otherwise output */
+	ulint		space_id,	/*!< in: space id */
 	const char*	tablename,	/*!< in: the table name in the usual
 					databasename/tablename format
 					of InnoDB, or a dir path to a temp
@@ -2584,6 +2597,8 @@ fil_create_new_single_table_tablespace(
 	ibool		success;
 	char*		path;
 
+	ut_a(space_id > 0);
+	ut_a(space_id < SRV_LOG_SPACE_FIRST_ID);
 	ut_a(size >= FIL_IBD_FILE_INITIAL_SIZE);
 	/* The tablespace flags (FSP_SPACE_FLAGS) should be 0 for
 	ROW_FORMAT=COMPACT
@@ -2640,38 +2655,21 @@ fil_create_new_single_table_tablespace(
 		return(DB_ERROR);
 	}
 
-	buf2 = ut_malloc(3 * UNIV_PAGE_SIZE);
-	/* Align the memory for file i/o if we might have O_DIRECT set */
-	page = ut_align(buf2, UNIV_PAGE_SIZE);
-
 	ret = os_file_set_size(path, file, size * UNIV_PAGE_SIZE, 0);
 
 	if (!ret) {
-		ut_free(buf2);
-		os_file_close(file);
-		os_file_delete(path);
-
-		mem_free(path);
-		return(DB_OUT_OF_FILE_SPACE);
-	}
-
-	if (*space_id == 0) {
-		*space_id = fil_assign_new_space_id();
-	}
-
-	/* printf("Creating tablespace %s id %lu\n", path, *space_id); */
-
-	if (*space_id == ULINT_UNDEFINED) {
-		ut_free(buf2);
+		err = DB_OUT_OF_FILE_SPACE;
 error_exit:
 		os_file_close(file);
 error_exit2:
 		os_file_delete(path);
 
 		mem_free(path);
-		return(DB_ERROR);
+		return(err);
 	}
 
+	/* printf("Creating tablespace %s id %lu\n", path, space_id); */
+
 	/* We have to write the space id to the file immediately and flush the
 	file to disk. This is because in crash recovery we must be aware what
 	tablespaces exist and what are their space id's, so that we can apply
@@ -2681,10 +2679,14 @@ error_exit2:
 	with zeros from the call of os_file_set_size(), until a buffer pool
 	flush would write to it. */
 
+	buf2 = ut_malloc(3 * UNIV_PAGE_SIZE);
+	/* Align the memory for file i/o if we might have O_DIRECT set */
+	page = ut_align(buf2, UNIV_PAGE_SIZE);
+
 	memset(page, '\0', UNIV_PAGE_SIZE);
 
-	fsp_header_init_fields(page, *space_id, flags);
-	mach_write_to_4(page + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, *space_id);
+	fsp_header_init_fields(page, space_id, flags);
+	mach_write_to_4(page + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, space_id);
 
 	if (!(flags & DICT_TF_ZSSIZE_MASK)) {
 		buf_flush_init_for_writing(page, NULL, 0);
@@ -2715,6 +2717,7 @@ error_exit2:
 		      " to tablespace ", stderr);
 		ut_print_filename(stderr, path);
 		putc('\n', stderr);
+		err = DB_ERROR;
 		goto error_exit;
 	}
 
@@ -2724,22 +2727,20 @@ error_exit2:
 		fputs("InnoDB: Error: file flush of tablespace ", stderr);
 		ut_print_filename(stderr, path);
 		fputs(" failed\n", stderr);
+		err = DB_ERROR;
 		goto error_exit;
 	}
 
 	os_file_close(file);
 
-	if (*space_id == ULINT_UNDEFINED) {
-		goto error_exit2;
-	}
-
-	success = fil_space_create(path, *space_id, flags, FIL_TABLESPACE);
+	success = fil_space_create(path, space_id, flags, FIL_TABLESPACE);
 
 	if (!success) {
+		err = DB_ERROR;
 		goto error_exit2;
 	}
 
-	fil_node_create(path, size, *space_id, FALSE);
+	fil_node_create(path, size, space_id, FALSE);
 
 #ifndef UNIV_HOTBACKUP
 	{
@@ -2750,7 +2751,7 @@ error_exit2:
 		fil_op_write_log(flags
 				 ? MLOG_FILE_CREATE2
 				 : MLOG_FILE_CREATE,
-				 *space_id,
+				 space_id,
 				 is_temp ? MLOG_FILE_FLAG_TEMP : 0,
 				 flags,
 				 tablename, NULL, &mtr);

=== modified file 'storage/innodb_plugin/include/dict0boot.h'
--- a/storage/innodb_plugin/include/dict0boot.h	revid:vasil.dimov@strippedc
+++ b/storage/innodb_plugin/include/dict0boot.h	revid:marko.makela@stripped
@@ -46,13 +46,14 @@ dict_hdr_get(
 /*=========*/
 	mtr_t*	mtr);	/*!< in: mtr */
 /**********************************************************************//**
-Returns a new row, table, index, or tree id.
-@return	the new id */
+Returns a new table, index, or space id. */
 UNIV_INTERN
-dulint
+void
 dict_hdr_get_new_id(
 /*================*/
-	ulint	type);	/*!< in: DICT_HDR_ROW_ID, ... */
+	dulint*	table_id,	/*!< out: table id (not assigned if NULL) */
+	dulint*	index_id,	/*!< out: index id (not assigned if NULL) */
+	ulint*	space_id);	/*!< out: space id (not assigned if NULL) */
 /**********************************************************************//**
 Returns a new row id.
 @return	the new id */
@@ -119,7 +120,8 @@ dict_create(void);
 #define DICT_HDR_ROW_ID		0	/* The latest assigned row id */
 #define	DICT_HDR_TABLE_ID	8	/* The latest assigned table id */
 #define	DICT_HDR_INDEX_ID	16	/* The latest assigned index id */
-#define	DICT_HDR_MIX_ID		24	/* Obsolete, always 0. */
+#define DICT_HDR_MAX_SPACE_ID	24	/* The latest assigned space id, or 0*/
+#define	DICT_HDR_MIX_ID_LOW	28	/* Obsolete,always DICT_HDR_FIRST_ID */
 #define	DICT_HDR_TABLES		32	/* Root of the table index tree */
 #define	DICT_HDR_TABLE_IDS	36	/* Root of the table index tree */
 #define	DICT_HDR_COLUMNS	40	/* Root of the column index tree */

=== modified file 'storage/innodb_plugin/include/fil0fil.h'
--- a/storage/innodb_plugin/include/fil0fil.h	revid:vasil.dimov@stripped42uk5q3pp0vsinac
+++ b/storage/innodb_plugin/include/fil0fil.h	revid:marko.makela@strippedrlmt07tzd9
@@ -225,6 +225,16 @@ fil_space_create(
 				0 for uncompressed tablespaces */
 	ulint		purpose);/*!< in: FIL_TABLESPACE, or FIL_LOG if log */
 /*******************************************************************//**
+Assigns a new space id for a new single-table tablespace. This works simply by
+incrementing the global counter. If 4 billion id's is not enough, we may need
+to recycle id's.
+@return	TRUE if assigned, FALSE if not */
+UNIV_INTERN
+ibool
+fil_assign_new_space_id(
+/*====================*/
+	ulint*	space_id);	/*!< in/out: space id */
+/*******************************************************************//**
 Returns the size of the space in pages. The tablespace must be cached in the
 memory cache.
 @return	space size, 0 if space not found */
@@ -427,9 +437,7 @@ UNIV_INTERN
 ulint
 fil_create_new_single_table_tablespace(
 /*===================================*/
-	ulint*		space_id,	/*!< in/out: space id; if this is != 0,
-					then this is an input parameter,
-					otherwise output */
+	ulint		space_id,	/*!< in: space id */
 	const char*	tablename,	/*!< in: the table name in the usual
 					databasename/tablename format
 					of InnoDB, or a dir path to a temp

=== modified file 'storage/innodb_plugin/row/row0mysql.c'
--- a/storage/innodb_plugin/row/row0mysql.c	revid:vasil.dimov@stripped
+++ b/storage/innodb_plugin/row/row0mysql.c	revid:marko.makela@stripped
@@ -2427,7 +2427,7 @@ row_discard_tablespace_for_mysql(
 		goto funct_exit;
 	}
 
-	new_id = dict_hdr_get_new_id(DICT_HDR_TABLE_ID);
+	dict_hdr_get_new_id(&new_id, NULL, NULL);
 
 	/* Remove all locks except the table-level S and X locks. */
 	lock_remove_all_on_table(table, FALSE);
@@ -2789,10 +2789,11 @@ row_truncate_table_for_mysql(
 
 			dict_index_t*	index;
 
-			space = 0;
+			dict_hdr_get_new_id(NULL, NULL, &space);
 
-			if (fil_create_new_single_table_tablespace(
-				    &space, table->name, FALSE, flags,
+			if (space == ULINT_UNDEFINED
+			    || fil_create_new_single_table_tablespace(
+				    space, table->name, FALSE, flags,
 				    FIL_IBD_FILE_INITIAL_SIZE) != DB_SUCCESS) {
 				ut_print_timestamp(stderr);
 				fprintf(stderr,
@@ -2897,7 +2898,7 @@ next_rec:
 
 	mem_heap_free(heap);
 
-	new_id = dict_hdr_get_new_id(DICT_HDR_TABLE_ID);
+	dict_hdr_get_new_id(&new_id, NULL, NULL);
 
 	info = pars_info_create();
 

Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20100524110439-fazi70rlmt07tzd9.bundle
Thread
bzr commit into mysql-5.1-innodb branch (marko.makela:3479) Bug#53578marko.makela24 May