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

[Sergei Glushchenko]

Add option to specify sleep in log copying loop.
http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/389/BUILD_TYPE=release,Host=ubuntu-lucid-32bit,xtrabackuptarget=xtradb56/

[Sergei Glushchenko (sergei.glushchenko)]

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

[Alexey Kopytov (akopytov)]

  - I think --log-copy-interval would be a bit more self-descriptive name
  - there's no code in innobackupex that actually passes the value of
    $option_log_copy_sleep_time as an xtrabackup command line argument
  - 60 seconds is way too much. let's keep it at 1 second as previously
  - I prefer defining default values in one place. for example
    $option_log_copy_sleep_time does not have a default value, it is
    not passed to xtrabackup, and then the default value defined in
    xtrabackup is used. this way we also avoid unnecessary command line
    argument to xtrabackup when the option is not changed from its
    default value
  - there's an upstream bug with os_event_wait_time() macro defined
    incorrectly. Note failed Jenkins builds for innodb55/xtradb55.
    The easiest workaround is to probably define
    xb_event_wait_time() which maps to either os_event_wait_time() or
    os_event_wait_time_low() depending on the build.
  - we no longer need 'counter' in log_copying_thread(), but I don't
    see it's declaration being removed
  - we also don't need SLEEPING_PERIOD anymore?

review: Needs Fixing

[Alexey Kopytov (akopytov)]

also s/loc/log/ in the test case comment.

[Sergei Glushchenko (sergei.glushchenko)]

Hi Alexey,

> - I think --log-copy-interval would be a bit more self-descriptive name
let it be so

> - there's no code in innobackupex that actually passes the value of
> $option_log_copy_sleep_time as an xtrabackup command line argument
yes, you are right

> - 60 seconds is way too much. let's keep it at 1 second as previously
set back to 1 second

> - I prefer defining default values in one place. for example
> $option_log_copy_sleep_time does not have a default value, it is
> not passed to xtrabackup, and then the default value defined in
> xtrabackup is used. this way we also avoid unnecessary command line
> argument to xtrabackup when the option is not changed from its
> default value
agree

> - there's an upstream bug with os_event_wait_time() macro defined
> incorrectly. Note failed Jenkins builds for innodb55/xtradb55.
> The easiest workaround is to probably define
> xb_event_wait_time() which maps to either os_event_wait_time() or
> os_event_wait_time_low() depending on the build.
done

> - we no longer need 'counter' in log_copying_thread(), but I don't
> see it's declaration being removed
> - we also don't need SLEEPING_PERIOD anymore?
removed

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

Thanks,
Sergei

[Sergei Glushchenko (sergei.glushchenko)]

and typo in test case comment has been fixed

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'doc/source/innobackupex/innobackupex_option_reference.rst'
--- doc/source/innobackupex/innobackupex_option_reference.rst	2013-01-17 15:08:37 +0000
+++ doc/source/innobackupex/innobackupex_option_reference.rst	2013-04-15 11:17:27 +0000
@@ -86,6 +86,10 @@
 
    This option accepts a string argument that specifies the log sequence number (:term:`LSN`) to use for the incremental backup. It is used with the :option:`--incremental` option. It is used instead of specifying :option:`--incremental-basedir`. For databases created by *MySQL* and *Percona Server* 5.0-series versions, specify the as two 32-bit integers in high:low format. For databases created in 5.1 and later, specify the LSN as a single 64-bit integer.
 
+.. option:: --log-copy-interval
+
+   This option specifies time interval between checks done by log copying thread in milliseconds.
+
 .. option:: --move-back
 
     Move all the files in a previously made backup from the backup directory to their original locations. As this option removes backup files, it must be used with caution.

=== modified file 'doc/source/xtrabackup_bin/xbk_option_reference.rst'
--- doc/source/xtrabackup_bin/xbk_option_reference.rst	2012-05-18 11:19:17 +0000
+++ doc/source/xtrabackup_bin/xbk_option_reference.rst	2013-04-15 11:17:27 +0000
@@ -105,6 +105,10 @@
 
    This option is to set the group which should be read from the configuration file. This is used by innobackupex if you use the `--defaults-group` option. It is needed for mysqld_multi deployments.
 
+.. option:: --log-copy-interval
+
+   This option specifies time interval between checks done by log copying thread in milliseconds (default is 1 second).
+
 .. option:: --log-stream
 
    Makes xtrabackup not copy data files, and output the contents of the InnoDB log files to STDOUT until the :option:`--suspend-at-end` file is deleted. This option enables :option:`--suspend-at-end` automatically.

