bk commit into 6.0 tree (rafal:1.2606) BUG#34721

[rsomla]

Below is the list of changes that have just been committed into a local
6.0 repository of rafal.  When rafal does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-15 12:29:25+02:00, rafal@quant.(none) +5 -0
  BUG#34721 (Backup driver can't refuse to provide driver):
  
  This patch modifies backup kernel so that it falls back to built in backup 
  engines in case a native backup engine can not be created. 
  
  If storage engine defines a get_backup_engine() factory function (inside the 
  handlerton), but that function fails to create a backup engine then the current 
  code interrupts backup/restore operation and reports error. This is not the 
  expected behaviour. This patch fixes the backup engine selection logic so that 
  in the above case it puts warning and continues using the built-in engines.

  mysql-test/lib/mtr_report.pl@stripped, 2008-04-15 12:29:19+02:00, rafal@quant.(none) +3 -0
    Added entry for backup_no_be test.

  mysql-test/r/backup_no_be.result@stripped, 2008-04-15 12:29:20+02:00, rafal@quant.(none) +41
-0
    Results for the test.

  mysql-test/r/backup_no_be.result@stripped, 2008-04-15 12:29:20+02:00, rafal@quant.(none) +0
-0

  mysql-test/t/backup_no_be.test@stripped, 2008-04-15 12:29:19+02:00, rafal@quant.(none) +92 -0
    New test which uses error injection to test the backup engine selection logic.

  mysql-test/t/backup_no_be.test@stripped, 2008-04-15 12:29:19+02:00, rafal@quant.(none) +0 -0

  sql/backup/backup_info.cc@stripped, 2008-04-15 12:29:19+02:00, rafal@quant.(none) +68 -9
    Changes in find_backup_engine() function:
    
    1. Debug code injected which temporarily sets MyISAM backup factory function to 
    a dummy one. This is used to test the backup engine selection logic.
    
    2. When creating a Native_snapshot object, replce DBUG_ASSERT with if condition.
    If it is not possible to use native engine, the built-in ones will be tried.

  sql/backup/be_native.h@stripped, 2008-04-15 12:29:19+02:00, rafal@quant.(none) +6 -1
    In Native_snapshot::init() report ER_BACKUP_CREATE_BE as warning, not as error.

diff -Nrup a/mysql-test/lib/mtr_report.pl b/mysql-test/lib/mtr_report.pl
--- a/mysql-test/lib/mtr_report.pl	2008-03-12 09:25:20 +01:00
+++ b/mysql-test/lib/mtr_report.pl	2008-04-15 12:29:19 +02:00
@@ -326,6 +326,9 @@ sub mtr_report_stats ($) {
 		  /Backup:/ or /Restore:/ or /Can't open the online backup progress tables/
 		) or
 		
+		# ignore warning generated when backup engine selection algorithm is tested
+		($testname eq 'main.backup_no_be') and /Backup: Cannot create backup engine/ or
+		
 		/Sort aborted/ or
 		/Time-out in NDB/ or
 		/Warning:\s+One can only use the --user.*root/ or
