Merge lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0 into lp:percona-xtrabackup/2.0

[Sergei Glushchenko]

Bug #1062684 Applying incremental backup using xtrabackup 2.0.3 fails
when innodb_data_file_path = ibdata1:2000M;ibdata2:10M:autoextend is set
in [mysqld]

Fixed by initialization of srv_data_file_sizes in xb_delta_open_matching_space.

[Sergei Glushchenko (sergei.glushchenko)]

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

[Sergei Glushchenko (sergei.glushchenko)]

#25967

[Alexey Kopytov (akopytov)]

It looks like in this fix we are doing some changes to some srv_* variables based on their previous values in xb_delta_open_matching_space(). What if this function is called multiple times?

review: Needs Information

[Sergei Glushchenko (sergei.glushchenko)]

This change is in xb_data_files_init, not in xb_delta_open_matching_space. Those srv_* values first inited in innodb_init_param. We should call xb_data_files_init once after calling innodb_init_param, and I can't see why we would need to do it more times. Another possible place to put this code in is innodb_init_param.

[Alexey Kopytov (akopytov)]

OK, I was just reading the commit / MP comment: "Fixed by srv_data_file_sizes in xb_delta_open_matching_space."

[Alexey Kopytov (akopytov)]

Still something doesn't add up. So the code this patch moves from xtrabackup_backup_func() to xb_data_files_init() has been previously copy-pasted from srv_normalize_init_values().

Now srv_normalize_init_values is called from srv_boot() which in turn is called from innobase_start_or_create_for_mysql().

Which means when applying an incremental backup, we first execute that code by calling xb_data_files_init(). And then execute this code once again when starting an InnoDB instance by calling innodb_init().

Am I missing anything here?

Please also provide a proper description of the problem in revision comments. I.e. what exactly on the code level led to the crash. And the current solution ("Fixed by srv_data_file_sizes in xb_delta_open_matching_space.") doesn't make much sense either.

review: Needs Information

[Sergei Glushchenko (sergei.glushchenko)]

Alexey,

> Still something doesn't add up. So the code this patch moves from
> xtrabackup_backup_func() to xb_data_files_init() has been previously copy-
> pasted from srv_normalize_init_values().
>
> Now srv_normalize_init_values is called from srv_boot() which in turn is
> called from innobase_start_or_create_for_mysql().
>
> Which means when applying an incremental backup, we first execute that code by
> calling xb_data_files_init(). And then execute this code once again when
> starting an InnoDB instance by calling innodb_init().
>
> Am I missing anything here?
>

Yes, you do. srv_parse_data_file_paths_and_sizes is called from innodb_init_param which resets srv_data_file_sizes. innodb_init_param called twice during incremental backup.

> Please also provide a proper description of the problem in revision comments.
> I.e. what exactly on the code level led to the crash. And the current solution
> ("Fixed by srv_data_file_sizes in xb_delta_open_matching_space.") doesn't make
> much sense either.

I'd like to update this branch and comments too once prerequisite branch being approved.

[Alexey Kopytov (akopytov)]

Sergei,

Thanks for explanations.

Some things I noticed when branching lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0 locally:

- revision #472 containing the fix for this bug is a _merge_ revision. It appears you committed the fix for #1066843 and #1068470, then merged it into another branch and then implemented a fix for bug #1062684 as a merge revision. Please create separate branches and separate revisions.

- xb_normalize_init_values() contains trailing whitespace for some lines

- both lines with calls to xb_normalize_init_values() use spaces rather than tabs for indentation.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'src/xtrabackup.c'
--- src/xtrabackup.c	2012-11-16 11:26:22 +0000
+++ src/xtrabackup.c	2012-11-16 11:26:22 +0000
@@ -3975,6 +3975,46 @@
 	return(fil_load_single_table_tablespaces());
 }
 
+/*********************************************************************//**
+Normalizes init parameter values to use units we use inside InnoDB.
+@return	DB_SUCCESS or error code */
+void
+xb_normalize_init_values(void)
+/*==========================*/
+{
+	ulint	i;
+
+	for (i = 0; i < srv_n_data_files; i++) {
+		srv_data_file_sizes[i] = srv_data_file_sizes[i]
+					* ((1024 * 1024) / UNIV_PAGE_SIZE);
+	}
+
+	srv_last_file_size_max = srv_last_file_size_max
+					* ((1024 * 1024) / UNIV_PAGE_SIZE);
+
+	srv_log_file_size = srv_log_file_size / UNIV_PAGE_SIZE;
+
+	srv_log_buffer_size = srv_log_buffer_size / UNIV_PAGE_SIZE;
+
+#ifndef INNODB_VERSION_SHORT
+	srv_pool_size = srv_pool_size / (UNIV_PAGE_SIZE / 1024);
+
+	srv_awe_window_size = srv_awe_window_size / UNIV_PAGE_SIZE;
+
+	if (srv_use_awe) {
+	        /* If we are using AWE we must save memory in the 32-bit
+		address space of the process, and cannot bind the lock
+		table size to the real buffer pool size. */
+
+	        srv_lock_table_size = 20 * srv_awe_window_size;
+	} else {
+	        srv_lock_table_size = 5 * srv_pool_size;
+	}
+#else
+	srv_lock_table_size = 5 * (srv_buf_pool_size / UNIV_PAGE_SIZE);
+#endif
+}
+
 /************************************************************************
 Destroy the tablespace memory cache. */
 void
@@ -4213,6 +4253,8 @@
         if(innodb_init_param())
                 exit(EXIT_FAILURE);
 
+	xb_normalize_init_values();
+
 #ifndef __WIN__        
         if (srv_file_flush_method_str == NULL) {
         	/* These are the default options */
@@ -4290,43 +4332,6 @@
                                                 computers */
         }
 
