List:Commits« Previous MessageNext Message »
From:Rafal Somla Date:February 19 2009 2:37pm
Subject:bzr commit into mysql-6.0-backup branch (Rafal.Somla:2773) Bug#35079
WL#4538
View as plain text  
#At file:///ext/mysql/bzr/backup/wl4538-new/

 2773 Rafal Somla	2009-02-19
      BUG#35079/WL#4538 - Make BACKUP/RESTORE statements interruptible.
            
      The server detects interruptions in statement execution such as when client 
      connection is closed or if user hits Ctrl+C. However, the code inside the server 
      must actively check if an interruption has happened and abort execution in that 
      case.
            
      This patch adds such checks to the backup code, as described in WL#4538. After 
      this patch it should be possible to interrupt on-going BACKUP/RESTORE command. 
      It also fixes few problems in the code so that the new tests can pass through.
added:
  mysql-test/suite/backup/t/backup_intr_errors.test
  mysql-test/suite/backup_engines/include/backup_restore_interrupt.inc
  mysql-test/suite/backup_engines/t/backup_interruption.test
modified:
  sql/backup/backup_kernel.h
  sql/backup/be_default.h
  sql/backup/be_snapshot.h
  sql/backup/be_thread.cc
  sql/backup/be_thread.h
  sql/backup/data_backup.cc
  sql/backup/kernel.cc
  sql/backup/logger.cc
  sql/backup/logger.h
  sql/backup/stream.cc
  sql/backup/stream.h
  sql/share/errmsg.txt
  sql/si_objects.cc
  sql/sql_prepare.cc

per-file messages:
  mysql-test/suite/backup/t/backup_intr_errors.test
    New test which checks error reporting after an interruption.
  mysql-test/suite/backup_engines/include/backup_restore_interrupt.inc
    A helper "subroutine" for the interruption tests.
  mysql-test/suite/backup_engines/t/backup_interruption.test
    The main test for interruptions.
  sql/backup/backup_kernel.h
    Add is_killed() method.
  sql/backup/be_default.h
    Add error injection code.
  sql/backup/be_snapshot.h
    Add error injection code.
  sql/backup/be_thread.cc
    Fix possible deadlock when the Locking_thread_st destructor is waiting
    for a locking thread which has not been started before.
  sql/backup/be_thread.h
    Add m_thread_started member to Locking_thread_st.
  sql/backup/data_backup.cc
    - Fix error in write_table_data() logic which caused the commit blocker to 
      not be deactivated if error branch was executed.
    - Add checks for interruptions in various stages of backup and restore 
      protocol, including  Scheduler::step() which is the main worker method in 
      write_table_data().
    - Add explicit log_level::ERROR to report_error() where needed.
    - Add extra debug sync points needed for testing.
  sql/backup/kernel.cc
    - Add more debug sync points needed for testing.
    - Add checks for interruption inside Backup_restore_ctx methods,
      at various stages of executing BACKUP/RESTORE statement. Note: we rely
      on the fact that report/log_error() will report interruption if one has
      happened.
    - Set m_state in prepare_for_backup() early, so that in case of error/
      interruption backup file is correctly removed.
    - Inside prepare_for_*() methods, move the place where m_catalog is 
      assigned so that it is always deleted in the destructor.
    - Use explicit log_level::ERROR when needed.
    - Fix Backup_restore_ctx::close() to always call report_stop()
      matching previous report_start().
    - Fix possible memory leak in Backup_restore_ctx::restore_triggers_and_events().
    - Move the temporary cleanup code needed after restoring triggers and events 
      from do_restore() to restore_triggers_and_events() so that it is always 
      executed.
  sql/backup/logger.cc
    Add Logger::report_killed() method.
  sql/backup/logger.h
    - Declare report_error() method.
    - Declare variant of log_error() which does not detect interruptions.
    - Add m_kill_reported flag.
    - Change semantics of report_error() and log_error() so that they detect
      and report interruptions.
  sql/backup/stream.cc
    Add error injection code.
  sql/backup/stream.h
    Move the temporary cleanup code to the wrapper function read_meta_data() so 
    that we don't have to depend on the caller for executing it.
  sql/share/errmsg.txt
    Add message used to notify about an interruption of BACKUP/RESTORE operations.
  sql/si_objects.cc
    Do not continue object creation if the initial drop() has failed.
  sql/sql_prepare.cc
    Fix a problem that interruptions were not handled inside execute_direct
    which could lead to a later assertion in post-query cleanup code.
=== added file 'mysql-test/suite/backup/t/backup_intr_errors.test'
--- a/mysql-test/suite/backup/t/backup_intr_errors.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/t/backup_intr_errors.test	2009-02-19 14:37:37 +0000
@@ -0,0 +1,126 @@
+#
+# This test checks what happens if errors occur during BACKUP/RESTORE shutdown
+# sequence in case of interruption of one of these statements. The errors are
+# triggered using error injection code. They should be reported on server's 
+# error stack after the standard "Query execution was interrupted" error. The
+# latter error should be reported by the interrupted statement.
+#
+
+--source include/not_embedded.inc
+--source include/have_innodb.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+
+call mtr.add_suppression("Backup:");
+call mtr.add_suppression("Restore:");
+
+--echo #
+--echo # Setup
+--echo #
+let $bdir=`SELECT @@backupdir`;
+
+--disable_warnings
+DROP DATABASE IF EXISTS bup_intr;
+--error 0,1
+--remove_file $bdir/bup_intr.bkp
+--enable_warnings
+
+CREATE DATABASE bup_intr;
+USE bup_intr;
+
+CREATE TABLE t1(engine char(6));
+#
+# Table t1 is used in the test loop which will iterate over its rows and
+# change t1's storage engine as indicated. We pick innodb and memory to 
+# trigger use of the default and the snapshot backup/restore drivers.
+#
+INSERT INTO  t1 VALUES ('innodb'),('memory');
+#
+# Table t2 is here so that some table data is always stored in the image
+# and the synchronization point used below is always reached.
+#
+CREATE TABLE t2(a int);
+INSERT INTO t2 VALUES (1);
+
+# connection required by backup_restore_interrupt.inc
+--connect (killer,localhost,root,,)
+
+#
+# Loop over rows in t1 which indicate what storage engine should be used.
+#
+
+while (`SELECT count(*) > 0 FROM bup_intr.t1`)
+{
+
+  # read the next engine and remove it from t1
+  let $engine=`SELECT engine FROM bup_intr.t1 LIMIT 1`;
+  eval DELETE FROM bup_intr.t1 WHERE engine='$engine';
+
+  --echo
+  --echo ########################################
+  --echo ## Testing with $engine engine.
+  --echo ########################################
+  --echo
+
+  # change storage engine of t1
+  eval ALTER TABLE bup_intr.t1 ENGINE=$engine;
+
+  #
+  # Test interruption of BACKUP
+  #
+  let $do_restore=0;
+  # activate error injection code
+  SET SESSION DEBUG="+d,backup_driver_cancel_error";
+  SET SESSION DEBUG="+d,backup_stream_close_error";
+  SET SESSION DEBUG="+d,backup_remove_location_error";
+  #
+  # Test interruption in the middle of BACKUP operation, just before VP
+  # is created.
+  #
+  let $sync_point=before_backup_data_lock;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  SET SESSION DEBUG="-d";
+
+  --echo #
+  --echo # Prepare bup_intr.bkp for RESTORE testing. Note that above BACKUP
+  --echo # command should not create the file because it was interrupted.
+  --echo #
+  --replace_column 1 #
+  BACKUP DATABASE bup_intr TO 'bup_intr.bkp';
+  DROP DATABASE bup_intr;
+
+  #
+  # Test interruption of RESTORE
+  #
+  let $do_restore=1;
+  # activate error injection code
+  SET SESSION DEBUG="+d,backup_driver_cancel_error";
+  SET SESSION DEBUG="+d,backup_stream_close_error";
+  #
+  # Test interruption of RESTORE when the first block of table data is
+  # sent to a restore driver.
+  #
+  let $sync_point=restore_before_sending_data;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  SET SESSION DEBUG="-d";
+
+  --echo #
+  --echo # Restore original database - the interrupted RESTORE statement could
+  --echo # corrupt it.
+  --echo #
+  --replace_column 1 #
+  RESTORE FROM 'bup_intr.bkp' OVERWRITE;
+  --remove_file $bdir/bup_intr.bkp
+
+} # end of the while loop.
+
+--echo #
+--echo # Cleanup
+--echo #
+SET DEBUG_SYNC='reset';
+--disable_warnings
+DROP DATABASE IF EXISTS bup_intr;
+--error 0,1
+--remove_file $bdir/bup_intr.bkp
+--enable_warnings
+