diff -Nrup a/mysql-test/r/backup_no_be.result b/mysql-test/r/backup_no_be.result
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/r/backup_no_be.result	2008-04-15 12:29:20 +02:00
@@ -0,0 +1,41 @@
+DROP DATABASE IF EXISTS db1;
+CREATE DATABASE db1;
+USE db1;
+CREATE TABLE t1 (a int) ENGINE=Myisam;
+INSERT INTO t1 VALUES (1),(2),(3);
+SET SESSION debug="d,";
+SELECT @@debug;
+@@debug
+d
+BACKUP DATABASE db1 TO 'db1.bak';
+backup_id
+#
+SHOW WARNINGS;
+Level	Code	Message
+SELECT max(backup_id) INTO @id FROM mysql.online_backup 
+WHERE command LIKE 'BACKUP DATABASE db1 %';
+SELECT engines FROM mysql.online_backup WHERE backup_id=@id;
+engines
+Default
+SET SESSION debug="d,backup_test_dummy_be_factory";
+SELECT @@debug;
+@@debug
+d,backup_test_dummy_be_factory
+BACKUP DATABASE db1 TO 'db1.bak';
+backup_id
+#
+SHOW WARNINGS;
+Level	Code	Message
+Warning	#	Cannot create backup engine for storage engine MyISAM
+SELECT max(backup_id) INTO @id FROM mysql.online_backup 
+WHERE command LIKE 'BACKUP DATABASE db1 %';
+SELECT engines FROM mysql.online_backup WHERE backup_id=@id;
+engines
+Default
+RESTORE FROM 'db1.bak';
+backup_id
+#
+SHOW TABLES IN db1;
+Tables_in_db1
+t1
+DROP DATABASE db1;
diff -Nrup a/mysql-test/t/backup_no_be.test b/mysql-test/t/backup_no_be.test
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/t/backup_no_be.test	2008-04-15 12:29:19 +02:00
@@ -0,0 +1,92 @@
+--source include/have_debug.inc
+
+#
+# This test script tests the behaviour reported in BUG#34721
+#
+# Test uses error injection activated with 'backup_test_dummy_be_factory' keyword.
+# The code is injected into has_native_backup() function in backup_info.cc.
+# When activated, it overwrites the native MyISAM backup factory with a dummy one 
+# which never creates an engine. This change is permanent and can not be reversed by 
+# disabling the error injection code. The only way to restore the original situation 
+# is by restarting the server. Therefore:
+#
+# 1. This test should have .opt file so that a separate server instance is run for 
+# it.
+#
+# 2. The order of operations in the test is relevant: we must first check the 
+# standard behaviour and only then activate the injected debug code.
+#
+
+--disable_warnings
+DROP DATABASE IF EXISTS db1;
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/db1.bak
+--enable_warnings
+
+CREATE DATABASE db1;
+USE db1;
+CREATE TABLE t1 (a int) ENGINE=Myisam;
+INSERT INTO t1 VALUES (1),(2),(3);
+
+# First check the normal behaviour when server is not modified.
+
+# make sure that injected code is disabled
+SET SESSION debug="d,";
+SELECT @@debug;
+
+# BACKUP should succeed without a warning.
+
+--replace_column 1 #
+BACKUP DATABASE db1 TO 'db1.bak';
+SHOW WARNINGS;
+
+# See what engines were used for BACKUP
+
+SELECT max(backup_id) INTO @id FROM mysql.online_backup 
+WHERE command LIKE 'BACKUP DATABASE db1 %';
+
+# Note: right now default engine will be used (as below) because there is no
+# native backup for MyISAM. But when it is added, the SELECT below should list native 
+# MYISAM engine.
+
+SELECT engines FROM mysql.online_backup WHERE backup_id=@id;
+
+# Now see what happens when injected code is activated and MyISAM uses the dummy
+# factory function.
+
+SET SESSION debug="d,backup_test_dummy_be_factory";
+SELECT @@debug;
+
+# The following BACKUP command should generate a warning but otherwise
+# should complete using the default backup engine.
+# We disable warnings temporarily and show them with SHOW WARNINGS instead. This is to 
+# avoid showing message codes (which change sometimes)
+
+--disable_warnings
+--remove_file $MYSQLTEST_VARDIR/master-data/db1.bak
+--replace_column 1 #
+BACKUP DATABASE db1 TO 'db1.bak';
+--enable_warings
+
+# Don't show warning message code
+--replace_column 2 #
+SHOW WARNINGS;
+
+# See what engines were used for BACKUP
+
+SELECT max(backup_id) INTO @id FROM mysql.online_backup 
+WHERE command LIKE 'BACKUP DATABASE db1 %';
+
+SELECT engines FROM mysql.online_backup WHERE backup_id=@id;
+
+# check that we can restore from the created image.
+
+--replace_column 1 #
+RESTORE FROM 'db1.bak';
+SHOW TABLES IN db1;
+
+
+# Clean up.
+
+DROP DATABASE db1;
+--remove_file $MYSQLTEST_VARDIR/master-data/db1.bak
diff -Nrup a/sql/backup/backup_info.cc b/sql/backup/backup_info.cc
--- a/sql/backup/backup_info.cc	2008-03-21 09:43:43 +01:00
+++ b/sql/backup/backup_info.cc	2008-04-15 12:29:19 +02:00
@@ -39,6 +39,24 @@ storage_engine_ref get_storage_engine(TH
   return se;
 }
 
