Merge lp:~akopytov/percona-xtrabackup/bugs-1177206-and-1187071-2.0 into lp:percona-xtrabackup/2.0

[Alexey Kopytov]

    Bug #1177206: InnoDB: Assertion failure in thread <nr> in file
                  log0recv.c line 1105 InnoDB: Failing assertion: !page
                  || (ibool)!!page_is_comp(page) ==
                  dict_table_is_comp(index->table)

    Bug #1187071: xb 2.1 against ps 5.6 gives Error: Tablespace is not
                  sensible during backup"

    The problems are related, so fixing in a single revision.

    The problem in bug #1187071 was a difference in behavior between InnoDB
    5.5- and 5.6 codebases in cases when a newly created tablespace has
    uninitialized first page at the time when XtraBackup opens it while
    creating a list of tablespaces to backup. 5.1/5.5 InnoDB code would just
    complain about tablespace ID being non-sensible and omit it from the
    backup. But in 5.6 it fails with an error in such a case.

    A related problem in bug #1177206 was that
    xb_data_files_init() (i.e. loading tablespaces into fil_system) was
    called before log copying was started. This could lead to a race between
    the datafiles state in the resulting backup and xtrabackup_logfile: a
    tablespace created at a sensitive time would be missing in both the
    backup itself and as the corresponding MLOG_FILE_CREATE* log record in
    xtrabackup_logfile, so it would not be created on --apply-log
    either. This problem has existed since the very first XtraBackup
    version.

    Fixed by:

    1. Ignoring tablespaces with uninitialized pages on xtrabackup startup
    for InnoDB 5.6 codebase to match the behavior of InnoDB 5.5-.
    2. Moving the call to xb_data_files_init() in xtrabackup_backup_func()
    to a later stage so that it is only called after the log copying is
    started. This involved a minor code refactoring, since
    xb_data_files_init() previously did 2 things: initializing the 'fil'
    subsystem (which is also required by the log copying code) and actually
    populating fil_system. We now have 2 separate functions:
    xb_fil_io_init() (called before log copying is started) and
    xb_load_tablespaces() (called after log copying is started).

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/461/

[Laurynas Biveinis (laurynas-biveinis)]

Was

13 ++ fsp_open_info* fsp;
14 ++ ulong minimum_size;
15 ++ ibool file_space_create_success;

intentional?

It looks like it's unrelated to the fix, just increases the patch size.

review: Needs Information

[Laurynas Biveinis (laurynas-biveinis)]

Nevermind, I guess it's to fix "goto jumps past var initialization" warnings, isn't it?

review: Approve

[Alexey Kopytov (akopytov)]

Exactly. Was something like "jump to protected scope" with clang.

=== modified file 'patches/innodb56.patch'
--- patches/innodb56.patch	2013-05-16 18:01:50 +0000
+++ patches/innodb56.patch	2013-06-19 05:51:29 +0000
@@ -330,7 +330,17 @@
  
  		err = DB_CORRUPTION;
  
-@@ -4018,6 +4141,9 @@
+@@ -3974,6 +4097,9 @@
+ #ifdef UNIV_HOTBACKUP
+ 	fil_space_t*	space;
+ #endif
++	fsp_open_info*	fsp;
++	ulong		minimum_size;
++	ibool		file_space_create_success;
+ 
+ 	memset(&def, 0, sizeof(def));
+ 	memset(&remote, 0, sizeof(remote));
+@@ -4018,6 +4144,9 @@
  # endif /* !UNIV_HOTBACKUP */
  #endif
  
@@ -340,24 +350,76 @@
  	/* Check for a link file which locates a remote tablespace. */
  	remote.success = fil_open_linked_file(
  		tablename, &remote.filepath, &remote.file);
-@@ -4030,6 +4156,7 @@
+@@ -4028,8 +4157,20 @@
+ 		if (!remote.success) {
+ 			os_file_close(remote.file);
  			mem_free(remote.filepath);
++
++			if (srv_backup_mode) {
++
++				/* Ignore files that have uninitialized space
++				IDs on the backup stage. This means that a
++				tablespace has just been created and we will
++				replay the corresponding log records on
++				prepare. */
++
++				goto func_exit_after_close;
++			}
  		}
  	}
 +	}
  
  
  	/* Try to open the tablespace in the datadir. */
-@@ -4135,7 +4262,7 @@
+@@ -4042,6 +4183,17 @@
+ 		fil_validate_single_table_tablespace(tablename, &def);
+ 		if (!def.success) {
+ 			os_file_close(def.file);
++
++			if (srv_backup_mode) {
++
++				/* Ignore files that have uninitialized space
++				IDs on the backup stage. This means that a
++				tablespace has just been created and we will
++				replay the corresponding log records on
++				prepare. */
++
++				goto func_exit_after_close;
++			}
+ 		}
+ 	}
+ 
+@@ -4114,7 +4266,7 @@
+ 	/* At this point, only one tablespace is open */
+ 	ut_a(def.success == !remote.success);
+ 
+-	fsp_open_info*	fsp = def.success ? &def : &remote;
++	fsp = def.success ? &def : &remote;
+ 
+ 	/* Get and test the file size. */
+ 	size = os_file_get_size(fsp->file);
+@@ -4133,9 +4285,9 @@
+ 
+ 	/* Every .ibd file is created >= 4 pages in size. Smaller files
  	cannot be ok. */