=== modified file 'innobackupex'
--- innobackupex	2013-04-03 07:04:25 +0000
+++ innobackupex	2013-04-15 11:17:27 +0000
@@ -97,6 +97,7 @@
 my $option_galera_info = '';
 my $option_no_lock = '';
 my $option_ibbackup_binary = 'autodetect';
+my $option_log_copy_interval = 0;
 
 my $option_defaults_file = '';
 my $option_defaults_extra_file = '';
@@ -1043,6 +1044,9 @@
     if ($option_throttle) {
         $options = $options . " --throttle=$option_throttle";
     }
+    if ($option_log_copy_interval) {
+        $options = $options . " --log-copy-interval=$option_log_copy_interval";
+    }
     if ($option_sleep) {
         $options = $options . " --sleep=$option_sleep";
     }
@@ -1878,6 +1882,7 @@
                         'help' => \$option_help,
                         'version' => \$option_version,
                         'throttle=i' => \$option_throttle,
+                        'log-copy-interval=i', \$option_log_copy_interval,
                         'sleep=i' => \$option_sleep,
                         'apply-log' => \$option_apply_log,
                         'redo-only' => \$option_redo_only,
@@ -3004,6 +3009,10 @@
 
 This option specifies the directory where the incremental backup will be combined with the full backup to make a new full backup.  The option accepts a string argument. It is used with the --incremental option.
 
+=item --log-copy-interval
+
+This option specifies time interval between checks done by log copying thread in milliseconds.
+
 =item --incremental-lsn
 
 This option specifies the log sequence number (LSN) to use for the incremental backup.  The option accepts a string argument. It is used with the --incremental option. It is used instead of specifying --incremental-basedir. For databases created by MySQL and Percona Server 5.0-series versions, specify the LSN as two 32-bit integers in high:low format. For databases created in 5.1 and later, specify the LSN as a single 64-bit integer.

=== modified file 'src/xtrabackup.cc'
--- src/xtrabackup.cc	2013-04-03 22:53:49 +0000
+++ src/xtrabackup.cc	2013-04-15 11:17:27 +0000
@@ -623,6 +623,39 @@
 } /* extern "C" */
 #endif
 
+/**********************************************************//**
+Waits for an event object until it is in the signaled state or
+a timeout is exceeded.
+@return 0 if success, OS_SYNC_TIME_EXCEEDED if timeout was exceeded */
+static
+ulint
+xb_event_wait_time(
+/*===============*/
+	os_event_t	event,			/*!< in: event to wait */
+	ulint		time_in_usec);		/*!< in: timeout in
+						microseconds, or
+						OS_SYNC_INFINITE_TIME */
+
+/**********************************************************//**
+Waits for an event object until it is in the signaled state or
+a timeout is exceeded.
+@return 0 if success, OS_SYNC_TIME_EXCEEDED if timeout was exceeded */
+static
+ulint
+xb_event_wait_time(
+/*===============*/
+	os_event_t	event,			/*!< in: event to wait */
+	ulint		time_in_usec)		/*!< in: timeout in
+						microseconds, or
+						OS_SYNC_INFINITE_TIME */
+{
+#if (MYSQL_VERSION_ID > 50500) && (MYSQL_VERSION_ID > 50600)
+	return os_event_wait_time_low(event, time_in_usec, 0);
+#else
+	return os_event_wait_time(event, time_in_usec);
+#endif
+}
+
 /****************************************************************//**
 A simple function to open or create a file.
 @return own: handle to the file, not defined if error, error number
@@ -1315,6 +1348,7 @@
 long xtrabackup_throttle = 0; /* 0:unlimited */
 lint io_ticket;
 os_event_t wait_throttle = NULL;
+os_event_t log_copying_stop = NULL;
 
 my_bool xtrabackup_log_only = FALSE;
 char *xtrabackup_incremental = NULL;
@@ -1367,6 +1401,10 @@
 ibool xtrabackup_compress = FALSE;
 uint xtrabackup_compress_threads;
 
+/* sleep interval beetween log copy iterations in log copying thread
+in milliseconds (default is 1 second) */
+int xtrabackup_log_copy_interval = 1000;
+
 /* === metadata of backup === */
 #define XTRABACKUP_METADATA_FILENAME "xtrabackup_checkpoints"
 char metadata_type[30] = ""; /*[full-backuped|full-prepared|incremental]*/