-	{
-	ulint	nr;
-	ulint	i;
-
-	nr = srv_n_data_files;
-	
-	for (i = 0; i < nr; i++) {
-		srv_data_file_sizes[i] = srv_data_file_sizes[i]
-					* ((1024 * 1024) / UNIV_PAGE_SIZE);
-	}		
-
-	srv_last_file_size_max = srv_last_file_size_max
-					* ((1024 * 1024) / UNIV_PAGE_SIZE);
-		
-	srv_log_file_size = srv_log_file_size / UNIV_PAGE_SIZE;
-
-	srv_log_buffer_size = srv_log_buffer_size / UNIV_PAGE_SIZE;
-
-#ifndef INNODB_VERSION_SHORT
-	srv_pool_size = srv_pool_size / (UNIV_PAGE_SIZE / 1024);
-
-	srv_awe_window_size = srv_awe_window_size / UNIV_PAGE_SIZE;
-	
-	if (srv_use_awe) {
-	        /* If we are using AWE we must save memory in the 32-bit
-		address space of the process, and cannot bind the lock
-		table size to the real buffer pool size. */
-
-	        srv_lock_table_size = 20 * srv_awe_window_size;
-	} else {
-	        srv_lock_table_size = 5 * srv_pool_size;
-	}
-#else
-	srv_lock_table_size = 5 * (srv_buf_pool_size / UNIV_PAGE_SIZE);
-#endif
-	}
-
 	os_sync_mutex = NULL;
 	srv_general_init();
 
@@ -6409,6 +6414,8 @@
 	if(innodb_init_param())
 		goto error;
 
+	xb_normalize_init_values();
+
 	mem_init(srv_mem_pool_size);
 	if (xtrabackup_incremental) {
 		err = xb_data_files_init();

=== added file 'test/t/bug1062684.sh'
--- test/t/bug1062684.sh	1970-01-01 00:00:00 +0000
+++ test/t/bug1062684.sh	2012-11-16 11:26:22 +0000
@@ -0,0 +1,97 @@
+. inc/common.sh
+
+start_server --innodb-data-file-path="ibdata1:10M;ibdata2:5M:autoextend"
+load_dbase_schema incremental_sample
+
+echo "innodb-data-file-path=ibdata1:10M;ibdata2:5M:autoextend" >>$topdir/my.cnf
+
+# Adding initial rows
+vlog "Adding initial rows to database..."
+${MYSQL} ${MYSQL_ARGS} -e "insert into test values (1, 1);" incremental_sample
+
+# Full backup
+# backup root directory
+mkdir -p $topdir/backup
+
+vlog "Starting backup"
+innobackupex  $topdir/backup
+full_backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | awk -F\' '{print $2}'`
+vlog "Full backup done to directory $full_backup_dir"
+
+# Changing data
+
+vlog "Making changes to database"
+${MYSQL} ${MYSQL_ARGS} -e "create table t2 (a int(11) default null, number int(11) default null) engine=innodb" incremental_sample
+${MYSQL} ${MYSQL_ARGS} -e "insert into test values (10, 1);" incremental_sample
+${MYSQL} ${MYSQL_ARGS} -e "insert into t2 values (10, 1);" incremental_sample
+vlog "Changes done"
+
+# Saving the checksum of original table
+checksum_test_a=`checksum_table incremental_sample test`
+checksum_t2_a=`checksum_table incremental_sample t2`
+vlog "Table 'test' checksum is $checksum_test_a"
+vlog "Table 't2' checksum is $checksum_t2_a"
+
+vlog "Making incremental backup"
+
+vlog "###############"
+vlog "# INCREMENTAL #"
+vlog "###############"
+
+# Incremental backup
+innobackupex --incremental --incremental-basedir=$full_backup_dir \
+    $topdir/backup
+inc_backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | tail -n 1 | awk -F\' '{print $2}'`
+vlog "Incremental backup done to directory $inc_backup_dir"
+
+vlog "Preparing backup"
+# Prepare backup
+vlog "##############"
+vlog "# PREPARE #1 #"
+vlog "##############"
+innobackupex --apply-log --redo-only $full_backup_dir
+vlog "Log applied to full backup"
+vlog "##############"
+vlog "# PREPARE #2 #"
+vlog "##############"
+innobackupex --apply-log --redo-only --incremental-dir=$inc_backup_dir \
+    $full_backup_dir
+vlog "Delta applied to full backup"
+vlog "##############"
+vlog "# PREPARE #3 #"
+vlog "##############"
+innobackupex --apply-log $full_backup_dir
+vlog "Data prepared for restore"
+
+# Destroying mysql data
+stop_server
+rm -rf $mysql_datadir/*
+vlog "Data destroyed"
+
+# Restore backup
+vlog "Copying files"
+vlog "###########"
+vlog "# RESTORE #"
+vlog "###########"
+innobackupex --copy-back $full_backup_dir
+vlog "Data restored"
+
+start_server --innodb-data-file-path="ibdata1:10M;ibdata2:5M:autoextend"
+
+vlog "Checking checksums"
+checksum_test_b=`checksum_table incremental_sample test`
+checksum_t2_b=`checksum_table incremental_sample t2`
+
+if [ "$checksum_test_a" != "$checksum_test_b"  ]
+then 
+	vlog "Checksums for table 'test' are not equal"
+	exit -1
+fi
+
+if [ "$checksum_t2_a" != "$checksum_t2_b"  ]
+then 
+	vlog "Checksums for table 't2' are not equal"
+	exit -1
+fi
+
+vlog "Checksums are OK"