- 	ulong minimum_size = FIL_IBD_FILE_INITIAL_SIZE * UNIV_PAGE_SIZE;
+-	ulong minimum_size = FIL_IBD_FILE_INITIAL_SIZE * UNIV_PAGE_SIZE;
++	minimum_size = FIL_IBD_FILE_INITIAL_SIZE * UNIV_PAGE_SIZE;
  	if (size < minimum_size) {
 -#ifndef UNIV_HOTBACKUP
 +#if 0
  		ib_logf(IB_LOG_LEVEL_ERROR,
  			"The size of single-table tablespace file %s "
  			"is only " UINT64PF ", should be at least %lu!",
-@@ -4243,7 +4370,51 @@
+@@ -4216,7 +4368,7 @@
+ 	}
+ 	mutex_exit(&fil_system->mutex);
+ #endif /* UNIV_HOTBACKUP */
+-	ibool file_space_create_success = fil_space_create(
++	file_space_create_success = fil_space_create(
+ 		tablename, fsp->id, fsp->flags, FIL_TABLESPACE);
+ 
+ 	if (!file_space_create_success) {
+@@ -4243,13 +4395,55 @@
  	}
  
  func_exit:
@@ -408,9 +470,16 @@
 +		os_file_close(fsp->file);
 +	}
  
- #ifdef UNIV_HOTBACKUP
+-#ifdef UNIV_HOTBACKUP
  func_exit_after_close:
-@@ -4263,7 +4434,7 @@
+-#else
+ 	ut_ad(!mutex_own(&fil_system->mutex));
+-#endif
++
+ 	mem_free(tablename);
+ 	if (remote.success) {
+ 		mem_free(remote.filepath);
+@@ -4263,7 +4457,7 @@
  idea is to read as much good data as we can and jump over bad data.
  @return 0 if ok, -1 if error even after the retries, 1 if at the end
  of the directory */
@@ -419,7 +488,7 @@
  int
  fil_file_readdir_next_file(
  /*=======================*/
-@@ -4303,7 +4474,7 @@
+@@ -4303,7 +4497,7 @@
  @return	DB_SUCCESS or error number */
  UNIV_INTERN
  dberr_t
@@ -428,7 +497,7 @@
  /*===================================*/
  {
  	int		ret;
-@@ -4359,7 +4530,9 @@
+@@ -4359,7 +4553,9 @@
  			    "%s/%s", fil_path_to_mysql_datadir, dbinfo.name);
  		srv_normalize_path_for_win(dbpath);
  
@@ -439,7 +508,7 @@
  
  		if (dbdir != NULL) {
  
-@@ -4380,9 +4553,15 @@
+@@ -4380,9 +4576,15 @@
  				    && (0 == strcmp(fileinfo.name
  						   + strlen(fileinfo.name) - 4,
  						   ".ibd")
@@ -457,7 +526,7 @@
  					/* The name ends in .ibd or .isl;
  					try opening the file */
  					fil_load_single_table_tablespace(
-@@ -4538,6 +4717,7 @@
+@@ -4538,6 +4740,7 @@
  {
  	fil_space_t*	fnamespace;
  	fil_space_t*	space;
@@ -465,7 +534,7 @@
  
  	ut_ad(fil_system);
  
-@@ -4615,6 +4795,10 @@
+@@ -4615,6 +4818,10 @@
  		if (fnamespace == NULL) {
  			if (print_error_if_does_not_exist) {
  				fil_report_missing_tablespace(name, id);
@@ -476,7 +545,7 @@
  			}
  		} else {
  			ut_print_timestamp(stderr);
-@@ -4638,6 +4822,10 @@
+@@ -4638,6 +4845,10 @@
  
  		mutex_exit(&fil_system->mutex);
  
@@ -487,7 +556,7 @@
  		return(FALSE);
  	}
  
-@@ -4728,6 +4916,7 @@
+@@ -4728,6 +4939,7 @@
  	ulint		page_size;
  	ulint		pages_added;
  	ibool		success;
@@ -495,7 +564,7 @@
  
  	ut_ad(!srv_read_only_mode);
  
-@@ -4772,13 +4961,17 @@
+@@ -4772,13 +4984,17 @@
  		goto retry;
  	}
  
@@ -514,7 +583,7 @@
  	start_page_no = space->size;
  	file_start_page_no = space->size - node->size;
  
-@@ -5024,7 +5217,7 @@
+@@ -5024,7 +5240,7 @@
  off the LRU list if it is in the LRU list. The caller must hold the fil_sys
  mutex. */
  static
@@ -523,7 +592,7 @@
  fil_node_prepare_for_io(
  /*====================*/
  	fil_node_t*	node,	/*!< in: file node */
-@@ -5044,9 +5237,12 @@
+@@ -5044,9 +5260,12 @@
  	}
  
  	if (node->open == FALSE) {
@@ -537,7 +606,7 @@
  	}
  
  	if (node->n_pending == 0 && fil_space_belongs_in_lru(space)) {
-@@ -5058,6 +5254,8 @@
+@@ -5058,6 +5277,8 @@
  	}
  
  	node->n_pending++;
@@ -546,7 +615,7 @@
  }
  
  /********************************************************************//**
-@@ -5259,6 +5457,16 @@
+@@ -5259,6 +5480,16 @@
  
  	ut_ad(mode != OS_AIO_IBUF || space->purpose == FIL_TABLESPACE);
  
@@ -563,7 +632,7 @@
  	node = UT_LIST_GET_FIRST(space->chain);
  
  	for (;;) {
-@@ -5290,7 +5498,11 @@
+@@ -5290,7 +5521,11 @@
  	}
  
  	/* Open file if closed */
@@ -576,7 +645,7 @@
  
  	/* Check that at least the start offset is within the bounds of a
  	single-table tablespace, including rollback tablespaces. */
-@@ -6164,6 +6376,7 @@
+@@ -6164,6 +6399,7 @@
  	return(err);
  }
  
@@ -584,7 +653,7 @@
  /****************************************************************//**
  Generate redo logs for swapping two .ibd files */
  UNIV_INTERN
-@@ -6187,4 +6400,4 @@
+@@ -6187,4 +6423,4 @@
  			 0, 0, new_name, old_name, &mtr);
  	mtr_commit(&mtr);
  }

=== modified file 'src/xtrabackup.cc'
--- src/xtrabackup.cc	2013-05-16 18:01:50 +0000
+++ src/xtrabackup.cc	2013-06-19 05:51:29 +0000
@@ -4826,24 +4826,12 @@
 }
 
 /************************************************************************
-Initialize the tablespace memory cache and populate it by scanning for and
-opening data files.
-@returns DB_SUCCESS or error code.*/
+Initializes the I/O and tablespace cache subsystems. */
 static
-ulint
-xb_data_files_init(void)
-/*====================*/
+void
+xb_fil_io_init(void)
+/*================*/
 {
-	ulint	i;
-	ibool	create_new_db;
-#ifdef XTRADB_BASED
-	ibool	create_new_doublewrite_file;
-#endif
-	ulint	err;
-	lsn_t	min_flushed_lsn;
-	lsn_t	max_flushed_lsn;
-	ulint   sum_of_new_sizes;
-
 #ifndef INNODB_VERSION_SHORT
 	os_aio_init(8 * SRV_N_PENDING_IOS_PER_THREAD
 		    * srv_n_file_io_threads,
@@ -4868,6 +4856,25 @@
 #endif
 
 	fsp_init();
+}
+
+/****************************************************************************
+Populates the tablespace memory cache by scanning for and opening data files.
+@returns DB_SUCCESS or error code.*/
+static
+ulint
+xb_load_tablespaces(void)
+/*=====================*/
+{
+	ulint	i;
+	ibool	create_new_db;
+#ifdef XTRADB_BASED
+	ibool	create_new_doublewrite_file;
+#endif
+	ulint	err;
+	lsn_t	min_flushed_lsn;
+	lsn_t	max_flushed_lsn;
+	ulint   sum_of_new_sizes;
 
 	for (i = 0; i < srv_n_file_io_threads; i++) {
 		thread_nr[i] = i;
@@ -4932,6 +4939,20 @@
 	return(DB_SUCCESS);
 }
 
+/************************************************************************
+Initialize the tablespace memory cache and populate it by scanning for and
+opening data files.
+@returns DB_SUCCESS or error code.*/
+static
+ulint
+xb_data_files_init(void)
+/*====================*/
+{
+	xb_fil_io_init();
+
+	return(xb_load_tablespaces());
+}
+
 /*********************************************************************//**
 Normalizes init parameter values to use units we use inside InnoDB.
 @return	DB_SUCCESS or error code */
@@ -5438,12 +5459,7 @@
 	ulint	err;
 	ulint	i;
 
-	err = xb_data_files_init();
-	if (err != DB_SUCCESS) {
-		msg("xtrabackup: error: xb_data_files_init() failed with"
-		    "error code %lu\n", err);
-		exit(EXIT_FAILURE);
-	}
+	xb_fil_io_init();
 
 	log_init();
 
@@ -5655,6 +5671,14 @@
 	log_copying_stop = xb_os_event_create(NULL);
 	os_thread_create(log_copying_thread, NULL, &log_copying_thread_id);
 
+	/* Populate fil_system with tablespaces to copy */
+	err = xb_load_tablespaces();
+	if (err != DB_SUCCESS) {
+		msg("xtrabackup: error: xb_load_tablespaces() failed with"
+		    "error code %lu\n", err);
+		exit(EXIT_FAILURE);
+	}
+
 	if (xtrabackup_parallel > 1 && xtrabackup_stream &&
 	    xtrabackup_stream_fmt == XB_STREAM_FMT_TAR) {
 		msg("xtrabackup: warning: the --parallel option does not have "