=== added file 'mysql-test/suite/backup_engines/include/backup_restore_interrupt.inc'
--- a/mysql-test/suite/backup_engines/include/backup_restore_interrupt.inc	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup_engines/include/backup_restore_interrupt.inc	2009-02-19 14:37:37 +0000
@@ -0,0 +1,106 @@
+#
+# This is a "subroutine" for backup_interrupt.test. It performs the following:
+#
+# 1. Start BACKUP or RESTORE statement and use a synchronization point
+#    to stop it in the middle of execution.
+# 2. In another connection, wait for the statement to reach the synchronization
+#    point, and then KILL it.
+# 3. Signal the stopped BACKUP/RESTORE statement so that it resumes execution
+#    after being killed.
+# 4. Check the error response from the statement, contents of the error stack
+#    and backup logs.
+#
+# Variable $do_restore determines whether BACKUP or RESTORE is executed. The
+# synchronization point to use is stored in $sync_point. 
+#
+# This script uses the following assumptions, which must be satisfied by 
+# its user:
+#
+# - $bdir holds the current value of @@backupdir variable.
+# - There is a second connection named 'killer'.
+# - There exists database called 'bup_intr'.
+# - If $do_restore = 1 then $bdir/bup_intr.bkp should contain valid backup 
+#   image.
+#
+
+let $command= BACKUP DATABASE bup_intr TO 'bup_intr.bkp';
+
+if ($do_restore)
+{
+  let $command= RESTORE FROM 'bup_intr.bkp';
+}
+
+--echo 
+--echo ######################################################################
+--echo #
+--echo # Testing interruption of command $command
+--echo # at synchronization point $sync_point.
+--echo #
+--echo ######################################################################
+--echo 
+--connection default
+
+let $id=`select connection_id()`;
+
+PURGE BACKUP LOGS;
+SET DEBUG_SYNC='reset';
+eval SET DEBUG_SYNC='$sync_point SIGNAL here WAIT_FOR go';
+#
+# Arrange for 'here' signal to be always sent at the end of BACKUP/RESTORE 
+# operation (in the destructor of backup/restore context class). This way
+# test will not hang waiting for the signal even if $sync_point is invalid 
+# or never hit. The fact that we missed the requested synchronization point
+# will be detected with a SELECT from I_S.PROCESSLIST (see below). Also,
+# the reply from BACKUP/RESTORE will not be the expected ER_QUERY_INTERRUPTED
+# error in most cases.
+#
+SET DEBUG_SYNC='backup_restore_ctx_dtor SIGNAL here';
+
+--echo #
+--echo # Start the command.
+--echo #
+--send
+eval $command;
+
+  --connection killer
+
+  --echo #
+  --echo # Wait for the command to reach its synchronization point,
+  --echo # then kill it.
+  --echo #
+  SET DEBUG_SYNC='now WAIT_FOR here';
+  --replace_regex /id=[0-9]+/id=<query id>/
+  eval SELECT state FROM INFORMATION_SCHEMA.PROCESSLIST WHERE id=$id;
+  --replace_regex /QUERY [0-9]+/QUERY <query id>/
+  eval KILL QUERY $id;
+  SET DEBUG_SYNC='now SIGNAL go';
+
+--connection default
+
+--echo #
+--echo # Reap the command and show results.
+--echo #
+--error ER_QUERY_INTERRUPTED
+reap;
+--replace_column 2 <error-code>
+# One error message contains file path - mask it out.
+--replace_regex /Error on delete of '.*'/Error on delete of <backup image path>/
+SHOW WARNINGS;
+
+--echo #
+--echo # Examine backup logs.
+--echo #
+--echo # FIXME: Until BUG#39924 is fixed, change to BUP_CANCEL state will not be
+--echo # seen in backup_progress table and backup_history table will be empty.
+--echo # When the bug is fixed the output below will change and the result file
+--echo # should be modifed accordingly.
+--echo #
+SELECT object, error_num, notes FROM mysql.backup_progress;
+query_vertical SELECT * FROM mysql.backup_history;
+
+# check that backup image file was removed
+if (!$do_restore)
+{
+  --error 1
+  --remove_file $bdir/bup_intr.bkp
+}

=== added file 'mysql-test/suite/backup_engines/t/backup_interruption.test'
--- a/mysql-test/suite/backup_engines/t/backup_interruption.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup_engines/t/backup_interruption.test	2009-02-19 14:37:37 +0000
@@ -0,0 +1,264 @@
+#
+# This test checks that BACKUP/TESTORE commands behave correctly when
+# interrupted. The test strategy is as follows:
+#
+# 1. Start BACKUP or RESTORE statement and use a synchronization point
+#    to stop it in the middle of execution.
+# 2. In another connection, wait for the statement to reach the synchronization
+#    point, and then KILL it.
+# 3. Signal the stopped BACKUP/RESTORE statement so that it resumes execution
+#    after being killed.
+# 4. Check the error response from the statement, contents of the error stack
+#    and backup logs.
+# 
+# Above 4 steps are performed inside backup_restore_interrupt.inc "subroutine"
+# which is executed from this script for various synchronization points inside
+# BACKUP and RESTORE commands.
+#
+# Note: Because of BUG#39924, an interruption is not traced within 
+# backup_progress log and backup_history log entry for an interrupted statement
+# is not written. When the bug is fixed, the output of this test will change 
+# and the result file should be updated after careful inspection.
+#
+--source include/not_embedded.inc
+--source suite/backup_engines/include/backup_engine.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+
+call mtr.add_suppression("Operation aborted");
+
+#
+# Setup
+#
+
+let $bdir=`SELECT @@backupdir`;
+
+--disable_warnings
+DROP DATABASE IF EXISTS bup_intr;
+--error 0,1
+--remove_file $bdir/bup_intr.bkp
+--enable_warnings
+
+CREATE DATABASE bup_intr;
+USE bup_intr;
+
+CREATE TABLE t1(a int);
+INSERT INTO  t1 VALUES (1),(2),(3);
+
+# connection required by backup_restore_interrupt.inc
+--connect (killer,localhost,root,,)
+
+#
+# Test BACKUP interruptions.
+#
+
+let $do_restore=0;
+
+# at the very beginning		
+let $sync_point= before_backup_command;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# before preparations
+let $sync_point= before_backup_prepare;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# during preparations
+  # before logger initialization
+  let $sync_point= before_backup_logger_init;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  
+  # before common preparations
+  let $sync_point= before_backup_common_prepare;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # during common preparations
+    # before checking privileges
+    let $sync_point= before_backup_privileges;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+  
+    # before checking that no other BACKUP/RESTORE is running
+    let $sync_point= before_backup_single_op;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # before blocking DDLs
+    let $sync_point= before_backup_ddl_block;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before openning the stream
+  let $sync_point= before_backup_stream_open;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before creating catalogue
+  let $sync_point= before_backup_catalog;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# before populating backup catalogue
+let $sync_point= after_backup_start_backup;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# before do_backup
+let $sync_point= before_do_backup;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# inside do_backup
+  # before preamble is written
+  let $sync_point= before_backup_meta;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  let $sync_point= backup_before_write_preamble;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before write_table_data
+  let $sync_point= before_backup_data;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # inside write_table_data
+    # before initial phase
+    let $sync_point=before_backup_data_init;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # before prepare phase
+    let $sync_point=before_backup_data_prepare;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # before sync phase
+    let $sync_point=before_backup_data_lock;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # inside sync phase
+    let $sync_point=before_backup_data_unlock;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # after sync phase
+    let $sync_point=after_backup_binlog;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+    # before final phase
+    let $sync_point=before_backup_data_finish;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before writing summary section
+  let $sync_point= before_backup_summary;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# Note: after the call to do_backup() the operation is completed
+# and terminates successfully, even if interruption has happened 
+# after its completion.
+
+#
+# Test RESTORE interruptions.
+#
+
+--replace_column 1 #
+BACKUP DATABASE bup_intr TO 'bup_intr.bkp';
+DROP DATABASE bup_intr;
+
+let $do_restore=1;
+
+# before preparations
+let $sync_point= before_restore_prepare;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# during preparations
+  # before logger initialization
+  let $sync_point= before_restore_logger_init;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  
+  # before common preparations
+  let $sync_point= before_restore_common_prepare;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  
+  # before openning the stream
+  let $sync_point= before_restore_stream_open;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before creating catalogue
+  let $sync_point= before_restore_catalog;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before reading header
+  let $sync_point= before_restore_read_header;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before reading catalogue
+  let $sync_point= before_restore_read_catalog;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before writing incident event
+  let $sync_point= before_restore_binlog;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# before do_restore
+let $sync_point= after_backup_start_restore;
+--source suite/backup_engines/include/backup_restore_interrupt.inc
+
+# inside do_restore
+
+  # before disabling fkey constraints
+  let $sync_point= before_restore_fkey_disable;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # before reading metadata and creating objects
+  let $sync_point= before_restore_read_metadata;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+  # Note: now we pass the point when database is created during RESTORE,
+  # thus we need to DROP it before executing RESTORE another time.
+  DROP DATABASE bup_intr;
+
+  # before locking tables
+  let $sync_point= before_restore_lock_tables;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  DROP DATABASE bup_intr;
+
+  # before restoring table data
+  let $sync_point= before_restore_table_data;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  DROP DATABASE bup_intr;
+
+  # inside restore_table_data
+    # before creating restore drivers
+    let $sync_point= restore_before_drivers_create;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+    DROP DATABASE bup_intr;
+
+    # before initializing the drivers
+    let $sync_point= restore_before_drivers_init;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+    DROP DATABASE bup_intr;
+
+    # before reading table data chunk
+    let $sync_point= restore_before_read_data_chunk;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+    DROP DATABASE bup_intr;
+
+    # before sending the data to a driver
+    let $sync_point= restore_before_sending_data;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+    DROP DATABASE bup_intr;
+
+    # before shutting down the drivers
+    let $sync_point= restore_table_data_before_end;
+    --source suite/backup_engines/include/backup_restore_interrupt.inc
+    DROP DATABASE bup_intr;
+
+  # before restoring triggers
+  let $sync_point= before_restore_triggers;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+  DROP DATABASE bup_intr;
+
+  # before reading image summary block
+  let $sync_point= before_restore_completed;
+  --source suite/backup_engines/include/backup_restore_interrupt.inc
+
+
+#
+# Cleanup
+# 
+
+SET DEBUG_SYNC='reset';
+--disable_warnings
+DROP DATABASE IF EXISTS bup_intr;
+--error 0,1
+--remove_file $bdir/bup_intr.bkp
+--enable_warnings
+