+#ifndef DBUG_OFF
+
+/**
+  Dummy backup engine factory function.
+  
+  This factory function never creates any backup engines - it always returns
+  ERROR. It is used in @c has_native_backup() method for testing purposes.
+  
+  @return Always ERROR. 
+ */
+static
+Backup_result_t dummy_backup_engine_factory(handlerton*, Backup_engine* &eng)
+{
+  eng= NULL;
+  return backup::ERROR;
+}
+#endif
+
 /// Determine if a given storage engine has native backup support.
 static
 bool has_native_backup(storage_engine_ref se)
@@ -76,25 +94,66 @@ Backup_info::find_backup_engine(const ba
     DBUG_RETURN(NULL);
   }
   
+  /*
+    The code below is used to test the backup engine selection logic. 
+    
+    In case a storage engine defines the handlerton::get_backup_engine pointer 
+    but the factory function pointed by it refuses to create a backup engine, 
+    backup kernel should try to use built-in backup engines, the same as if 
+    handlerton::get_backup_engine was NULL.
+   
+    To test this behaviour, if "backup_test_dummy_be_factory" debug symbol is
+    defined (e.g., with --debug="d,backup_test_dummy_be_factory" option), we
+    set the get_backup_engine pointer for MyISAM handlerton to point at the 
+    dummy backup engine factory which never creates any engines.
+   */ 
+
+#ifndef DBUG_OFF
+  backup_factory *saved_factory; // to save hton->get_backup_engine
+
+  DBUG_EXECUTE_IF("backup_test_dummy_be_factory", 
+    {
+      handlerton *hton= se_hton(se);
+      saved_factory= hton->get_backup_engine;
+      if (hton == myisam_hton) 
+        hton->get_backup_engine= dummy_backup_engine_factory;
+    });
+#endif
+  
   snap= native_snapshots[se];
   
   if (!snap)
     if (has_native_backup(se))
     {
       Native_snapshot *nsnap= new Native_snapshot(m_ctx, se);
-      DBUG_ASSERT(nsnap);
-      snapshots.push_front(nsnap);
-      native_snapshots.insert(se, nsnap);
 
       /*
-        Question: Can native snapshot for a given storage engine not accept
-        a table using that engine? If yes, then what to do in that case - error 
-        or try other (default) snapshots?
-       */     
-      DBUG_ASSERT(nsnap->accept(tbl, se));
-      snap= nsnap;
+        Check if the snapshot object is valid - in particular has successfuly
+        created the native backup engine. If not, we will continue searching
+        for a backup engine, trying the built-in ones.
+      */
+      if (nsnap && nsnap->is_valid())
+      {
+        snapshots.push_front(nsnap);
+        native_snapshots.insert(se, nsnap);
+
+        if (nsnap->accept(tbl, se))        
+          snap= nsnap;
+      }
+      else
+        delete nsnap;
     }
   
+  /*
+    If "backup_test_dummy_be_factory" is used, the hton->get_backup_engine
+    pointer has been modified. Here we restore the originial value.
+   */ 
+  DBUG_EXECUTE_IF("backup_test_dummy_be_factory", 
+    {
+      handlerton *hton= se_hton(se);
+      hton->get_backup_engine= saved_factory;
+    });
+
   /* 
     If we couldn't locate native snapshot for that table - iterate over
     all existing snapshots and see if one of them can accept the table.
diff -Nrup a/sql/backup/be_native.h b/sql/backup/be_native.h
--- a/sql/backup/be_native.h	2008-03-04 17:06:22 +01:00
+++ b/sql/backup/be_native.h	2008-04-15 12:29:19 +02:00
@@ -97,7 +97,12 @@ int Native_snapshot::init(Logger &log, c
     if (m_be)
       m_be->free();
     m_be= NULL;
-    log.report_error(ER_BACKUP_CREATE_BE, m_name);
+    /*
+      We only report warning here because even if we failed to create native
+      backup engine it can be replaced by a built-in engine and the backup
+      operation might still succeed.  
+    */
+    log.report_error(log_level::WARNING, ER_BACKUP_CREATE_BE, m_name);
     return 1;
   }