@@ -1579,6 +1617,7 @@
   OPT_XTRA_SUSPEND_AT_END,
   OPT_XTRA_USE_MEMORY,
   OPT_XTRA_THROTTLE,
+  OPT_XTRA_LOG_COPY_INTERVAL,
   OPT_XTRA_LOG_ONLY,
   OPT_XTRA_INCREMENTAL,
   OPT_XTRA_INCREMENTAL_BASEDIR,
@@ -1706,6 +1745,9 @@
   {"throttle", OPT_XTRA_THROTTLE, "limit count of IO operations (pairs of read&write) per second to IOS values (for '--backup')",
    (G_PTR*) &xtrabackup_throttle, (G_PTR*) &xtrabackup_throttle,
    0, GET_LONG, REQUIRED_ARG, 0, 0, LONG_MAX, 0, 1, 0},
+  {"log-copy-interval", OPT_XTRA_LOG_COPY_INTERVAL, "time interval between checks done by log copying thread in milliseconds (default is 1 second).",
+   (G_PTR*) &xtrabackup_log_copy_interval, (G_PTR*) &xtrabackup_log_copy_interval,
+   0, GET_LONG, REQUIRED_ARG, 1000, 0, LONG_MAX, 0, 1, 0},
   {"log-stream", OPT_XTRA_LOG_ONLY, "outputs the contents of 'xtrabackup_logfile' to stdout only until the file 'xtrabackup_suspended' deleted (for '--backup').",
    (G_PTR*) &xtrabackup_log_only, (G_PTR*) &xtrabackup_log_only,
    0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
@@ -4486,9 +4528,6 @@
 	return(TRUE);
 }
 
-/* copying logfile in background */
-#define SLEEPING_PERIOD 5
-
 static
 #ifndef __WIN__
 void*
@@ -4498,20 +4537,19 @@
 log_copying_thread(
 	void*	arg __attribute__((unused)))
 {
-	ulint	counter = 0;
-
 	ut_a(dst_log_fd != XB_FILE_UNDEFINED);
 
 	log_copying_running = TRUE;
 
 	while(log_copying) {
-		os_thread_sleep(200000); /*0.2 sec*/
-
-		counter++;
-		if(counter >= SLEEPING_PERIOD * 5) {
-			if(xtrabackup_copy_logfile(log_copy_scanned_lsn, FALSE))
+		os_event_reset(log_copying_stop);
+		xb_event_wait_time(log_copying_stop,
+				xtrabackup_log_copy_interval * 1000ULL);
+		if (log_copying) {
+			if(xtrabackup_copy_logfile(log_copy_scanned_lsn,
+						   FALSE)) {
 				goto end;
-			counter = 0;
+			}
 		}
 	}
 
@@ -5399,6 +5437,7 @@
 		exit(EXIT_FAILURE);
 
 
+	log_copying_stop = xb_os_event_create(NULL);
 	os_thread_create(log_copying_thread, NULL, &log_copying_thread_id);
 
 	if (xtrabackup_parallel > 1 && xtrabackup_stream &&
@@ -5549,6 +5588,7 @@
 skip_last_cp:
 	/* stop log_copying_thread */
 	log_copying = FALSE;
+	os_event_set(log_copying_stop);
 	msg("xtrabackup: Stopping log copying thread.\n");
 	while (log_copying_running) {
 		msg(".");
@@ -5561,6 +5601,8 @@
 		exit(EXIT_FAILURE);
 	}
 
+	os_event_free(log_copying_stop);
+
 	/* Signal innobackupex that log copying has stopped and it may now
 	unlock tables, so we can possibly stream xtrabackup_logfile later
 	without holding the lock. */

=== added file 'test/t/bug1125993.sh'
--- test/t/bug1125993.sh	1970-01-01 00:00:00 +0000
+++ test/t/bug1125993.sh	2013-04-15 11:17:27 +0000
@@ -0,0 +1,15 @@
+########################################################################
+# Bug #1125993: To specify sleep interval in log copying thread
+########################################################################
+
+. inc/common.sh
+
+start_server --innodb_file_per_table
+
+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
+CREATE TABLE t1(a INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (4), (5), (6);
+EOF
+
+vlog "Starting backup"
+innobackupex  --no-timestamp $topdir/backup --log-copy-interval=590