=== modified file 'sql/backup/backup_kernel.h'
--- a/sql/backup/backup_kernel.h	2009-02-13 13:25:43 +0000
+++ b/sql/backup/backup_kernel.h	2009-02-19 14:37:37 +0000
@@ -70,6 +70,7 @@ public:
   ~Backup_restore_ctx();
 
   bool is_valid() const;
+  bool is_killed() const;
   ulonglong op_id() const;
 
   Backup_info*  prepare_for_backup(String *location, 
@@ -208,6 +209,13 @@ bool Backup_restore_ctx::is_valid() cons
   return m_error == 0;
 }
 
+/// Check if the operation has been interrupted.
+inline
+bool Backup_restore_ctx::is_killed() const
+{
+  return m_thd->killed;
+}
+
 /// Return global id of the backup/restore operation.
 inline
 ulonglong Backup_restore_ctx::op_id() const

=== modified file 'sql/backup/be_default.h'
--- a/sql/backup/be_default.h	2008-12-18 21:46:36 +0000
+++ b/sql/backup/be_default.h	2009-02-19 14:37:37 +0000
@@ -70,6 +70,7 @@ public:
     { 
       mode= CANCEL;
       cleanup();
+      DBUG_EXECUTE_IF("backup_driver_cancel_error", return backup::ERROR;);
       return backup::OK;
     }
     /// Return table list containing all tables
@@ -148,6 +149,7 @@ public:
     { 
       mode= CANCEL;
       cleanup();
+      DBUG_EXECUTE_IF("backup_driver_cancel_error", return backup::ERROR;);
       return backup::OK;
     }
     void free() { delete this; };

=== modified file 'sql/backup/be_snapshot.h'
--- a/sql/backup/be_snapshot.h	2008-12-18 21:46:36 +0000
+++ b/sql/backup/be_snapshot.h	2009-02-19 14:37:37 +0000
@@ -54,6 +54,7 @@ public:
     { 
       m_cancel= TRUE;
       cleanup();
+      DBUG_EXECUTE_IF("backup_driver_cancel_error", return backup::ERROR;);
       return backup::OK;
     }
 private:

=== modified file 'sql/backup/be_thread.cc'
--- a/sql/backup/be_thread.cc	2009-02-13 13:25:43 +0000
+++ b/sql/backup/be_thread.cc	2009-02-19 14:37:37 +0000
@@ -238,6 +238,7 @@ end2:
   Constructor for Locking_thread_st structure.
 */
 Locking_thread_st::Locking_thread_st()
+ :m_thread_started(FALSE)
 {
   /*
     Initialize the thread mutex and cond variable.
@@ -257,13 +258,15 @@ Locking_thread_st::Locking_thread_st()
 Locking_thread_st::~Locking_thread_st()
 {
   /*
-    If the locking thread is not finished, we need to wait until
-    it is finished so that we can destroy the mutexes safely knowing
-    the locking thread won't access them.
+    If the locking thread has been started we need to kill it. We also need to 
+    wait until it dies before destroying the mutexes so that the locking thread 
+    won't access them any more.
   */
-  kill_locking_thread();
-  wait_until_locking_thread_dies();
-
+  if (m_thread_started)
+  {
+    kill_locking_thread();
+    wait_until_locking_thread_dies();
+  }
   /*
     Destroy the thread mutexes and cond variables.
   */
@@ -290,6 +293,7 @@ result_t Locking_thread_st::start_lockin
   if (pthread_create(&th, &connection_attrib,
                      backup_thread_for_locking, this))
     SET_STATE_TO_ERROR_AND_DBUG_RETURN;
+  m_thread_started= TRUE;
   DBUG_RETURN(backup::OK);
 }
 
@@ -303,6 +307,11 @@ result_t Locking_thread_st::start_lockin
 void Locking_thread_st::kill_locking_thread()
 {
   DBUG_ENTER("Locking_thread_st::kill_locking_thread");
+
+  // Nothing to do if the locking thread has not been started.
+  if (!m_thread_started)
+    DBUG_VOID_RETURN;
+
   pthread_mutex_lock(&THR_LOCK_caller);
   if (lock_state == LOCK_ERROR)
     THD_SET_PROC_INFO(m_thd, "error in the locking thread");
@@ -334,6 +343,10 @@ void Locking_thread_st::kill_locking_thr
 */
 void Locking_thread_st::wait_until_locking_thread_dies()
 {
+  // Nothing to do if the locking thread has not been started.
+  if (!m_thread_started)
+    return;
+
   pthread_mutex_lock(&THR_LOCK_caller);
   if (lock_state != LOCK_DONE)
   {
@@ -347,4 +360,6 @@ void Locking_thread_st::wait_until_locki
   }
   else
     pthread_mutex_unlock(&THR_LOCK_caller);
+
+  m_thread_started= FALSE;
 }

=== modified file 'sql/backup/be_thread.h'
--- a/sql/backup/be_thread.h	2008-12-18 21:46:36 +0000
+++ b/sql/backup/be_thread.h	2009-02-19 14:37:37 +0000
@@ -43,6 +43,12 @@ pthread_handler_t backup_thread_for_lock
   The Backup_thread structure contains a mutex and condition variable
   for using a thread to open and lock the tables. This is meant to be a
   generic class that can be used elsewhere for opening and locking tables.
+
+  @note This class correctly handles the separate locking thread. However, 
+  the class itself is *not* multi-thread safe. An instance of Locking_thread_st
+  should be used by one thread only. In particular, calling 
+  @c start_locking_thread() from one thread and @c kill_locking_thread() from
+  another thread running in parallel is not guaranteed to work.
 */
 struct Locking_thread_st
 {
@@ -60,6 +66,8 @@ public:
   LOCK_STATE lock_state;           ///< Current state of the lock call
   THD *m_thd;                      ///< Pointer to current thread struct.
   String thd_name;                 ///< Name of locking thread
+  /// Indicates if the locking thread has been started.
+  my_bool m_thread_started;
 
   result_t start_locking_thread(const char *tname);
   void kill_locking_thread();

=== modified file 'sql/backup/data_backup.cc'
--- a/sql/backup/data_backup.cc	2009-02-03 07:48:09 +0000
+++ b/sql/backup/data_backup.cc	2009-02-19 14:37:37 +0000
@@ -544,7 +544,8 @@ int write_table_data(THD* thd, Backup_in
   List<Scheduler::Pump>  inactive;  // list of images not yet being created
 
   // keeps maximal init size for images in inactive list
-  size_t      max_init_size=0;
+  size_t  max_init_size=0;
+  bool    commits_blocked= FALSE;   // indicates if commit blocker is active
 
   DBUG_PRINT("backup_data",("initializing scheduler"));
 
@@ -558,13 +559,12 @@ int write_table_data(THD* thd, Backup_in
       continue;
 
     Scheduler::Pump *p= new Scheduler::Pump(*i, s);
-
     if (!p)
     {
       log.report_error(ER_OUT_OF_RESOURCES);
       goto error;
     }
-    if (!p->is_valid())
+    if (log.report_killed() || !p->is_valid())
     {
       log.report_error(ER_BACKUP_CREATE_BACKUP_DRIVER,p->m_name);
       delete p;
@@ -684,7 +684,11 @@ int write_table_data(THD* thd, Backup_in
       non-transactional tables and pass that to block_commits().
     */
     int error= 0;
-    error= block_commits(thd, NULL);
+
+    if (!(error= block_commits(thd, NULL)))
+      commits_blocked= TRUE;
+    if (log.report_killed())
+      goto error;
     if (error)
     {
       log.report_error(ER_BACKUP_SYNCHRONIZE);
@@ -702,6 +706,12 @@ int write_table_data(THD* thd, Backup_in
     
     DBUG_PRINT("backup_data",("-- SYNC PHASE --"));
 
+    /*
+      Before proceeding, check if the process is interrupted.
+    */
+    if (log.report_killed())
+      goto error;
+
     log.report_state(BUP_VALIDITY_POINT);
     /*
       This breakpoint is used to assist in testing state changes for
@@ -715,13 +725,18 @@ int write_table_data(THD* thd, Backup_in
     */
 
     DEBUG_SYNC(thd, "before_backup_data_lock");
+    /*
+      Note: we do not detect/react to process interruptions during the 
+      synchronization. We let the protocol to go through and check for
+      possible interruptions only at the end, i.e., after sch.unlock().
+    */ 
     if (sch.lock())    // logs errors
       goto error;
 
     save_vp_info(info);
 
     DEBUG_SYNC(thd, "before_backup_data_unlock");
-    if (sch.unlock())    // logs errors
+    if (sch.unlock() || log.report_killed())    // logs errors
       goto error;
 
     /*
@@ -729,6 +744,9 @@ int write_table_data(THD* thd, Backup_in
     */
     DEBUG_SYNC(thd, "before_backup_unblock_commit");
     unblock_commits(thd);
+    commits_blocked= FALSE;
+    if (log.report_killed())
+      goto error;
 
 
     report_vp_info(info);
@@ -755,6 +773,8 @@ int write_table_data(THD* thd, Backup_in
 
  error:
 
+  if (commits_blocked)
+    unblock_commits(thd);
   DBUG_RETURN(ERROR);
 }
 
@@ -811,10 +831,14 @@ public:
   Pick next backup pump and call its @c pump() method.
 
   Method updates statistics of number of drivers in each phase which is used
-  to detect end of a backup process.
+  to detect end of a backup process. A check for interruption is done each time
+  this method is called.
  */
 int Scheduler::step()
 {
+  if (m_log.report_killed())
+    return ER_QUERY_INTERRUPTED;
+
   // Pick next driver to pump data from.
 
   Pump_iterator p(*this);
@@ -927,7 +951,7 @@ int Scheduler::add(Pump *p)
   p->set_logger(&m_log);
   p->start_pos= avg;
 
-  if (p->begin())  // logs errors
+  if (p->begin() || m_log.report_killed())  // logs errors
   {
     delete p;
     return ERROR;
@@ -1046,7 +1070,7 @@ int Scheduler::prepare()
 
   for (Pump_iterator it(*this); it; ++it)
   {
-    if (it->prepare())  // logs errors
+    if (it->prepare() || m_log.report_killed())  // logs errors
     {
       remove_pump(it);  // Note: never errors.
       return ERROR;
@@ -1138,6 +1162,16 @@ Backup_pump::~Backup_pump()
   bitmap_free(&m_closed_streams);
 }
 
+/*
+  Note: The standard report_error() method does not log an error in case
+  the statement has been interrupted - the interruption is reported instead.
+  But we want to always report errors signalled by a backup driver, even if an
+  interruption has just happened. Therefore, inside Backup_pump methods where 
+  errors from the driver are reported, we use the variant of 
+  Logger::report_error() with explicit log_level::ERROR. This variant does not 
+  check for interruptions and always reports given error.
+*/ 
+
 /// Initialize backup driver.
 int Backup_pump::begin()
 {
@@ -1148,7 +1182,8 @@ int Backup_pump::begin()
   {
     state= backup_state::ERROR;
     if (m_log)
-      m_log->report_error(ER_BACKUP_INIT_BACKUP_DRIVER, m_name);
+      m_log->report_error(log_level::ERROR,
+                          ER_BACKUP_INIT_BACKUP_DRIVER, m_name);
     return ERROR;
   }
 
@@ -1166,7 +1201,8 @@ int Backup_pump::end()
     {
       state= backup_state::ERROR;
       if (m_log)
-        m_log->report_error(ER_BACKUP_STOP_BACKUP_DRIVER, m_name);
+        m_log->report_error(log_level::ERROR,
+                            ER_BACKUP_STOP_BACKUP_DRIVER, m_name);
       return ERROR;
     }
 
@@ -1195,7 +1231,8 @@ int Backup_pump::prepare()
   default:
     state= backup_state::ERROR;
     if (m_log)
-      m_log->report_error(ER_BACKUP_PREPARE_DRIVER, m_name);
+      m_log->report_error(log_level::ERROR,
+                          ER_BACKUP_PREPARE_DRIVER, m_name);
     return ERROR;
   }
 
@@ -1212,7 +1249,8 @@ int Backup_pump::lock()
   {
     state= backup_state::ERROR;
     if (m_log)
-      m_log->report_error(ER_BACKUP_CREATE_VP, m_name);
+      m_log->report_error(log_level::ERROR, 
+                          ER_BACKUP_CREATE_VP, m_name);
     return ERROR;
   }
 
@@ -1228,7 +1266,8 @@ int Backup_pump::unlock()
   {
     state= backup_state::ERROR;
     if (m_log)
-      m_log->report_error(ER_BACKUP_UNLOCK_DRIVER, m_name);
+      m_log->report_error(log_level::ERROR, 
+                          ER_BACKUP_UNLOCK_DRIVER, m_name);
     return ERROR;
   }
 
@@ -1241,7 +1280,8 @@ int Backup_pump::cancel()
   {
     state= backup_state::ERROR;
     if (m_log)
-      m_log->report_error(ER_BACKUP_CANCEL_BACKUP, m_name);
+      m_log->report_error(log_level::ERROR, 
+                          ER_BACKUP_CANCEL_BACKUP, m_name);
     return ERROR;
   }
   state= backup_state::CANCELLED;
@@ -1367,7 +1407,8 @@ int Backup_pump::pump(size_t *howmuch)
       case ERROR:
       default:
         if (m_log)
-          m_log->report_error(ER_BACKUP_GET_DATA, m_name);
+          m_log->report_error(log_level::ERROR,
+                              ER_BACKUP_GET_DATA, m_name);
         state= backup_state::ERROR;
         return ERROR;
 
@@ -1485,6 +1526,7 @@ int restore_table_data(THD *thd, Restore
   }
 
   // Create restore drivers
+  DEBUG_SYNC(thd, "restore_before_drivers_create");
   result_t res;
 
   for (uint n=0; n < info.snap_count(); ++n)
@@ -1498,6 +1540,8 @@ int restore_table_data(THD *thd, Restore
       continue;
 
     res= snap->get_restore_driver(drv[n]);
+    if (log.report_killed())
+      goto error;
     if (res == backup::ERROR)
     {
       log.report_error(ER_BACKUP_CREATE_RESTORE_DRIVER, snap->name());
@@ -1506,9 +1550,12 @@ int restore_table_data(THD *thd, Restore
  }
 
   // Initialize the drivers.
+  DEBUG_SYNC(thd, "restore_before_drivers_init");
   for (uint n=0; n < info.snap_count(); ++n)
   {
     res= drv[n]->begin(0);
+    if (log.report_killed())
+      goto error;
     if (res == backup::ERROR)
     {
       log.report_error(ER_BACKUP_INIT_RESTORE_DRIVER, info.m_snap[n]->name());
@@ -1539,9 +1586,13 @@ int restore_table_data(THD *thd, Restore
 
       case READING:
 
+        DEBUG_SYNC(thd, "restore_before_read_data_chunk");
         bzero(&chunk_info, sizeof(chunk_info));
         ret= bstream_rd_data_chunk(&s, &chunk_info);
 
+        if (log.report_killed())
+          goto error;
+
         /* Mimic error in bstream_rd_data_chunk */
         DBUG_EXECUTE_IF("restore_tbl_data_read", ret= BSTREAM_ERROR;);
 
@@ -1602,7 +1653,12 @@ int restore_table_data(THD *thd, Restore
            Note: For testing, error can be injected in default driver
            send_data by using dbug hook 'backup_default_send_data'
         */
+        DEBUG_SYNC(thd, "restore_before_sending_data");
         ret= drvr->send_data(buf);
+
+        if (log.report_killed())
+          goto error;
+
         switch (ret) {
 
         case backup::OK:
@@ -1663,6 +1719,17 @@ int restore_table_data(THD *thd, Restore
     DBUG_PRINT("restore",("Shutting down restore driver %s",
                            info.m_snap[n]->name()));
     res= active[n]->end();
+    if (log.report_killed())
+    {
+      /* 
+        If ->end() call has failed we set active[n] to NULL as the driver is
+        no longer active and its ->cancel() method should not be called when 
+        we jump to error:
+      */
+      if (res == backup::ERROR)
+        active[n]= NULL; 
+      goto error;
+    }
     if (res == backup::ERROR)
     {
       state= ERROR;
@@ -1691,7 +1758,6 @@ error:
     DBUG_PRINT("restore",("Cancelling restore driver %s",
                            info.m_snap[n]->name()));
     res= active[n]->cancel();
-
     if (res)
     {
       if (!bad_drivers.is_empty())
@@ -1702,8 +1768,14 @@ error:
 
 finish:  
 
+  /*
+    We always log shutdown errors, even if an interruption has happened before.
+    This is why we must supply explicit log_level::ERROR to report_error() as
+    otherwise the method will report interruption instead of an error.
+  */
   if (!bad_drivers.is_empty())
-    log.report_error(ER_BACKUP_STOP_RESTORE_DRIVERS, bad_drivers.c_ptr());
+    log.report_error(log_level::ERROR, 
+                     ER_BACKUP_STOP_RESTORE_DRIVERS, bad_drivers.c_ptr());
 
   // Call free() for all existing drivers
 

=== modified file 'sql/backup/kernel.cc'
--- a/sql/backup/kernel.cc	2009-02-13 13:25:43 +0000
+++ b/sql/backup/kernel.cc	2009-02-19 14:37:37 +0000
@@ -170,6 +170,7 @@ execute_backup_command(THD *thd, 
   {
     // prepare for backup operation
     
+    DEBUG_SYNC(thd, "before_backup_prepare");
     Backup_info *info= context.prepare_for_backup(backupdir, lex->backup_dir, 
                                                   thd->query,
                                                   lex->backup_compression);
@@ -203,8 +204,10 @@ execute_backup_command(THD *thd, 
 
     // perform backup
 
+    DEBUG_SYNC(thd, "before_do_backup");
     res= context.do_backup();
  
+    DEBUG_SYNC(thd, "after_do_backup");
     if (res)
       DBUG_RETURN(send_error(context, ER_BACKUP_BACKUP));
 
@@ -213,6 +216,7 @@ execute_backup_command(THD *thd, 
 
   case SQLCOM_RESTORE:
   {
+    DEBUG_SYNC(thd, "before_restore_prepare");
 
     Restore_info *info= context.prepare_for_restore(backupdir, lex->backup_dir, 
                                                     thd->query, skip_gap_event);
@@ -241,7 +245,10 @@ execute_backup_command(THD *thd, 
 
   } // switch(lex->sql_command)
 
-  if (context.close())
+  res= context.close();
+  DEBUG_SYNC(thd, "backup_restore_done");
+
+  if (res)
     DBUG_RETURN(send_error(context, ER_BACKUP_CONTEXT_REMOVE));
 
   // All seems OK - send positive reply to client
@@ -255,16 +262,17 @@ execute_backup_command(THD *thd, 
   @param[in]  ctx  The context of the backup/restore operation.
   @param[in]  error_code  Error to be reported if no errors reported yet.
 
-  If an error has been already reported then nothing is done - the first 
-  logged error will be send to the client. Otherwise, if no errors were 
-  reported yet, the given error is sent to the client (but not reported).
+  If an error has been already reported, or process has been interrupted,
+  then nothing is done - the first logged error or kill message will be sent 
+  to the client. Otherwise, if no errors were reported yet and no interruption
+  has been detected the given error is sent to the client (but not reported).
   
   @returns The error code given as argument.
 */
 static
 int send_error(Backup_restore_ctx &context, int error_code, ...)
 {
-  if (!context.error_reported())
+  if (!context.is_killed() && !context.error_reported())
   {
     char buf[MYSQL_ERRMSG_SIZE];
     va_list args;
@@ -388,12 +396,15 @@ Backup_restore_ctx::Backup_restore_ctx(T
     Check for progress tables.
   */
   MYSQL_BACKUP_LOG *backup_log= logger.get_backup_history_log_file_handler();
-  if (backup_log->check_backup_logs(thd))
+  if (report_killed())
+    m_error= ER_QUERY_INTERRUPTED;
+  else if (backup_log->check_backup_logs(thd))
     m_error= ER_BACKUP_PROGRESS_TABLES;
 }
 
 Backup_restore_ctx::~Backup_restore_ctx()
 {
+  DEBUG_SYNC(m_thd, "backup_restore_ctx_dtor");
   close();
 
   delete mem_alloc;
@@ -500,8 +511,9 @@ int Backup_restore_ctx::prepare(::String
     In case of error, we write only to backup logs, because check_global_access()
     pushes the same error on the error stack.
   */
+  DEBUG_SYNC(m_thd, "before_backup_privileges");
   ret= check_global_access(m_thd, SUPER_ACL);
-  if (ret)
+  if (ret || is_killed())
     return fatal_error(log_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, "SUPER"));
 
   /*
@@ -509,6 +521,7 @@ int Backup_restore_ctx::prepare(::String
     this operation.
    */
 
+  DEBUG_SYNC(m_thd, "before_backup_single_op");
   pthread_mutex_lock(&run_lock);
 
   if (!current_op)
@@ -543,7 +556,7 @@ int Backup_restore_ctx::prepare(::String
 
 #endif
 
-  if (bad_filename)
+  if (bad_filename || is_killed())
     return fatal_error(report_error(ER_BAD_PATH, location.str));
 
   /*
@@ -563,8 +576,9 @@ int Backup_restore_ctx::prepare(::String
 
   // Freeze all meta-data. 
 
+  DEBUG_SYNC(m_thd, "before_backup_ddl_block");
   ret= obs::bml_get(m_thd);
-  if (ret)
+  if (ret || is_killed())
     return fatal_error(report_error(ER_DDL_BLOCK));
 
   DEBUG_SYNC(m_thd, "after_backup_restore_prepare");
@@ -601,14 +615,16 @@ Backup_restore_ctx::prepare_for_backup(S
   // Do nothing if context is in error state.
   if (m_error)
     return NULL;
-  
+
   /*
    Note: Logger must be initialized before any call to report_error() - 
    otherwise an assertion will fail.
   */ 
-  if (Logger::init(BACKUP, query))      // Logs errors
+  DEBUG_SYNC(m_thd, "before_backup_logger_init");
+  int ret= Logger::init(BACKUP, query);  // Logs errors   
+  if (ret || is_killed())
   {
-    fatal_error(ER_BACKUP_LOGGER_INIT);
+    fatal_error(report_killed() ? ER_QUERY_INTERRUPTED : ER_BACKUP_LOGGER_INIT);
     return NULL;
   }
 
@@ -619,7 +635,8 @@ Backup_restore_ctx::prepare_for_backup(S
     Do preparations common to backup and restore operations. After call
     to prepare() all meta-data changes are blocked.
    */ 
-  if (prepare(backupdir, orig_loc))
+  DEBUG_SYNC(m_thd, "before_backup_common_prepare");
+  if (prepare(backupdir, orig_loc))  // Logs errors and detects interruptions
     return NULL;
 
   /*
@@ -630,17 +647,30 @@ Backup_restore_ctx::prepare_for_backup(S
                                       &m_path,
                                       with_compression);
   m_stream= s;
-  
-  if (!s)
+
+  if (!s || is_killed())
   {
     fatal_error(report_error(ER_OUT_OF_RESOURCES));
     return NULL;
   }
 
+  DEBUG_SYNC(m_thd, "before_backup_stream_open");
   int my_open_status= s->open();
-  if (my_open_status != 0)
+
+  /*
+    Set state to PREPARED_FOR_BACKUP in case output file was opened successfuly.
+    It is important to set the state here, because this ensures that the file
+    will be removed in case of error or interruption.
+   */ 
+  if (! my_open_status)
+    m_state= PREPARED_FOR_BACKUP;
+  
+  if (my_open_status != 0 || is_killed())
   {
-    report_stream_open_failure(my_open_status, &orig_loc);
+    if (report_killed())
+      fatal_error(ER_QUERY_INTERRUPTED);
+    else
+      report_stream_open_failure(my_open_status, &orig_loc);
     return NULL;
   }
 
@@ -648,14 +678,15 @@ Backup_restore_ctx::prepare_for_backup(S
     Create backup catalogue.
    */
 
+  DEBUG_SYNC(m_thd, "before_backup_catalog");
   Backup_info *info= new Backup_info(*this, m_thd);    // Logs errors
+  m_catalog= info;
 
-  if (!info)
+  if (!info || is_killed())
   {
     fatal_error(report_error(ER_OUT_OF_RESOURCES));
     return NULL;
   }
-
   if (!info->is_valid())
   {
     // Error has been logged by Backup_info constructor
@@ -679,8 +710,6 @@ Backup_restore_ctx::prepare_for_backup(S
     info->flags|= BSTREAM_FLAG_BINLOG; 
 
   info->save_start_time(when);
-  m_catalog= info;
-  m_state= PREPARED_FOR_BACKUP;  
 
   return info;
 }
@@ -715,7 +744,9 @@ Backup_restore_ctx::prepare_for_restore(
    Note: Logger must be initialized before any call to report_error() - 
    otherwise an assertion will fail.
   */ 
-  if (Logger::init(RESTORE, query))
+  DEBUG_SYNC(m_thd, "before_restore_logger_init");
+  int ret= Logger::init(RESTORE, query);  // Logs errors
+  if (ret || is_killed())
   {
     fatal_error(ER_BACKUP_LOGGER_INIT);
     return NULL;
@@ -742,6 +773,7 @@ Backup_restore_ctx::prepare_for_restore(
     Do preparations common to backup and restore operations. After this call
     changes of meta-data are blocked.
    */ 
+  DEBUG_SYNC(m_thd, "before_restore_common_prepare");
   if (prepare(backupdir, orig_loc))
     return NULL;
   
@@ -752,16 +784,20 @@ Backup_restore_ctx::prepare_for_restore(
   Input_stream *s= new Input_stream(*this, &m_path);
   m_stream= s;
   
-  if (!s)
+  if (!s || is_killed())
   {
     fatal_error(report_error(ER_OUT_OF_RESOURCES));
     return NULL;
   }
   
+  DEBUG_SYNC(m_thd, "before_restore_stream_open");
   int my_open_status= s->open();
-  if (my_open_status != 0)
+  if (my_open_status != 0 || is_killed())
   {
-    report_stream_open_failure(my_open_status, &orig_loc);
+    if (report_killed())
+      fatal_error(ER_QUERY_INTERRUPTED);
+    else
+      report_stream_open_failure(my_open_status, &orig_loc);
     return NULL;
   }
 
@@ -769,14 +805,15 @@ Backup_restore_ctx::prepare_for_restore(
     Create restore catalogue.
    */
 
+  DEBUG_SYNC(m_thd, "before_restore_catalog");
   Restore_info *info= new Restore_info(*this, m_thd);  // reports errors
+  m_catalog= info;
 
-  if (!info)
+  if (!info || is_killed())
   {
     fatal_error(report_error(ER_OUT_OF_RESOURCES));
     return NULL;
   }
-
   if (!info->is_valid())
   {
     // Errors are logged by Restore_info constructor. 
@@ -785,18 +822,18 @@ Backup_restore_ctx::prepare_for_restore(
   }
 
   info->save_start_time(when);
-  m_catalog= info;
-
-  int ret;
 
   /*
     Read header and catalogue from the input stream.
    */
 
+  DEBUG_SYNC(m_thd, "before_restore_read_header");
   ret= read_header(*info, *s);  // Can log errors via callback functions.
-  if (ret)
+  if (ret || is_killed())
   {
-    if (!error_reported())
+    if (report_killed())
+      ret= ER_QUERY_INTERRUPTED;
+    else if (!error_reported())
       report_error(ER_BACKUP_READ_HEADER);
     fatal_error(ret);
     return NULL;
@@ -811,10 +848,13 @@ Backup_restore_ctx::prepare_for_restore(
     return NULL;
   }
 
+  DEBUG_SYNC(m_thd, "before_restore_read_catalog");
   ret= read_catalog(*info, *s);  // Can log errors via callback functions.
-  if (ret)
+  if (ret || is_killed())
   {
-    if (!error_reported())
+    if (report_killed())
+      ret= ER_QUERY_INTERRUPTED;
+    else if (!error_reported())
       report_error(ER_BACKUP_READ_HEADER);
     fatal_error(ret);
     return NULL;
@@ -823,7 +863,7 @@ Backup_restore_ctx::prepare_for_restore(
   ret= s->next_chunk();
   /* Mimic error in next_chunk */
   DBUG_EXECUTE_IF("restore_prepare_next_chunk_2", ret= BSTREAM_ERROR; );
-  if (ret != BSTREAM_OK)
+  if (ret != BSTREAM_OK || is_killed())
   {
     fatal_error(report_error(ER_BACKUP_NEXT_CHUNK));
     return NULL;
@@ -831,6 +871,7 @@ Backup_restore_ctx::prepare_for_restore(
 
   m_state= PREPARED_FOR_RESTORE;
 
+  DEBUG_SYNC(m_thd, "before_restore_binlog");
   /*
     Do not allow slaves to connect during a restore.
 
@@ -851,6 +892,11 @@ Backup_restore_ctx::prepare_for_restore(
     obs::engage_binlog(FALSE);
   }
 
+  if (report_killed())
+  {
+    fatal_error(ER_QUERY_INTERRUPTED);
+    return NULL;
+  }
   return info;
 }
 
@@ -928,7 +974,7 @@ int Backup_restore_ctx::lock_tables_for_
                                     MYSQL_OPEN_SKIP_TEMPORARY 
                                           /* do not open tmp tables */
                                    );
-  if (ret)
+  if (ret || is_killed())
     return fatal_error(report_error(ER_BACKUP_OPEN_TABLES,"RESTORE"));
 
   m_tables_locked= TRUE;
@@ -986,6 +1032,15 @@ int Backup_restore_ctx::close()
   if (m_state == CLOSED)
     return 0;
 
+  /*
+    Note: The standard report_error() method does not log an error in case
+    the statement has been interrupted - the interruption is reported instead.
+    But we want to always report errors which happen during shutdown phase.
+    Therefore, for reporting errors here we use the variant of 
+    Logger::report_error() with explicit log_level::ERROR. This variant does not 
+    check for interruptions and always reports given error.
+  */ 
+
   using namespace backup;
 
   // Move context to error state if the catalog became corrupted.
@@ -1048,7 +1103,7 @@ int Backup_restore_ctx::close()
   if (m_stream && !m_stream->close())
   {
     // Note error, but complete clean-up
-    fatal_error(report_error(ER_BACKUP_CLOSE));
+    fatal_error(report_error(log_level::ERROR, ER_BACKUP_CLOSE));
   }
 
   /* 
@@ -1060,13 +1115,14 @@ int Backup_restore_ctx::close()
   if (m_state == PREPARED_FOR_BACKUP && !m_completed)
   {
     int ret= my_delete(m_path.c_ptr(), MYF(0));
-
+    DBUG_EXECUTE_IF("backup_remove_location_error", ret= TRUE;);
     /*
       Ignore ENOENT error since it is ok if the file doesn't exist.
      */
     if (ret && my_errno != ENOENT)
     {
-      fatal_error(report_error(ER_CANT_DELETE_FILE, m_path.c_ptr(), my_errno));
+      fatal_error(report_error(log_level::ERROR, ER_CANT_DELETE_FILE, 
+                               m_path.c_ptr(), my_errno));
     }
   }
 
@@ -1120,13 +1176,18 @@ int Backup_restore_ctx::do_backup()
 
   report_stats_pre(info);                       // Never errors
 
+  if (report_killed())
+    DBUG_RETURN(fatal_error(ER_QUERY_INTERRUPTED));
+
   DBUG_PRINT("backup",("Writing preamble"));
   DEBUG_SYNC(m_thd, "backup_before_write_preamble");
 
   ret= write_preamble(info, s);  // Can Log errors via callback functions.
-  if (ret)
+  if (ret || is_killed())
   {
-    if (!error_reported())
+    if (report_killed())
+      ret= ER_QUERY_INTERRUPTED;
+    else if (!error_reported())
       report_error(ER_BACKUP_WRITE_HEADER);
     DBUG_RETURN(fatal_error(ret));
   }
@@ -1135,14 +1196,15 @@ int Backup_restore_ctx::do_backup()
 
   DEBUG_SYNC(m_thd, "before_backup_data");
 
-  ret= write_table_data(m_thd, info, s); // logs errors
+  ret= write_table_data(m_thd, info, s); // logs errors and detects interruptions
   if (ret)
     DBUG_RETURN(fatal_error(ret));
 
   DBUG_PRINT("backup",("Writing summary"));
 
+  DEBUG_SYNC(m_thd, "before_backup_summary");
   ret= write_summary(info, s);  
-  if (ret)
+  if (ret || is_killed())
     DBUG_RETURN(fatal_error(report_error(ER_BACKUP_WRITE_SUMMARY)));
 
   DEBUG_SYNC(m_thd, "before_backup_completed");
@@ -1177,83 +1239,97 @@ int Backup_restore_ctx::restore_triggers
 
   DBUG_ENTER("restore_triggers_and_events");
 
-  DBUG_ASSERT(m_type == RESTORE);
-  Restore_info *info= static_cast<Restore_info*>(m_catalog);
   Image_info::Obj *obj;
   List<Image_info::Obj> events;
   Image_info::Obj::describe_buf buf;
-
-  Image_info::Iterator *dbit= info->get_dbs();
-  if (!dbit)
-    DBUG_RETURN(fatal_error(report_error(ER_OUT_OF_RESOURCES)));
+  // Note: The two iterators below must be deleted upon exit.
+  Image_info::Iterator *trgit= NULL;
+  Image_info::Iterator *dbit= m_catalog->get_dbs();
+  if (!dbit || is_killed())
+  {
+    fatal_error(report_error(ER_OUT_OF_RESOURCES));
+    goto exit;
+  }
 
   // create all trigers and collect events in the events list
   
   while ((obj= (*dbit)++)) 
   {
-    Image_info::Iterator *it=
-                       info->get_db_objects(*static_cast<Image_info::Db*>(obj));
-    if (!it)
-      DBUG_RETURN(fatal_error(report_error(ER_OUT_OF_RESOURCES)));
+    trgit= m_catalog->get_db_objects(*static_cast<Image_info::Db*>(obj));
+    if (!trgit || is_killed())
+    {
+      fatal_error(report_error(ER_OUT_OF_RESOURCES));
+      goto exit;
+    }
 
-    while ((obj= (*it)++))
+    while ((obj= (*trgit)++))
       switch (obj->type()) {
       
       case BSTREAM_IT_EVENT:
         DBUG_ASSERT(obj->m_obj_ptr);
         if (events.push_back(obj))
         {
-          delete it;
-          delete dbit;
           // Error has been reported, but not logged to backup logs
-          DBUG_RETURN(fatal_error(log_error(ER_OUT_OF_RESOURCES))); 
+          fatal_error(log_error(ER_OUT_OF_RESOURCES));
+          goto exit; 
         }
         break;
       
       case BSTREAM_IT_TRIGGER:
       {
         DBUG_ASSERT(obj->m_obj_ptr);
-        // Mark that data is being changed.
-        info->m_data_changed= TRUE;
-
-        bool ret= obj->m_obj_ptr->create(m_thd);
+        int ret= obj->m_obj_ptr->create(m_thd);
         /* Mimic error in restore of trigger */
         DBUG_EXECUTE_IF("restore_trigger", ret= TRUE;); 
-        if (ret)
+        if (ret || is_killed())
         {
-          delete it;
-          delete dbit;
-          int err= report_error(ER_BACKUP_CANT_RESTORE_TRIGGER,
-                                obj->describe(buf));
-          DBUG_RETURN(fatal_error(err));
+          fatal_error(report_error(ER_BACKUP_CANT_RESTORE_TRIGGER,
+                                   obj->describe(buf)));
+          goto exit;
         }
         break;
       }
       default: break;      
       }
 
-    delete it;
+    delete trgit;
+    trgit= NULL;
   }
 
   delete dbit;
+  dbit= NULL;
 
   // now create all events
-
-  List_iterator<Image_info::Obj> it(events);
-  Image_info::Obj *ev;
-
-  while ((ev= it++)) 
+  
   {
-    // Mark that data is being changed.
-    info->m_data_changed= TRUE;
-    if (ev->m_obj_ptr->create(m_thd))
+    List_iterator<Image_info::Obj> it(events);
+    Image_info::Obj *ev;
+
+    while ((ev= it++))
     {
-      int ret= report_error(ER_BACKUP_CANT_RESTORE_EVENT,ev->describe(buf));
-      DBUG_RETURN(fatal_error(ret));
-    };
+      int ret= ev->m_obj_ptr->create(m_thd); 
+      if (ret || is_killed())
+      {
+        fatal_error(report_error(ER_BACKUP_CANT_RESTORE_EVENT,ev->describe(buf)));
+        goto exit;
+      }
+    }
   }
 
-  DBUG_RETURN(0);
+  /* 
+    FIXME: this call is here because object services doesn't clean the
+    statement execution context properly, which leads to assertion failure.
+    It should be fixed inside object services implementation and then the
+    following line should be removed (see BUG#41294).
+   */
+  close_thread_tables(m_thd);                   // Never errors
+  m_thd->clear_error();                         // Never errors
+
+exit:
+
+  delete dbit;
+  delete trgit;
+  DBUG_RETURN(m_error);
 }
 
 /**
@@ -1288,6 +1364,9 @@ int Backup_restore_ctx::do_restore(bool 
 
   report_stats_pre(info);                       // Never errors
 
+  if (report_killed())
+    DBUG_RETURN(fatal_error(ER_QUERY_INTERRUPTED));
+
   DBUG_PRINT("restore", ("Restoring meta-data"));
 
   // unless RESTORE... OVERWRITE: return error if database already exists
@@ -1301,7 +1380,8 @@ int Backup_restore_ctx::do_restore(bool 
     Image_info::Db *mydb;
     while ((mydb= static_cast<Image_info::Db*>((*dbit)++)))
     {
-      if (!obs::check_db_existence(m_thd, &mydb->name()))
+      err= obs::check_db_existence(m_thd, &mydb->name());
+      if (!err || is_killed()) 
       {
         delete dbit;
         err= report_error(ER_RESTORE_DB_EXISTS, mydb->name().ptr());
@@ -1311,12 +1391,19 @@ int Backup_restore_ctx::do_restore(bool 
     delete dbit;
   }
 
+  DEBUG_SYNC(m_thd, "before_restore_fkey_disable");
   disable_fkey_constraints();                   // Never errors
 
-  err= read_meta_data(info, s);  // Can log errors via callback functions.
-  if (err)
-  {
-    if (!error_reported())
+  if (report_killed())
+    DBUG_RETURN(fatal_error(ER_QUERY_INTERRUPTED));
+
+  DEBUG_SYNC(m_thd, "before_restore_read_metadata");
+  err= read_meta_data(m_thd, info, s); // Can log errors via callback functions.
+  if (err || is_killed())
+  {
+    if (report_killed())
+      err= ER_QUERY_INTERRUPTED;
+    else if (!error_reported())
       report_error(ER_BACKUP_READ_META);
     DBUG_RETURN(fatal_error(err));
   }
@@ -1324,37 +1411,29 @@ int Backup_restore_ctx::do_restore(bool 
   err= s.next_chunk();
   /* Mimic error in next_chunk */
   DBUG_EXECUTE_IF("restore_stream_next_chunk", err= BSTREAM_ERROR; );
-  if (err == BSTREAM_ERROR)
+  if (err == BSTREAM_ERROR || is_killed())
   {
     DBUG_RETURN(fatal_error(report_error(ER_BACKUP_NEXT_CHUNK)));
   }
 
   DBUG_PRINT("restore",("Restoring table data"));
 
-  /* 
-    FIXME: this call is here because object services doesn't clean the
-    statement execution context properly, which leads to assertion failure.
-    It should be fixed inside object services implementation and then the
-    following line should be removed.
-   */
-  close_thread_tables(m_thd);                   // Never errors
-  m_thd->stmt_da->reset_diagnostics_area();     // Never errors
-
+  DEBUG_SYNC(m_thd, "before_restore_lock_tables");
   err= lock_tables_for_restore();               // logs errors
-  if (err)
-    DBUG_RETURN(fatal_error(err));
+  if (err || is_killed())
+    DBUG_RETURN(fatal_error(report_killed() ? ER_QUERY_INTERRUPTED : err));
 
   /* 
    Here restore drivers are created to restore table data. Data is being
    (potentially) changed so we set m_data_changed flag.
   */
   info.m_data_changed= TRUE;
+  DEBUG_SYNC(m_thd, "before_restore_table_data");
   err= restore_table_data(m_thd, info, s);      // logs errors
 
   unlock_tables();                              // Never errors
-
-  if (err)
-    DBUG_RETURN(fatal_error(err));
+  if (err || is_killed())
+    DBUG_RETURN(fatal_error(report_killed() ? ER_QUERY_INTERRUPTED : err));
 
   /* 
    Re-create all triggers and events (it was not done in @c bcat_create_item()).
@@ -1363,26 +1442,18 @@ int Backup_restore_ctx::do_restore(bool 
    creation of these objects will fail.
   */
 
-  err= restore_triggers_and_events();           // logs errors
+  DEBUG_SYNC(m_thd, "before_restore_triggers");
+  err= restore_triggers_and_events();   // logs errors and detects interruptions
   if (err)
      DBUG_RETURN(fatal_error(err));
 
-  /* 
-    FIXME: this call is here because object services doesn't clean the
-    statement execution context properly, which leads to assertion failure.
-    It should be fixed inside object services implementation and then the
-    following line should be removed.
-   */
-  close_thread_tables(m_thd);                   // Never errors
-  m_thd->stmt_da->reset_diagnostics_area();     // Never errors
-
   DBUG_PRINT("restore",("Done."));
 
   DEBUG_SYNC(m_thd, "before_restore_completed");
   m_completed= TRUE;
 
   err= read_summary(info, s);
-  if (err)
+  if (err || is_killed())
     DBUG_RETURN(fatal_error(report_error(ER_BACKUP_READ_SUMMARY)));
 
   /*

=== modified file 'sql/backup/logger.cc'
--- a/sql/backup/logger.cc	2009-02-16 12:20:31 +0000
+++ b/sql/backup/logger.cc	2009-02-19 14:37:37 +0000
@@ -156,6 +156,7 @@ void Logger::report_stats_pre(const Imag
 {
   DBUG_ASSERT(m_state == RUNNING);
   backup_log->num_objects(info.object_count());
+
   // Compose list of databases.
 
   Image_info::Db_iterator *it= info.get_dbs();
@@ -222,4 +223,48 @@ bool Logger::push_errors(bool flag)
   return old;
 } 
 
+/**
+  Report the fact that BACKUP/RESTORE operation has been cancelled.
+
+  This method does nothing if no interruption has happened or if an 
+  interruption has already been reported. Otherwise it logs
+  ER_BACKUP_INTERRUPTED note.
+
+  @note The server takes care of giving feedback to the client in case of a 
+  cancelled statement - nothing needs to be done here (nothing pushed on the 
+  server's error stack)
+
+  @returns TRUE if interruption has been detected and reported, 
+  FALSE otherwise.
+ */ 
+bool Logger::report_killed()
+{
+
+  if (!m_thd->killed)
+    return FALSE;
+
+  if (m_kill_reported)
+    return TRUE;
+
+  m_thd->send_kill_message();
+  m_kill_reported= TRUE;
+
+  if (m_state == CREATED || m_state == READY)
+    return TRUE;
+
+  // log_error() does not push messages on the server's error stack.
+  log_error(backup::log_level::INFO, ER_BACKUP_INTERRUPTED);  
+
+  /*
+    Note: It is not possible to open online backup log tables if current
+    query has been killed (m_thd->killed is non-zero). Thus in that case 
+    we can't update state accordingly. This is a limitation of the current 
+    implementation of the online backup logging mechanism.
+  */ 
+  if (m_state == RUNNING)
+    report_state(BUP_CANCEL);
+
+  return TRUE;
+}
+
 } // backup namespace

=== modified file 'sql/backup/logger.h'
--- a/sql/backup/logger.h	2009-02-13 13:25:43 +0000
+++ b/sql/backup/logger.h	2009-02-19 14:37:37 +0000
@@ -57,6 +57,7 @@ public:
    int report_error(const char *format, ...);
    int write_message(log_level::value level, const char *msg, ...);
    int log_error(int error_code, ...);
+   int log_error(log_level::value level, int error_code, ...);
 
    void report_start(time_t);
    void report_completed(time_t);
@@ -69,6 +70,7 @@ public:
    void report_backup_file(char * path);
    void report_stats_pre(const Image_info&);
    void report_stats_post(const Image_info&);
+   bool report_killed();
 
    /// Return the Backup_id of the current operation
    ulonglong get_op_id() const 
@@ -97,6 +99,8 @@ private:
 
   bool m_push_errors;        ///< Should errors be pushed on warning stack?
   bool m_error_reported;     ///< Has any error been reported?
+  /// Flag preventing double reporting of process interruption.
+  bool m_kill_reported;
 
   Backup_log *backup_log;    ///< Backup log interface class.
 };
@@ -104,7 +108,7 @@ private:
 inline
 Logger::Logger(THD *thd) 
    :m_type(BACKUP), m_state(CREATED), m_thd(thd), m_push_errors(TRUE), 
-    m_error_reported(FALSE), backup_log(0)
+    m_error_reported(FALSE), m_kill_reported(FALSE), backup_log(0)
 {}
 
 inline
@@ -126,10 +130,18 @@ int Logger::write_message(log_level::val
   return res;
 }
 
-/// Reports error with log_level::ERROR.
+/** 
+  Reports error with log_level::ERROR.
+
+  Before reporting error, this method checks for interruption. In case there 
+  was one, the interruption is reported instead.
+*/
 inline
 int Logger::report_error(int error_code, ...)
 {
+  if (report_killed())
+    return ER_QUERY_INTERRUPTED;
+
   va_list args;
 
   va_start(args, error_code);
@@ -152,10 +164,21 @@ int Logger::report_error(log_level::valu
   return res;
 }
 
-/// Reports error with given description.
+/** 
+  Reports given message with log_level::ERROR.
+
+  This method can be used for reporting errors which are not registered in 
+  errmsg.txt.
+
+  Before reporting the message, this method checks for interruption. In case 
+  there was one, the interruption is reported instead.
+*/
 inline
 int Logger::report_error(const char *format, ...)
 {
+  if (report_killed())
+    return ER_QUERY_INTERRUPTED;
+
   va_list args;
 
   va_start(args, format);
@@ -165,10 +188,18 @@ int Logger::report_error(const char *for
   return res;
 }
 
-/// Reports error without pushing it on server's error stack.
+/** 
+  Reports error with log_level::ERROR without pushing it on error stack. 
+
+  Before reporting the error, this method checks for interruption. In case 
+  there was one, the interruption is reported instead.
+*/
 inline
 int Logger::log_error(int error_code, ...)
 {
+  if (report_killed())
+    return ER_QUERY_INTERRUPTED;
+
   va_list args;
   bool    saved= push_errors(FALSE);
   
@@ -181,6 +212,25 @@ int Logger::log_error(int error_code, ..
   return res;
 }
 
+/** 
+  Reports error on the given log_level, without pushing it on server's 
+  error stack.
+*/
+inline
+int Logger::log_error(log_level::value level, int error_code, ...)
+{
+  va_list args;
+  bool    saved= push_errors(FALSE);
+
+  va_start(args, error_code);
+  int res= v_report_error(level, error_code, args);
+  va_end(args);
+
+  push_errors(saved);
+
+  return res;
+}
+
 /// Report start of an operation.
 inline
 void Logger::report_start(time_t when)

=== modified file 'sql/backup/stream.cc'
--- a/sql/backup/stream.cc	2009-02-09 18:17:55 +0000
+++ b/sql/backup/stream.cc	2009-02-19 14:37:37 +0000
@@ -269,12 +269,14 @@ int Stream::open()
 bool Stream::close()
 {
   bool ret= TRUE;
+
   if (m_fd >= 0)
   {
     if (my_close(m_fd, MYF(0)))
     {
       ret= FALSE;
     }
+    DBUG_EXECUTE_IF("backup_stream_close_error", ret= FALSE;);
     m_fd= -1;
   }
   return ret;

=== modified file 'sql/backup/stream.h'
--- a/sql/backup/stream.h	2009-02-06 08:28:24 +0000
+++ b/sql/backup/stream.h	2009-02-19 14:37:37 +0000
@@ -244,16 +244,34 @@ read_catalog(Image_info &info, Input_str
 /**
   Read the metadata.
 
+  @param[in]  thd   Connection thread handle. 
   @param[in]  info  The image info.
   @param[in]  s     The input stream.
 
   @retval  ERROR if stream error, OK if no errors.
+
+  FIXME: the thd parameter for read_meta_data() is here only because the
+  temporary code within the function needs it. It should be removed once the
+  issue is fixed (see BUG#41294).
 */
 inline
 result_t
-read_meta_data(Image_info &info, Input_stream &s)
+read_meta_data(THD *thd, Image_info &info, Input_stream &s)
 {
   int ret= bstream_rd_meta_data(&s, static_cast<st_bstream_image_header*>(&info));
+
+  /* 
+    FIXME: the following code is here because object services doesn't clean the
+    statement execution context properly, which leads to assertion failure.
+    It should be fixed inside object services implementation and then the
+    following line should be removed (see BUG#41294).
+   */
+  DBUG_ASSERT(thd);
+  close_thread_tables(thd);                   // Never errors
+  if (ret != BSTREAM_ERROR)
+    thd->clear_error();                       // Never errors  
+  /* end of temporary code */
+
   DBUG_EXECUTE_IF("restore_read_meta_data", ret= BSTREAM_ERROR;);
   return ret == BSTREAM_ERROR ? ERROR : OK;
 }

=== modified file 'sql/share/errmsg.txt'
--- a/sql/share/errmsg.txt	2009-01-29 21:17:59 +0000
+++ b/sql/share/errmsg.txt	2009-02-19 14:37:37 +0000
@@ -6241,7 +6241,7 @@ ER_BACKUP_INIT_RESTORE_DRIVER
 ER_BACKUP_STOP_BACKUP_DRIVER
         eng "Can't shut down %-.64s backup driver"
 ER_BACKUP_STOP_RESTORE_DRIVERS
-        eng "Can't shut down %-.64s backup driver(s)"
+        eng "Can't shut down %-.64s restore driver(s)"
 ER_BACKUP_PREPARE_DRIVER
         eng "%-.64s backup driver can't prepare for synchronization"
 ER_BACKUP_CREATE_VP
@@ -6461,3 +6461,5 @@ ER_OPERATION_ABORTED
   eng "Operation aborted"
 ER_OPERATION_ABORTED_CORRUPTED
   eng "Operation aborted - data might be corrupted"
+ER_BACKUP_INTERRUPTED
+  eng "Operation has been interrupted."

=== modified file 'sql/si_objects.cc'
--- a/sql/si_objects.cc	2009-02-13 13:25:43 +0000
+++ b/sql/si_objects.cc	2009-02-19 14:37:37 +0000
@@ -755,7 +755,8 @@ bool Abstract_obj::create(THD *thd)
     @note this semantics will be changed in the future.
     That's why drop() is a public separate operation of Obj.
   */
-  drop(thd);
+  if (drop(thd))
+    DBUG_RETURN(TRUE);
 
   /*
     Now, proceed with creating the object.

=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc	2009-01-31 16:21:19 +0000
+++ b/sql/sql_prepare.cc	2009-02-19 14:37:37 +0000
@@ -2945,6 +2945,13 @@ Execute_sql_statement::execute_server_co
 
   error= mysql_execute_command(thd);
 
+  if (thd->killed_errno())
+  {
+    if (! thd->stmt_da->is_set())
+      thd->send_kill_message();
+  }
+
+  /* report error issued during command execution */
   if (error == 0 && thd->spcont == NULL)
     general_log_write(thd, COM_STMT_EXECUTE,
                       thd->query, thd->query_length);

Thread
bzr commit into mysql-6.0-backup branch (Rafal.Somla:2773) Bug#35079WL#4538Rafal Somla19 Feb