From: Date: November 5 2008 1:40pm Subject: bzr commit into mysql-6.0-backup branch (Rafal.Somla:2721) Bug#40303 Bug#40304 List-Archive: http://lists.mysql.com/commits/57880 X-Bug: 40303,40304 Message-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///ext/mysql/bzr/backup/bug40303/ 2721 Rafal Somla 2008-11-05 BUG#40303 (Backup: Wrong error send to client if several errors reported) BUG#40304 (Backup: Errors reported during BACKUP show as warnings on the error stack) Before: Errors reported using backup::Logger class could be saved and in case several errors are reported, the last one was sent to client as an error reply from BACKUP/RESTORE command. After: Errors reported with backup::Logger are reported to the server using my_printf_error(..) function. This function pushes error on the error stack (unles told otherwise) and stores the first reported error so that it is send to the client. Additionally. - Function log_error(..) is moved to the Logger class. - Code is changed do consistently use Logger::report/log_error() for error reporting. - Semantics of Backup_restore_ctx::fatal_error() has been changed to discourage its use as an error reporting facility. Now it is an internal helper method which moves the context object into error state. It is used only by the class methods and it is made private. The idea is that context object should move into error state only as a result of executing one of its methods (not externally). - Improved error reporting in few places. modified: mysql-test/suite/backup/r/backup_errors.result mysql-test/suite/backup/r/backup_views.result mysql-test/suite/backup/t/backup_errors.test mysql-test/suite/backup/t/backup_myisam1.test mysql-test/suite/backup/t/backup_views.test sql/backup/backup_info.cc sql/backup/backup_info.h sql/backup/backup_kernel.h sql/backup/backup_test.cc sql/backup/data_backup.cc sql/backup/kernel.cc sql/backup/logger.cc sql/backup/logger.h sql/backup/restore_info.h sql/backup/stream.cc sql/backup/stream.h sql/share/errmsg.txt per-file messages: mysql-test/suite/backup/t/backup_errors.test - Add SHOW WARNINGS to see what is pushed on the error stack in case of errors. - Use @@backupdir and @@datadir for the corresponding paths. mysql-test/suite/backup/t/backup_myisam1.test - Update the test, taking into account that after this patch failed BACKUP does not leave the unfinished backup image file around. - Use @@backupdir for the corresponding path. mysql-test/suite/backup/t/backup_views.test - Update the test taking into account that now RESTORE returns the first error reported. sql/backup/backup_info.cc - Use m_log.report_error(...) for error reporting. - Use the m_thd member instead of refering to a context class instance. sql/backup/backup_info.h Instead of storing a reference to a complete Backup_restore_ctx instance, store only what is needed: a Logger instance and a THD handle. sql/backup/backup_kernel.h - Move log_error(..) method to the Logger class. - Make fatal_error(...) private and change its semantics. Now it only moves context object to an error state, but it does not report any errors - Logger methods should be used for reporting them. sql/backup/data_backup.cc - Use Logger instance for reporting errors. - Report few errors not logged previously (e.g. when (un)block_commits(..) fails). - Remove cancel_backup() calls as this is done in Backup_restore_ctx::close() called from destructor. - Inside Backup_pump class: log errors only if logger is set. - Fix the shutdown logic in restore_table_data(..). Drivers need to be correctly de-initialized by a call to ->end() during normal operation or ->cacnel() in case of interruption. sql/backup/kernel.cc - Use explicit calls to report/log_error(..) for error logging - now fatal_error(..) does not log anything. - Simplified send_error(..) implementation - now it does not have to look for the last error reported. The first reported error is automatically stored by the server and send to the client. - Move error reporting context preparations to Logger::init(..). - To clarify the code, calls to s->next_chunk() have been moved to wrapper functions read_header(..), read_catalog(..) and read_meta_data(..) where they really belong. sql/backup/logger.cc - Remove code saving reported errors. - Use my_printf_error(..) instead of push_warning_printf(..) for reporting errors to the server core. This has the side effect of storing the first reported error so that it is sent to the client. sql/backup/logger.h - Add log_error(..) method moved here from Backup_restore_ctx. - Remove methods for saving reported errors - not needed any more because the first reported error is remembered by the server core. - Add error_reported() method. - Add error context cleanup (on server level) to init() method, instead of doing it inside Backup_restore_ctx class. sql/backup/restore_info.h Only store what is needed: reference to Logger and THD handle. sql/backup/stream.cc Remove Input_stream::next_chunk() method. sql/backup/stream.h - Move to next chunk after reading header, catalogue and metadata sections. Do it inside wrapper read_*(..) functions using direct calls to bstream library. - Remove Input_stream::next_chunk() method. sql/share/errmsg.txt Add error message for reporting problems during validity point creation. === modified file 'mysql-test/suite/backup/r/backup_errors.result' --- a/mysql-test/suite/backup/r/backup_errors.result 2008-10-07 17:15:44 +0000 +++ b/mysql-test/suite/backup/r/backup_errors.result 2008-11-05 12:40:24 +0000 @@ -7,19 +7,36 @@ CREATE DATABASE bdb; CREATE TABLE bdb.t1(a int) ENGINE=MEMORY; BACKUP DATABASE adb TO ''; ERROR HY000: Malformed file path '' +SHOW WARNINGS; +Level Code Message +Error # Malformed file path '' BACKUP DATABASE adb TO "bdb/t1.frm"; ERROR HY000: Can't write to backup location 'bdb/t1.frm' (file already exists?) +SHOW WARNINGS; +Level Code Message +Error # Can't write to backup location 'bdb/t1.frm' (file already exists?) BACKUP DATABASE adb TO "test.bak"; backup_id # +SHOW WARNINGS; +Level Code Message BACKUP DATABASE adb TO "test.bak"; ERROR HY000: Can't write to backup location 'test.bak' (file already exists?) +SHOW WARNINGS; +Level Code Message +Error # Can't write to backup location 'test.bak' (file already exists?) DROP DATABASE IF EXISTS foo; DROP DATABASE IF EXISTS bar; BACKUP DATABASE foo TO 'test.bak'; ERROR 42000: Unknown database 'foo' +SHOW WARNINGS; +Level Code Message +Error 1049 Unknown database 'foo' BACKUP DATABASE test,foo,bdb,bar TO 'test.bak'; ERROR 42000: Unknown database 'foo,bar' +SHOW WARNINGS; +Level Code Message +Error # Unknown database 'foo,bar' use adb; create table t1 (a int); create procedure p1() backup database test to 'test.bak'; @@ -47,21 +64,39 @@ DROP DATABASE bdb; Backup of mysql, information_schema scenario 1 BACKUP DATABASE mysql TO 't.bak'; ERROR HY000: Database 'mysql' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'mysql' cannot be included in a backup Backup of mysql, information_schema scenario 2 BACKUP DATABASE information_schema TO 't.bak'; ERROR HY000: Database 'information_schema' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'information_schema' cannot be included in a backup Backup of mysql, information_schema scenario 3 BACKUP DATABASE mysql, information_schema TO 't.bak'; ERROR HY000: Database 'mysql' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'mysql' cannot be included in a backup Backup of mysql, information_schema scenario 4 BACKUP DATABASE mysql, test TO 't.bak'; ERROR HY000: Database 'mysql' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'mysql' cannot be included in a backup Backup of mysql, information_schema scenario 5 BACKUP DATABASE information_schema, test TO 't.bak'; ERROR HY000: Database 'information_schema' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'information_schema' cannot be included in a backup Backup of mysql, information_schema scenario 6 BACKUP DATABASE mysql, information_schema, test TO 't.bak'; ERROR HY000: Database 'mysql' cannot be included in a backup +SHOW WARNINGS; +Level Code Message +Error # Database 'mysql' cannot be included in a backup Making copies of progress tables. CREATE TABLE IF NOT EXISTS test.ob_copy LIKE mysql.backup_history; CREATE TABLE IF NOT EXISTS test.obp_copy LIKE mysql.backup_progress; @@ -76,7 +111,7 @@ DROP TABLE mysql.backup_history; Backup the database; BACKUP DATABASE test_ob_error TO 'ob_err.bak'; ERROR HY000: Can't open the backup logs as tables. Check 'mysql.backup_history' and 'mysql.backup_progress' or run mysql_upgrade to repair. -SHOW ERRORS; +SHOW WARNINGS; Level Code Message Error # Table 'mysql.backup_history' doesn't exist Error # Cannot create backup/restore execution context @@ -87,7 +122,7 @@ DROP TABLE mysql.backup_progress; Backup the database; BACKUP DATABASE test_ob_error TO 'ob_err.bak'; ERROR HY000: Can't open the backup logs as tables. Check 'mysql.backup_history' and 'mysql.backup_progress' or run mysql_upgrade to repair. -SHOW ERRORS; +SHOW WARNINGS; Level Code Message Error # Table 'mysql.backup_progress' doesn't exist Error # Cannot create backup/restore execution context === modified file 'mysql-test/suite/backup/r/backup_views.result' --- a/mysql-test/suite/backup/r/backup_views.result 2008-10-07 17:15:44 +0000 +++ b/mysql-test/suite/backup/r/backup_views.result 2008-11-05 12:40:24 +0000 @@ -277,10 +277,10 @@ DROP DATABASE bup_db2; Restore database. restore database with view dependency to other, non-existing db RESTORE FROM 'bup_objectview1.bak'; -ERROR HY000: Could not restore view `bup_db1`.`v5`. Please check the view definition for possible missing dependencies. +ERROR 42S02: Table 'bup_db2.t2' doesn't exist DROP DATABASE bup_db1; RESTORE FROM 'bup_objectview2.bak'; -ERROR HY000: Could not restore view `bup_db2`.`student_details`. Please check the view definition for possible missing dependencies. +ERROR 42S02: Table 'bup_db1.t3' doesn't exist DROP DATABASE bup_db2; RESTORE FROM 'bup_objectview.bak'; backup_id === modified file 'mysql-test/suite/backup/t/backup_errors.test' --- a/mysql-test/suite/backup/t/backup_errors.test 2008-10-07 17:15:44 +0000 +++ b/mysql-test/suite/backup/t/backup_errors.test 2008-11-05 12:40:24 +0000 @@ -7,11 +7,14 @@ # TODO: When we know how to do that, check that the backup progress table # contains appropriate rows when errors have been detected. +let $bdir= `SELECT @@backupdir`; +let $ddir= `SELECT @@datadir`; + --disable_warnings DROP DATABASE IF EXISTS adb; DROP DATABASE IF EXISTS bdb; --error 0,1 ---remove_file $MYSQLTEST_VARDIR/master-data/test.bak +--remove_file $bdir/test.bak --enable_warnings # non-existent backup archive @@ -25,19 +28,27 @@ CREATE TABLE bdb.t1(a int) ENGINE=MEMORY # invalid location --error ER_BAD_PATH BACKUP DATABASE adb TO ''; +--replace_column 2 # +SHOW WARNINGS; # don't overwrite existing files --error ER_BACKUP_WRITE_LOC BACKUP DATABASE adb TO "bdb/t1.frm"; +--replace_column 2 # +SHOW WARNINGS; --replace_column 1 # BACKUP DATABASE adb TO "test.bak"; +--replace_column 2 # +SHOW WARNINGS; # don't overwrite existing backup image --error ER_BACKUP_WRITE_LOC BACKUP DATABASE adb TO "test.bak"; +--replace_column 2 # +SHOW WARNINGS; ---remove_file $MYSQLTEST_VARDIR/master-data/test.bak +--remove_file $bdir/test.bak # non-existent database --disable_warnings @@ -47,8 +58,11 @@ DROP DATABASE IF EXISTS bar; -- error ER_BAD_DB_ERROR BACKUP DATABASE foo TO 'test.bak'; +SHOW WARNINGS; -- error ER_BAD_DB_ERROR BACKUP DATABASE test,foo,bdb,bar TO 'test.bak'; +--replace_column 2 # +SHOW WARNINGS; # Test that BACKUP/RESTORE statements are disable inside stored routines, # triggers and events. @@ -89,7 +103,7 @@ DROP DATABASE bdb; # Note: the file should be removed - if it is not, the following command # will fail and we will be warned that something is wrong with the test --error 1 ---remove_file $MYSQLTEST_VARDIR/master-data/test.bak +--remove_file $bdir/test.bak # Check error conditions for including mysql and information_schema in # the list of databases. @@ -113,49 +127,61 @@ DROP DATABASE bdb; --echo Backup of mysql, information_schema scenario 1 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE mysql TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # Scenario 2 --echo Backup of mysql, information_schema scenario 2 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE information_schema TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # Scenario 3 --echo Backup of mysql, information_schema scenario 3 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE mysql, information_schema TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # Scenario 4 --echo Backup of mysql, information_schema scenario 4 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE mysql, test TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # Scenario 5 --echo Backup of mysql, information_schema scenario 5 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE information_schema, test TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # Scenario 6 --echo Backup of mysql, information_schema scenario 6 --error ER_BACKUP_CANNOT_INCLUDE_DB BACKUP DATABASE mysql, information_schema, test TO 't.bak'; +--replace_column 2 # +SHOW WARNINGS; --error 0, 1 ---remove_file $MYSQLTEST_VARDIR/master-data/t.bak +--remove_file $bdir/t.bak # # BUG#33352 - Test for presence of online backup progress tables; # @@ -174,7 +200,7 @@ INSERT INTO test_ob_error.t1 VALUES (1), --echo Backup the database; --replace_column 1 # BACKUP DATABASE test_ob_error TO 'ob_err.bak'; ---remove_file $MYSQLTEST_VARDIR/master-data/ob_err.bak +--remove_file $bdir/ob_err.bak # Drop one of the tables and try a backup. DROP TABLE mysql.backup_history; @@ -184,9 +210,9 @@ DROP TABLE mysql.backup_history; --error ER_BACKUP_PROGRESS_TABLES BACKUP DATABASE test_ob_error TO 'ob_err.bak'; --error 0,1 ---remove_file $MYSQLTEST_VARDIR/master-data/ob_err.bak +--remove_file $bdir/ob_err.bak --replace_column 2 # -SHOW ERRORS; +SHOW WARNINGS; # Restore the table --echo Restoring the table @@ -201,9 +227,9 @@ DROP TABLE mysql.backup_progress; --error ER_BACKUP_PROGRESS_TABLES BACKUP DATABASE test_ob_error TO 'ob_err.bak'; --error 0,1 ---remove_file $MYSQLTEST_VARDIR/master-data/ob_err.bak +--remove_file $bdir/ob_err.bak --replace_column 2 # -SHOW ERRORS; +SHOW WARNINGS; # Restore the table --echo Restoring the table @@ -244,8 +270,8 @@ SET DEBUG_SYNC='now WAIT_FOR running'; --echo delete database files so that check_db_dir_exists will fail in --echo si_objects.cc @ DatabaseObj::do_serialize ---remove_file $MYSQLTEST_VARDIR/master-data/db1/db.opt ---rmdir $MYSQLTEST_VARDIR/master-data/db1 +--remove_file $ddir/db1/db.opt +--rmdir $ddir/db1 SET DEBUG_SYNC='now SIGNAL db_will_fail'; === modified file 'mysql-test/suite/backup/t/backup_myisam1.test' --- a/mysql-test/suite/backup/t/backup_myisam1.test 2008-10-07 17:15:44 +0000 +++ b/mysql-test/suite/backup/t/backup_myisam1.test 2008-11-05 12:40:24 +0000 @@ -3,6 +3,8 @@ --source include/not_embedded.inc +let $bdir=`SELECT @@backupdir`; + # # Cleanup from former test cases # @@ -10,7 +12,7 @@ drop database if exists mysqltest; --enable_warnings --error 0,1 ---remove_file $MYSQLTEST_VARDIR/master-data/test.ba +--remove_file $bdir/test.ba create database mysqltest; use mysqltest; @@ -24,5 +26,7 @@ BACKUP DATABASE mysqltest TO 'test.ba'; # Cleanup from this test case # DROP DATABASE mysqltest; ---remove_file $MYSQLTEST_VARDIR/master-data/test.ba +# Note: The backup file should not exist as BACKUP commad failed. +--error 1 +--remove_file $bdir/test.ba === modified file 'mysql-test/suite/backup/t/backup_views.test' --- a/mysql-test/suite/backup/t/backup_views.test 2008-10-07 17:15:44 +0000 +++ b/mysql-test/suite/backup/t/backup_views.test 2008-11-05 12:40:24 +0000 @@ -205,14 +205,14 @@ DROP DATABASE bup_db2; --echo restore database with view dependency to other, non-existing db ---error ER_BACKUP_CANT_RESTORE_VIEW +--error ER_NO_SUCH_TABLE RESTORE FROM 'bup_objectview1.bak'; # An incomplete bup_db1 was created by the failing restore operation. # Remove it before trying restore of bup_db2. DROP DATABASE bup_db1; ---error ER_BACKUP_CANT_RESTORE_VIEW +--error ER_NO_SUCH_TABLE RESTORE FROM 'bup_objectview2.bak'; # An incomplete bup_db2 was created by the failing restore operation. === modified file 'sql/backup/backup_info.cc' --- a/sql/backup/backup_info.cc 2008-10-27 13:06:21 +0000 +++ b/sql/backup/backup_info.cc 2008-11-05 12:40:24 +0000 @@ -87,11 +87,11 @@ Backup_info::find_backup_engine(const ba // See if table has native backup engine - storage_engine_ref se= get_storage_engine(m_ctx.m_thd, tbl); + storage_engine_ref se= get_storage_engine(m_thd, tbl); if (!se) { - m_ctx.fatal_error(ER_NO_STORAGE_ENGINE, tbl.describe(buf)); + m_log.report_error(ER_NO_STORAGE_ENGINE, tbl.describe(buf)); DBUG_RETURN(NULL); } @@ -126,7 +126,7 @@ Backup_info::find_backup_engine(const ba if (!snap) if (has_native_backup(se)) { - Native_snapshot *nsnap= new Native_snapshot(m_ctx, se); + Native_snapshot *nsnap= new Native_snapshot(m_log, se); /* Check if the snapshot object is valid - in particular has successfully @@ -175,7 +175,7 @@ Backup_info::find_backup_engine(const ba } if (!snap) - m_ctx.fatal_error(ER_BACKUP_NO_BACKUP_DRIVER,tbl.describe(buf)); + m_log.report_error(ER_BACKUP_NO_BACKUP_DRIVER,tbl.describe(buf)); DBUG_RETURN(snap); } @@ -301,7 +301,7 @@ Backup_info::Dep_node::get_key(const uch @c find_backup_engine(). */ Backup_info::Backup_info(Backup_restore_ctx &ctx) - :m_ctx(ctx), m_state(Backup_info::ERROR), native_snapshots(8), + :m_log(ctx), m_thd(ctx.thd()), m_state(Backup_info::ERROR), native_snapshots(8), m_dep_list(NULL), m_dep_end(NULL), m_srout_end(NULL), m_view_end(NULL), m_trigger_end(NULL), m_event_end(NULL) { @@ -318,7 +318,7 @@ Backup_info::Backup_info(Backup_restore_ Dep_node::get_key, Dep_node::free, MYF(0))) { // Allocation failed. Error has been reported, but not logged to backup logs - m_ctx.log_error(ER_OUT_OF_RESOURCES); + m_log.log_error(ER_OUT_OF_RESOURCES); return; } @@ -328,10 +328,10 @@ Backup_info::Backup_info(Backup_restore_ element on that list, as a "catch all" entry. */ - snap= new Nodata_snapshot(m_ctx); // logs errors + snap= new Nodata_snapshot(m_log); // logs errors if (!snap) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return; } @@ -341,14 +341,14 @@ Backup_info::Backup_info(Backup_restore_ if (snapshots.push_back(snap)) { // Allocation failed. Error has been reported, but not logged to backup logs - m_ctx.log_error(ER_OUT_OF_RESOURCES); + m_log.log_error(ER_OUT_OF_RESOURCES); return; } - snap= new CS_snapshot(m_ctx); // logs errors + snap= new CS_snapshot(m_log); // logs errors if (!snap) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return; } @@ -358,14 +358,14 @@ Backup_info::Backup_info(Backup_restore_ if (snapshots.push_back(snap)) { // Allocation failed. Error has been reported, but not logged to backup logs - m_ctx.log_error(ER_OUT_OF_RESOURCES); + m_log.log_error(ER_OUT_OF_RESOURCES); return; // Error has been logged } - snap= new Default_snapshot(m_ctx); // logs errors + snap= new Default_snapshot(m_log); // logs errors if (!snap) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return; } @@ -375,7 +375,7 @@ Backup_info::Backup_info(Backup_restore_ if (snapshots.push_back(snap)) { // Allocation failed. Error has been reported, but not logged to backup logs - m_ctx.log_error(ER_OUT_OF_RESOURCES); + m_log.log_error(ER_OUT_OF_RESOURCES); return; // Error has been logged } @@ -417,7 +417,7 @@ int Backup_info::close() // report backup drivers used in the image for (ushort n=0; n < snap_count(); ++n) - m_ctx.report_driver(m_snap[n]->name()); + m_log.report_driver(m_snap[n]->name()); m_state= CLOSED; return 0; @@ -465,7 +465,7 @@ backup::Image_info::Ts* Backup_info::add if (!ts) { - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_TS, name); + m_log.report_error(ER_BACKUP_CATALOG_ADD_TS, name); return NULL; } @@ -479,7 +479,7 @@ backup::Image_info::Ts* Backup_info::add if (!n1) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return NULL; } @@ -513,7 +513,7 @@ backup::Image_info::Db* Backup_info::add if (!db) { - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_DB, name->ptr()); + m_log.report_error(ER_BACKUP_CATALOG_ADD_DB, name->ptr()); return NULL; } @@ -546,7 +546,7 @@ int Backup_info::add_dbs(List< ::LEX_STR if (is_internal_db_name(&db_name)) { - m_ctx.fatal_error(ER_BACKUP_CANNOT_INCLUDE_DB, db_name.c_ptr()); + m_log.report_error(ER_BACKUP_CANNOT_INCLUDE_DB, db_name.c_ptr()); goto error; } @@ -585,7 +585,7 @@ int Backup_info::add_dbs(List< ::LEX_STR if (!unknown_dbs.is_empty()) { - m_ctx.fatal_error(ER_BAD_DB_ERROR, unknown_dbs.c_ptr()); + m_log.report_error(ER_BAD_DB_ERROR, unknown_dbs.c_ptr()); goto error; } @@ -610,11 +610,11 @@ int Backup_info::add_all_dbs() using namespace obs; int res= 0; - Obj_iterator *dbit= get_databases(m_ctx.m_thd); + Obj_iterator *dbit= get_databases(m_thd); if (!dbit) { - m_ctx.fatal_error(ER_BACKUP_LIST_DBS); + m_log.report_error(ER_BACKUP_LIST_DBS); return ERROR; } @@ -697,11 +697,11 @@ int Backup_info::add_db_items(Db &db) // Add tables. - Obj_iterator *it= get_db_tables(m_ctx.m_thd, &db.name()); + Obj_iterator *it= get_db_tables(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_TABLES, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_TABLES, db.name().ptr()); return ERROR; } @@ -727,7 +727,7 @@ int Backup_info::add_db_items(Db &db) // If this table uses a tablespace, add this tablespace to the catalogue. - obj= get_tablespace_for_table(m_ctx.m_thd, &db.name(), &tbl->name()); + obj= get_tablespace_for_table(m_thd, &db.name(), &tbl->name()); if (obj) { @@ -744,11 +744,11 @@ int Backup_info::add_db_items(Db &db) // Add other objects. delete it; - it= get_db_stored_procedures(m_ctx.m_thd, &db.name()); + it= get_db_stored_procedures(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_SROUT, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_SROUT, db.name().ptr()); goto error; } @@ -756,11 +756,11 @@ int Backup_info::add_db_items(Db &db) goto error; delete it; - it= get_db_stored_functions(m_ctx.m_thd, &db.name()); + it= get_db_stored_functions(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_SROUT, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_SROUT, db.name().ptr()); goto error; } @@ -768,11 +768,11 @@ int Backup_info::add_db_items(Db &db) goto error; delete it; - it= get_db_views(m_ctx.m_thd, &db.name()); + it= get_db_views(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_VIEWS, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_VIEWS, db.name().ptr()); goto error; } @@ -780,11 +780,11 @@ int Backup_info::add_db_items(Db &db) goto error; delete it; - it= get_db_events(m_ctx.m_thd, &db.name()); + it= get_db_events(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_EVENTS, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_EVENTS, db.name().ptr()); goto error; } @@ -792,11 +792,11 @@ int Backup_info::add_db_items(Db &db) goto error; delete it; - it= get_db_triggers(m_ctx.m_thd, &db.name()); + it= get_db_triggers(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_TRIGGERS, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_TRIGGERS, db.name().ptr()); goto error; } @@ -804,11 +804,11 @@ int Backup_info::add_db_items(Db &db) goto error; delete it; - it= get_all_db_grants(m_ctx.m_thd, &db.name()); + it= get_all_db_grants(m_thd, &db.name()); if (!it) { - m_ctx.fatal_error(ER_BACKUP_LIST_DB_PRIV, db.name().ptr()); + m_log.report_error(ER_BACKUP_LIST_DB_PRIV, db.name().ptr()); goto error; } @@ -885,8 +885,8 @@ backup::Image_info::Table* Backup_info:: if (!tbl) { - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_TABLE, - dbi.name().ptr(), t.name().ptr()); + m_log.report_error(ER_BACKUP_CATALOG_ADD_TABLE, + dbi.name().ptr(), t.name().ptr()); return NULL; } @@ -939,7 +939,7 @@ int Backup_info::add_view_deps(obs::Obj // Get an iterator to iterate over base views of the given one. - obs::Obj_iterator *it= obs::get_view_base_views(m_ctx.m_thd, db_name, name); + obs::Obj_iterator *it= obs::get_view_base_views(m_thd, db_name, name); if (!it) return ERROR; @@ -1082,7 +1082,7 @@ Backup_info::add_db_object(Db &db, const if (res == get_dep_node_res::ERROR) { - m_ctx.fatal_error(error, db.name().ptr(), name->ptr()); + m_log.report_error(error, db.name().ptr(), name->ptr()); return NULL; } @@ -1097,7 +1097,7 @@ Backup_info::add_db_object(Db &db, const if (type == BSTREAM_IT_VIEW) if (add_view_deps(*obj)) { - m_ctx.fatal_error(error, db.name().ptr(), name->ptr()); + m_log.report_error(error, db.name().ptr(), name->ptr()); return NULL; } @@ -1118,7 +1118,7 @@ Backup_info::add_db_object(Db &db, const if (!o) { - m_ctx.fatal_error(error, db.name().ptr(), name->ptr()); + m_log.report_error(error, db.name().ptr(), name->ptr()); return NULL; } @@ -1338,7 +1338,7 @@ int Backup_info::Global_iterator::init() if (!m_it) { const Backup_info* info= static_cast(&m_info); - return info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + return info->m_log.report_error(ER_OUT_OF_RESOURCES); } next(); // Never errors @@ -1384,7 +1384,7 @@ Backup_info::Global_iterator::next() if (!m_it) { const Backup_info* info= static_cast(&m_info); - info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + info->m_log.report_error(ER_OUT_OF_RESOURCES); mode= DONE; return FALSE; } @@ -1467,7 +1467,7 @@ backup::Image_info::Iterator* Backup_inf Global_iterator* it = new Global_iterator(*this); if (it == NULL) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return NULL; } if (it->init()) // Error has been logged @@ -1484,7 +1484,7 @@ backup::Image_info::Iterator* Backup_inf Perdb_iterator* it = new Perdb_iterator(*this); if (it == NULL) { - m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + m_log.report_error(ER_OUT_OF_RESOURCES); return NULL; } if (it->init()) // Error has been logged === modified file 'sql/backup/backup_info.h' --- a/sql/backup/backup_info.h 2008-10-27 13:06:21 +0000 +++ b/sql/backup/backup_info.h 2008-11-05 12:40:24 +0000 @@ -31,7 +31,7 @@ class Backup_info: public backup::Image_ { public: - Backup_restore_ctx &m_ctx; + backup::Logger &m_log; Backup_info(Backup_restore_ctx&); ~Backup_info(); @@ -48,6 +48,8 @@ class Backup_info: public backup::Image_ private: + THD *m_thd; + class Global_iterator; ///< Iterates over global items (for which meta-data is stored). class Perdb_iterator; ///< Iterates over all per-database objects (except tables). === modified file 'sql/backup/backup_kernel.h' --- a/sql/backup/backup_kernel.h 2008-10-27 13:06:21 +0000 +++ b/sql/backup/backup_kernel.h 2008-11-05 12:40:24 +0000 @@ -75,8 +75,6 @@ class Backup_restore_ctx: public backup: int do_backup(); int do_restore(); - int fatal_error(int, ...); - int log_error(int, ...); int close(); @@ -84,6 +82,8 @@ class Backup_restore_ctx: public backup: private: + int fatal_error(int); + /** @c current_op points to the @c Backup_restore_ctx for the ongoing backup/restore operation. If pointer is null, no operation is currently running. */ @@ -139,8 +139,6 @@ class Backup_restore_ctx: public backup: int report_stream_open_failure(int open_error, const LEX_STRING *location); - friend class Backup_info; - friend class Restore_info; friend int backup_init(); friend void backup_shutdown(); friend bstream_byte* bstream_alloc(unsigned long int); @@ -169,7 +167,7 @@ void Backup_restore_ctx::disable_fkey_co } /** - Report error and move context object into error state. + Move context object into error state. After this method is called the context object is in error state and cannot be normally used. It still can be examined for saved error messages. @@ -182,19 +180,14 @@ void Backup_restore_ctx::disable_fkey_co a fatal error was reported before. */ inline -int Backup_restore_ctx::fatal_error(int error_code, ...) +int Backup_restore_ctx::fatal_error(int error_code) { + m_remove_loc= TRUE; + if (m_error) return m_error; - va_list args; - m_error= error_code; - m_remove_loc= TRUE; - - va_start(args,error_code); - v_report_error(backup::log_level::ERROR, error_code, args); - va_end(args); return error_code; } === modified file 'sql/backup/backup_test.cc' --- a/sql/backup/backup_test.cc 2008-10-17 11:28:25 +0000 +++ b/sql/backup/backup_test.cc 2008-11-05 12:40:24 +0000 @@ -10,6 +10,8 @@ #include "si_objects.h" #include "backup_aux.h" +extern "C" void my_message_sql(uint error, const char *str, myf MyFlags); + using namespace obs; /** @@ -25,6 +27,32 @@ int execute_backup_test_command(THD *thd DBUG_ENTER("execute_backup_test_command"); DBUG_ASSERT(thd); + for (uint cnt=1; cnt <= 5 && !thd->killed; cnt++) + my_sleep(1*1000000); + + if (FALSE) +// if (thd->killed) + { + //push_warning(thd,MYSQL_ERROR::WARN_LEVEL_ERROR,ER_BACKUP_BACKUP,"First error"); + //push_warning(thd,MYSQL_ERROR::WARN_LEVEL_ERROR,ER_BACKUP_RESTORE,"Second error"); + thd->no_warnings_for_error= FALSE; + //my_printf_error(ER_QUERY_INTERRUPTED,"Final error message", MYF(0)); + //my_message_sql(ER_BACKUP_RESTORE, "First error", MYF(0)); + //my_message_sql(ER_BACKUP_BACKUP, "Second error", MYF(0)); + //thd->main_da.can_overwrite_status= TRUE; + //thd->main_da.set_error_status(thd, ER_QUERY_INTERRUPTED, "Final message"); + + DBUG_RETURN(ER_QUERY_INTERRUPTED); + } + + thd->main_da.reset_diagnostics_area(); + my_ok(thd); + DBUG_RETURN(0); + + my_error(ER_QUERY_INTERRUPTED,MYF(0)); + + DBUG_RETURN(ER_QUERY_INTERRUPTED); + Protocol *protocol= thd->protocol; // client comms List field_list; // list of fields to send String op_str; // operations string === modified file 'sql/backup/data_backup.cc' --- a/sql/backup/data_backup.cc 2008-10-14 12:08:56 +0000 +++ b/sql/backup/data_backup.cc 2008-11-05 12:40:24 +0000 @@ -248,7 +248,7 @@ class Scheduler private: LIST *m_pumps, *m_last; - Logger *m_log; ///< used to report errors if not NULL + Logger &m_log; ///< for reporting errors uint m_count; ///< current number of pumps size_t m_total; ///< accumulated position of all drivers size_t m_init_left; ///< how much of init data is left (estimate) @@ -256,7 +256,7 @@ class Scheduler Output_stream &m_str; ///< stream to which we write bool cancelled; ///< true if backup process was cancelled - Scheduler(Output_stream &s, Logger *log) + Scheduler(Output_stream &s, Logger &log) :init_count(0), prepare_count(0), finish_count(0), m_pumps(NULL), m_last(NULL), m_log(log), m_count(0), m_total(0), m_init_left(0), m_known_count(0), @@ -427,7 +427,8 @@ int write_table_data(THD* thd, Backup_in if (info.snap_count() == 0 || info.table_count() == 0) // nothing to backup DBUG_RETURN(0); - Scheduler sch(s, &info.m_ctx); // scheduler instance + Logger &log= info.m_log; + Scheduler sch(s, log); // scheduler instance List inactive; // list of images not yet being created // keeps maximal init size for images in inactive list @@ -448,9 +449,15 @@ int write_table_data(THD* thd, Backup_in Scheduler::Pump *p= new Scheduler::Pump(*i, s); - if (!p || !p->is_valid()) + if (!p) { - info.m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + log.report_error(ER_OUT_OF_RESOURCES); + goto error; + } + if (!p->is_valid()) + { + log.report_error(ER_BACKUP_CREATE_BACKUP_DRIVER,p->m_name); + delete p; goto error; } @@ -458,7 +465,7 @@ int write_table_data(THD* thd, Backup_in if (init_size == Driver::UNKNOWN_SIZE) { - if (sch.add(p)) + if (sch.add(p)) // logs errors goto error; } else @@ -471,7 +478,7 @@ int write_table_data(THD* thd, Backup_in /* Allocation failed. Error has been reported, but not logged to backup logs. */ - info.m_ctx.log_error(ER_OUT_OF_RESOURCES); + log.log_error(ER_OUT_OF_RESOURCES); goto error; } } @@ -529,7 +536,7 @@ int write_table_data(THD* thd, Backup_in // poll drivers - if (sch.step()) + if (sch.step()) // logs errors goto error; } @@ -552,7 +559,7 @@ int write_table_data(THD* thd, Backup_in DBUG_PRINT("backup_data",("-- PREPARE PHASE --")); DEBUG_SYNC(thd, "before_backup_data_prepare"); - if (sch.prepare()) + if (sch.prepare()) // logs errors goto error; while (sch.prepare_count > 0) @@ -565,7 +572,7 @@ int write_table_data(THD* thd, Backup_in LOG_INFO binlog_pos; - info.m_ctx.report_state(BUP_VALIDITY_POINT); + log.report_state(BUP_VALIDITY_POINT); /* This breakpoint is used to assist in testing state changes for the backup progress. It is not to be used to indicate actual @@ -583,10 +590,13 @@ int write_table_data(THD* thd, Backup_in int error= 0; error= block_commits(thd, NULL); if (error) + { + log.report_error(ER_BACKUP_VP_FAILED); goto error; + } DEBUG_SYNC(thd, "before_backup_data_lock"); - if (sch.lock()) + if (sch.lock()) // logs errors goto error; /* @@ -595,7 +605,7 @@ int write_table_data(THD* thd, Backup_in if (mysql_bin_log.is_open()) if (mysql_bin_log.get_current_log(&binlog_pos)) { - info.m_ctx.fatal_error(ER_BACKUP_BINLOG); + log.report_error(ER_BACKUP_BINLOG); goto error; } @@ -605,7 +615,7 @@ int write_table_data(THD* thd, Backup_in vp_time= my_time(0); DEBUG_SYNC(thd, "before_backup_data_unlock"); - if (sch.unlock()) + if (sch.unlock()) // logs errors goto error; /* @@ -614,20 +624,23 @@ int write_table_data(THD* thd, Backup_in DEBUG_SYNC(thd, "before_backup_unblock_commit"); error= unblock_commits(thd); if (error) + { + log.report_error(ER_BACKUP_VP_FAILED); goto error; + } // Report and save information about VP info.save_vp_time(vp_time); - info.m_ctx.report_vp_time(vp_time, TRUE); // TRUE = also write to progress log + log.report_vp_time(vp_time, TRUE); // TRUE = also write to progress log if (mysql_bin_log.is_open()) { info.save_binlog_pos(binlog_pos); - info.m_ctx.report_binlog_pos(info.binlog_pos); + log.report_binlog_pos(info.binlog_pos); } - info.m_ctx.report_state(BUP_RUNNING); + log.report_state(BUP_RUNNING); DEBUG_SYNC(thd, "after_backup_binlog"); /**** VP creation (end) ********************************************/ @@ -788,9 +801,6 @@ int Scheduler::step() case backup_state::ERROR: remove_pump(p); // Note: never errors. - if (res) - cancel_backup(); // we hit an error - bail out - // Note: cancel_backup() never errors. break; default: break; @@ -815,11 +825,14 @@ int Scheduler::add(Pump *p) if (!p) // no pump to add return 0; - p->set_logger(m_log); + p->set_logger(&m_log); p->start_pos= avg; - if (p->begin()) - goto error; + if (p->begin()) // logs errors + { + delete p; + return ERROR; + } // in case of error, above call should return non-zero code (and report error) DBUG_ASSERT(p->state != backup_state::ERROR); @@ -863,12 +876,6 @@ int Scheduler::add(Pump *p) (unsigned long)m_init_left)); return 0; - - error: - - delete p; - cancel_backup(); - return ERROR; } /// Move backup pump to the end of scheduler's list. @@ -940,9 +947,9 @@ int Scheduler::prepare() for (Pump_iterator it(*this); it; ++it) { - if (it->prepare()) + if (it->prepare()) // logs errors { - cancel_backup(); // Note: never errors. + remove_pump(it); // Note: never errors. return ERROR; } if (it->state == backup_state::PREPARING) @@ -963,9 +970,9 @@ int Scheduler::lock() DBUG_PRINT("backup_data",("calling lock() for all drivers")); for (Pump_iterator it(*this); it; ++it) - if (it->lock()) + if (it->lock()) // logs errors { - cancel_backup(); // Note: never errors. + remove_pump(it); // Note: never errors. return ERROR; } @@ -982,9 +989,9 @@ int Scheduler::unlock() for(Pump_iterator it(*this); it; ++it) { - if (it->unlock()) + if (it->unlock()) // logs errors { - cancel_backup(); // Note: never errors. + remove_pump(it); // Note: never errors. return ERROR; } if (it->state == backup_state::FINISHING) @@ -1041,9 +1048,7 @@ int Backup_pump::begin() if (ERROR == m_drv->begin(m_bw.buf_size)) { state= backup_state::ERROR; - // We check if logger is always setup. Later the assertion can - // be replaced with "if (m_log)" - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_INIT_BACKUP_DRIVER, m_name); return ERROR; } @@ -1061,7 +1066,7 @@ int Backup_pump::end() if (ERROR == m_drv->end()) { state= backup_state::ERROR; - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_STOP_BACKUP_DRIVER, m_name); return ERROR; } @@ -1090,9 +1095,9 @@ int Backup_pump::prepare() case ERROR: default: state= backup_state::ERROR; - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_PREPARE_DRIVER, m_name); - return ERROR; + return ERROR; } DBUG_PRINT("backup_data",(" preparing %s, goes to %s state", @@ -1107,7 +1112,7 @@ int Backup_pump::lock() if (ERROR == m_drv->lock()) { state= backup_state::ERROR; - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_CREATE_VP, m_name); return ERROR; } @@ -1123,7 +1128,7 @@ int Backup_pump::unlock() if (ERROR == m_drv->unlock()) { state= backup_state::ERROR; - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_UNLOCK_DRIVER, m_name); return ERROR; } @@ -1136,7 +1141,7 @@ int Backup_pump::cancel() if (ERROR == m_drv->cancel()) { state= backup_state::ERROR; - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_CANCEL_BACKUP, m_name); return ERROR; } @@ -1217,7 +1222,7 @@ int Backup_pump::pump(size_t *howmuch) case Block_writer::ERROR: default: - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_GET_BUF); state= backup_state::ERROR; return ERROR; @@ -1262,7 +1267,7 @@ int Backup_pump::pump(size_t *howmuch) case ERROR: default: - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_GET_DATA, m_name); state= backup_state::ERROR; return ERROR; @@ -1297,7 +1302,7 @@ int Backup_pump::pump(size_t *howmuch) case Block_writer::ERROR: - DBUG_ASSERT(m_log); + if (m_log) m_log->report_error(ER_BACKUP_WRITE_DATA, m_name, m_buf.table_num); state= backup_state::ERROR; return ERROR; @@ -1340,11 +1345,25 @@ int restore_table_data(THD *thd, Restore if (info.snap_count() == 0 || info.table_count() == 0) // nothing to restore DBUG_RETURN(0); + Logger &log= info.m_log; + + /* Drv[n] points at restore driver used to process snapshot n. */ Restore_driver* drv[256]; + /* + Active[n] is not NULL if driver drv[n] has been activated. Such driver needs + an end() or cancel() call to shut it down properly. + */ + Restore_driver* active[256]; + /* + Bad_drivers string for holding comma separated list of drivers which + signalled errors during shutdown. If non-empty, an error will be logged + at the end of the function (finish: label). + */ + String bad_drivers; if (info.snap_count() > 256) { - info.m_ctx.fatal_error(ER_BACKUP_TOO_MANY_IMAGES, info.snap_count(), 256); + log.report_error(ER_BACKUP_TOO_MANY_IMAGES, info.snap_count(), 256); DBUG_RETURN(ERROR); } @@ -1353,7 +1372,7 @@ int restore_table_data(THD *thd, Restore for (uint n=0; n < info.snap_count(); ++n) { - drv[n]= NULL; + active[n]= drv[n]= NULL; Snapshot_info *snap= info.m_snap[n]; @@ -1364,7 +1383,7 @@ int restore_table_data(THD *thd, Restore res= snap->get_restore_driver(drv[n]); if (res == backup::ERROR) { - info.m_ctx.fatal_error(ER_BACKUP_CREATE_RESTORE_DRIVER, snap->name()); + log.report_error(ER_BACKUP_CREATE_RESTORE_DRIVER, snap->name()); goto error; }; } @@ -1375,9 +1394,11 @@ int restore_table_data(THD *thd, Restore res= drv[n]->begin(0); if (res == backup::ERROR) { - info.m_ctx.fatal_error(ER_BACKUP_INIT_RESTORE_DRIVER, info.m_snap[n]->name()); + log.report_error(ER_BACKUP_INIT_RESTORE_DRIVER, info.m_snap[n]->name()); goto error; } + + active[n]= drv[n]; } DEBUG_SYNC(thd, "restore_in_progress"); @@ -1418,9 +1439,8 @@ int restore_table_data(THD *thd, Restore break; case BSTREAM_ERROR: - info.m_ctx.fatal_error(ER_BACKUP_READ_DATA); + log.report_error(ER_BACKUP_READ_DATA); default: - state= ERROR; goto error; } @@ -1460,7 +1480,8 @@ int restore_table_data(THD *thd, Restore */ DBUG_ASSERT(snap && drvr); - switch( drvr->send_data(buf) ) { + ret= drvr->send_data(buf); + switch (ret) { case backup::OK: info.data_size += buf.size; @@ -1473,8 +1494,13 @@ int restore_table_data(THD *thd, Restore case backup::ERROR: if( errors > MAX_ERRORS ) { - info.m_ctx.fatal_error(ER_BACKUP_SEND_DATA, buf.table_num, snap->name()); - state= ERROR; + log.report_error(ER_BACKUP_SEND_DATA, buf.table_num, snap->name()); + /* + If driver signals error then it is not active any longer - neither + ->end() nor ->cancel() should be called on it, only ->free(). + This is why we need to remove it from active[] array. + */ + active[snap_num]= NULL; goto error; } errors++; @@ -1485,8 +1511,7 @@ int restore_table_data(THD *thd, Restore default: if( repeats > MAX_REPEATS ) { - info.m_ctx.fatal_error(ER_BACKUP_SEND_DATA_RETRY, repeats, snap->name()); - state= ERROR; + log.report_error(ER_BACKUP_SEND_DATA_RETRY, repeats, snap->name()); goto error; } repeats++; @@ -1495,7 +1520,7 @@ int restore_table_data(THD *thd, Restore default: break; - } // switch(state) + } // switch(ret) } // main reading loop @@ -1505,49 +1530,69 @@ int restore_table_data(THD *thd, Restore } DEBUG_SYNC(::current_thd, "restore_table_data_before_end"); - - { // Shutting down drivers - String bad_drivers; + // Call end() for all active drivers. - for (uint n=0; n < info.snap_count(); ++n) - { - if (!drv[n]) - continue; + for (uint n=0; n < info.snap_count(); ++n) + { + if (!active[n]) + continue; - DBUG_PRINT("restore",("Shutting down restore driver %s", - info.m_snap[n]->name())); - res= drv[n]->end(); - if (res == backup::ERROR) - { - state= ERROR; + DBUG_PRINT("restore",("Shutting down restore driver %s", + info.m_snap[n]->name())); + res= active[n]->end(); + if (res == backup::ERROR) + { + state= ERROR; - if (!bad_drivers.is_empty()) - bad_drivers.append(","); - bad_drivers.append(info.m_snap[n]->name()); - } - drv[n]->free(); // Never errors + if (!bad_drivers.is_empty()) + bad_drivers.append(","); + bad_drivers.append(info.m_snap[n]->name()); } - - if (!bad_drivers.is_empty()) - info.m_ctx.report_error(ER_BACKUP_STOP_RESTORE_DRIVERS, bad_drivers.c_ptr()); } - DBUG_RETURN(state == ERROR ? backup::ERROR : 0); + goto finish; - error: +error: + + state= ERROR; DBUG_PRINT("restore",("Cancelling restore process")); + // Call cancel() for all active drivers + for (uint n=0; n < info.snap_count(); ++n) { - if (!drv[n]) + if (!active[n]) continue; + DBUG_PRINT("restore",("Cancelling restore driver %s", + info.m_snap[n]->name())); + res= active[n]->cancel(); + + if (res) + { + if (!bad_drivers.is_empty()) + bad_drivers.append(","); + bad_drivers.append(info.m_snap[n]->name()); + } + } + +finish: + + if (!bad_drivers.is_empty()) + log.report_error(ER_BACKUP_STOP_RESTORE_DRIVERS, bad_drivers.c_ptr()); + + // Call free() for all existing drivers + + for (uint n=0; n < info.snap_count(); ++n) + { + if (!drv[n]) + continue; drv[n]->free(); // Never errors } - DBUG_RETURN(backup::ERROR); + DBUG_RETURN(state == ERROR ? backup::ERROR : 0); } === modified file 'sql/backup/kernel.cc' --- a/sql/backup/kernel.cc 2008-10-28 14:17:05 +0000 +++ b/sql/backup/kernel.cc 2008-11-05 12:40:24 +0000 @@ -154,11 +154,8 @@ execute_backup_command(THD *thd, LEX *le folders in the path could have been moved, deleted, etc. */ if (backupdir->length() && my_access(backupdir->c_ptr(), (F_OK|W_OK))) - { - context.fatal_error(ER_BACKUP_BACKUPDIR, backupdir->c_ptr()); DBUG_RETURN(send_error(context, ER_BACKUP_BACKUPDIR, backupdir->c_ptr())); - } - + switch (lex->sql_command) { case SQLCOM_BACKUP: @@ -195,7 +192,7 @@ execute_backup_command(THD *thd, LEX *le if (info->db_count() == 0) { - context.fatal_error(ER_BACKUP_NOTHING_TO_BACKUP); + context.report_error(ER_BACKUP_NOTHING_TO_BACKUP); DBUG_RETURN(send_error(context, ER_BACKUP_NOTHING_TO_BACKUP)); } @@ -247,23 +244,21 @@ execute_backup_command(THD *thd, LEX *le } /** - Report errors. + Sends error notification after failed backup/restore operation. - Current implementation reports the last error saved in the logger if it exist. - Otherwise it reports error given by @c error_code. + @param[in] ctx The context of the backup/restore operation. + @param[in] error_code Error to be reported if no errors reported yet. - @returns 0 on success, error code otherwise. - */ -int send_error(Backup_restore_ctx &log, int error_code, ...) + 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). + + @returns The error code given as argument. +*/ +static +int send_error(Backup_restore_ctx &context, int error_code, ...) { - util::SAVED_MYSQL_ERROR *error= log.last_saved_error(); - - if (error && !util::report_mysql_error(log.thd(), error, error_code)) - { - if (error->code) - error_code= error->code; - } - else // there are no error information in the logger - report error_code + if (!context.error_reported()) { char buf[ERRMSGSIZE + 20]; va_list args; @@ -275,8 +270,8 @@ int send_error(Backup_restore_ctx &log, va_end(args); } - if (log.backup::Logger::m_state == backup::Logger::RUNNING) - log.report_stop(my_time(0), FALSE); // FASLE = no success + if (context.backup::Logger::m_state == backup::Logger::RUNNING) + context.report_stop(my_time(0), FALSE); // FASLE = no success return error_code; } @@ -287,6 +282,8 @@ int send_error(Backup_restore_ctx &log, Currently the id of the operation is returned to the client. It can be used to select correct entries from the backup progress tables. + @note If an error has been reported, send_error() is invoked instead. + @returns 0 on success, error code otherwise. */ int send_reply(Backup_restore_ctx &context) @@ -297,6 +294,9 @@ int send_reply(Backup_restore_ctx &conte DBUG_ENTER("send_reply"); + if (context.error_reported()) + return send_error(context, ER_UNKNOWN_ERROR); + /* Send field list. */ @@ -328,9 +328,9 @@ int send_reply(Backup_restore_ctx &conte DBUG_RETURN(0); err: - DBUG_RETURN(context.fatal_error(ER_BACKUP_SEND_REPLY, - context.m_type == backup::Logger::BACKUP - ? "BACKUP" : "RESTORE")); + DBUG_RETURN(context.report_error(ER_BACKUP_SEND_REPLY, + context.m_type == backup::Logger::BACKUP + ? "BACKUP" : "RESTORE")); } @@ -488,22 +488,17 @@ int Backup_restore_ctx::prepare(String * if (m_error) return m_error; - // Prepare error reporting context. - - mysql_reset_errors(m_thd, 0); // Never errors - m_thd->no_warnings_for_error= FALSE; - - save_errors(); // Never errors - + int ret= 0; /* Check access for SUPER rights. If user does not have SUPER, fail with error. + + In case of error, we write only to backup logs, because check_global_access() + pushes the same error on the error stack. */ - if (check_global_access(m_thd, SUPER_ACL)) - { - fatal_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, "SUPER"); - return m_error; - } + ret= check_global_access(m_thd, SUPER_ACL); + if (ret) + return fatal_error(log_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, "SUPER")); /* Check if another BACKUP/RESTORE is running and if not, register @@ -520,7 +515,10 @@ int Backup_restore_ctx::prepare(String * pthread_mutex_unlock(&run_lock); if (m_error) + { + report_error(ER_BACKUP_RUNNING); return m_error; + } // check if location is valid (we assume it is a file path) @@ -542,10 +540,7 @@ int Backup_restore_ctx::prepare(String * #endif if (bad_filename) - { - fatal_error(ER_BAD_PATH, location.str); - return m_error; - } + return fatal_error(report_error(ER_BAD_PATH, location.str)); /* Computer full path to backup file. @@ -560,18 +555,13 @@ int Backup_restore_ctx::prepare(String * mem_alloc= new Mem_allocator(); if (!mem_alloc) - { - fatal_error(ER_OUT_OF_RESOURCES); - return m_error; - } + return fatal_error(report_error(ER_OUT_OF_RESOURCES)); // Freeze all meta-data. - if (obs::ddl_blocker_enable(m_thd)) - { - fatal_error(ER_DDL_BLOCK); - return m_error; - } + ret= obs::ddl_blocker_enable(m_thd); + if (ret) + return fatal_error(report_error(ER_DDL_BLOCK)); return 0; } @@ -631,10 +621,13 @@ Backup_restore_ctx::prepare_for_backup(S if (!s) { - fatal_error(ER_OUT_OF_RESOURCES); + fatal_error(report_error(ER_OUT_OF_RESOURCES)); return NULL; } - + + // Mark that the file should be removed unless operation completes successfuly + m_remove_loc= TRUE; + int my_open_status= s->open(); if (my_open_status != 0) { @@ -650,12 +643,16 @@ Backup_restore_ctx::prepare_for_backup(S if (!info) { - fatal_error(ER_OUT_OF_RESOURCES); + fatal_error(report_error(ER_OUT_OF_RESOURCES)); return NULL; } if (!info->is_valid()) - return NULL; // Error has been logged by Backup_Info constructor + { + // Error has been logged by Backup_Info constructor + fatal_error(ER_BACKUP_BACKUP_PREPARE); + return NULL; + } /* If binlog is enabled, set BSTREAM_FLAG_BINLOG in the header to indicate @@ -726,7 +723,7 @@ Backup_restore_ctx::prepare_for_restore( if (!s) { - fatal_error(ER_OUT_OF_RESOURCES); + fatal_error(report_error(ER_OUT_OF_RESOURCES)); return NULL; } @@ -745,41 +742,40 @@ Backup_restore_ctx::prepare_for_restore( if (!info) { - fatal_error(ER_OUT_OF_RESOURCES); + fatal_error(report_error(ER_OUT_OF_RESOURCES)); return NULL; } if (!info->is_valid()) + { + fatal_error(ER_BACKUP_RESTORE_PREPARE); // Errors logged by Restore_info ctor. return NULL; + } info->save_start_time(when); m_catalog= info; + int ret; + /* - Read catalogue from the input stream. + Read header and catalogue from the input stream. */ - if (read_header(*info, *s)) - { - fatal_error(ER_BACKUP_READ_HEADER); - return NULL; - } - - if (s->next_chunk() != BSTREAM_OK) + ret= read_header(*info, *s); // Can log errors via callback functions. + if (ret) { - fatal_error(ER_BACKUP_NEXT_CHUNK); + if (!error_reported()) + report_error(ER_BACKUP_READ_HEADER); + fatal_error(ret); return NULL; } - if (read_catalog(*info, *s)) + ret= read_catalog(*info, *s); // Can log errors via callback functions. + if (ret) { - fatal_error(ER_BACKUP_READ_HEADER); - return NULL; - } - - if (s->next_chunk() != BSTREAM_OK) - { - fatal_error(ER_BACKUP_NEXT_CHUNK); + if (!error_reported()) + report_error(ER_BACKUP_READ_HEADER); + fatal_error(ret); return NULL; } @@ -807,6 +803,7 @@ Backup_restore_ctx::prepare_for_restore( int Backup_restore_ctx::lock_tables_for_restore() { TABLE_LIST *tables= NULL; + int ret; /* Iterate over all tables in all snapshots and create a linked TABLE_LIST @@ -827,7 +824,7 @@ int Backup_restore_ctx::lock_tables_for_ if (!ptr) { // Error has been reported, but not logged to backup logs - return log_error(ER_OUT_OF_RESOURCES); + return fatal_error(log_error(ER_OUT_OF_RESOURCES)); } tables= backup::link_table_list(*ptr, tables); // Never errors @@ -842,11 +839,9 @@ int Backup_restore_ctx::lock_tables_for_ to do derived tables processing. Processing derived tables even leads to crashes as those reported in BUG#34758. */ - if (simple_open_n_lock_tables(m_thd,tables)) - { - fatal_error(ER_BACKUP_OPEN_TABLES,"RESTORE"); - return m_error; - } + ret= simple_open_n_lock_tables(m_thd,tables); + if (ret) + return fatal_error(report_error(ER_BACKUP_OPEN_TABLES,"RESTORE")); m_tables_locked= TRUE; return 0; @@ -871,39 +866,6 @@ void Backup_restore_ctx::unlock_tables() /** - Report error and move context object into error state without pushing the - error on the server's warning stack. - - Similar to @c fatal_error, but does not push the error on the - server's warning stack. To be used when an error is reported from a - server function that has already pushed the error on the warning stack. - - @return error code given as input or stored in the context object if - a fatal error was reported before. - */ -inline -int Backup_restore_ctx::log_error(int error_code, ...) -{ - if (m_error) - return m_error; - - bool saved = push_errors(FALSE); // Do not use warning stack - - m_error= error_code; - m_remove_loc= TRUE; - - va_list args; - va_start(args,error_code); - v_report_error(backup::log_level::ERROR, error_code, args); - va_end(args); - - push_errors(saved); // Reset - - return error_code; -} - - -/** Destroy a backup/restore context. This should reverse all settings made when context was created and prepared. @@ -917,7 +879,6 @@ int Backup_restore_ctx::log_error(int er */ int Backup_restore_ctx::close() { - int error= 0; if (m_state == CLOSED) return 0; @@ -940,7 +901,7 @@ int Backup_restore_ctx::close() if (m_stream && !m_stream->close()) { // Note error, but complete clean-up - error= ER_BACKUP_CLOSE; + fatal_error(report_error(ER_BACKUP_CLOSE)); } if (m_catalog) @@ -960,25 +921,19 @@ int Backup_restore_ctx::close() Ignore ENOENT error since it is ok if the file doesn't exist. */ if (res && my_errno != ENOENT) - { - error= ER_CANT_DELETE_FILE; - } + fatal_error(report_error(ER_CANT_DELETE_FILE, m_path.c_ptr(), my_errno)); } /* We report completion of the operation only if no errors were detected, and logger has been initialized. */ - if (!error) + if (!m_error) { if (backup::Logger::m_state == backup::Logger::RUNNING) { report_stop(when, TRUE); } } - else - { - fatal_error(error); // Log error - } /* Destroy backup stream's memory allocator (this frees memory) @@ -999,7 +954,7 @@ int Backup_restore_ctx::close() pthread_mutex_unlock(&run_lock); m_state= CLOSED; - return error; + return m_error; } /** @@ -1022,6 +977,7 @@ int Backup_restore_ctx::do_backup() using namespace backup; + int ret; Output_stream &s= *static_cast(m_stream); Backup_info &info= *static_cast(m_catalog); @@ -1032,27 +988,33 @@ int Backup_restore_ctx::do_backup() DBUG_PRINT("backup",("Writing preamble")); DEBUG_SYNC(m_thd, "backup_before_write_preamble"); - if (write_preamble(info, s)) + ret= write_preamble(info, s); // Can Log errors via callback functions. + if (ret) { - fatal_error(ER_BACKUP_WRITE_HEADER); - DBUG_RETURN(m_error); + if (!error_reported()) + report_error(ER_BACKUP_WRITE_HEADER); + DBUG_RETURN(fatal_error(ret)); } DBUG_PRINT("backup",("Writing table data")); DEBUG_SYNC(m_thd, "before_backup_data"); - if (write_table_data(m_thd, info, s)) // logs errors - DBUG_RETURN(send_error(*this, ER_BACKUP_BACKUP)); + ret= write_table_data(m_thd, info, s); // logs errors + if (ret) + DBUG_RETURN(fatal_error(ret)); DBUG_PRINT("backup",("Writing summary")); - if (write_summary(info, s)) - { - fatal_error(ER_BACKUP_WRITE_SUMMARY); - DBUG_RETURN(m_error); - } + ret= write_summary(info, s); + if (ret) + DBUG_RETURN(fatal_error(report_error(ER_BACKUP_WRITE_SUMMARY))); + /* + Now backup image has been written. Set m_remove_loc to FALSE, so that the + backup file is not removed in Backup_restore_ctx::close(). + */ + m_remove_loc= FALSE; report_stats_post(info); // Never errors DBUG_PRINT("backup",("Backup done.")); @@ -1089,9 +1051,7 @@ int Backup_restore_ctx::restore_triggers Image_info::Iterator *dbit= m_catalog->get_dbs(); if (!dbit) - { - DBUG_RETURN(fatal_error(ER_OUT_OF_RESOURCES)); - } + DBUG_RETURN(fatal_error(report_error(ER_OUT_OF_RESOURCES))); // create all trigers and collect events in the events list @@ -1100,9 +1060,8 @@ int Backup_restore_ctx::restore_triggers Image_info::Iterator *it= m_catalog->get_db_objects(*static_cast(obj)); if (!it) - { - DBUG_RETURN(fatal_error(ER_OUT_OF_RESOURCES)); - } + DBUG_RETURN(fatal_error(report_error(ER_OUT_OF_RESOURCES))); + while ((obj= (*it)++)) switch (obj->type()) { @@ -1111,7 +1070,7 @@ int Backup_restore_ctx::restore_triggers if (events.push_back(obj)) { // Error has been reported, but not logged to backup logs - DBUG_RETURN(log_error(ER_OUT_OF_RESOURCES)); + DBUG_RETURN(fatal_error(log_error(ER_OUT_OF_RESOURCES))); } break; @@ -1121,8 +1080,8 @@ int Backup_restore_ctx::restore_triggers { delete it; delete dbit; - fatal_error(ER_BACKUP_CANT_RESTORE_TRIGGER,obj->describe(buf)); - DBUG_RETURN(m_error); + int err= report_error(ER_BACKUP_CANT_RESTORE_TRIGGER,obj->describe(buf)); + DBUG_RETURN(fatal_error(err)); } break; @@ -1142,8 +1101,8 @@ int Backup_restore_ctx::restore_triggers while ((ev= it++)) if (ev->m_obj_ptr->execute(m_thd)) { - fatal_error(ER_BACKUP_CANT_RESTORE_EVENT,ev->describe(buf)); - DBUG_RETURN(m_error); + int err= report_error(ER_BACKUP_CANT_RESTORE_EVENT,ev->describe(buf)); + DBUG_RETURN(fatal_error(err)); }; DBUG_RETURN(0); @@ -1180,17 +1139,12 @@ int Backup_restore_ctx::do_restore() disable_fkey_constraints(); // Never errors - if (read_meta_data(info, s)) - { - m_thd->main_da.reset_diagnostics_area(); // Never errors - - fatal_error(ER_BACKUP_READ_META); - DBUG_RETURN(m_error); - } - - if (s.next_chunk() == BSTREAM_ERROR) + err= read_meta_data(info, s); // Can log errors via callback functions. + if (err) { - DBUG_RETURN(fatal_error(ER_BACKUP_NEXT_CHUNK)); + if (!error_reported()) + report_error(ER_BACKUP_READ_META); + DBUG_RETURN(fatal_error(err)); } DBUG_PRINT("restore",("Restoring table data")); @@ -1204,16 +1158,17 @@ int Backup_restore_ctx::do_restore() close_thread_tables(m_thd); // Never errors m_thd->main_da.reset_diagnostics_area(); // Never errors - if (lock_tables_for_restore()) // logs errors - DBUG_RETURN(m_error); + err= lock_tables_for_restore(); // logs errors + if (err) + DBUG_RETURN(fatal_error(err)); // Here restore drivers are created to restore table data - err= restore_table_data(m_thd, info, s); // reports errors + err= restore_table_data(m_thd, info, s); // logs errors unlock_tables(); // Never errors if (err) - DBUG_RETURN(ER_BACKUP_RESTORE); + DBUG_RETURN(fatal_error(err)); /* Re-create all triggers and events (it was not done in @c bcat_create_item()). @@ -1222,16 +1177,9 @@ int Backup_restore_ctx::do_restore() creation of these objects will fail. */ - if (restore_triggers_and_events()) // reports errors - DBUG_RETURN(ER_BACKUP_RESTORE); - - DBUG_PRINT("restore",("Done.")); - - if (read_summary(info, s)) - { - fatal_error(ER_BACKUP_READ_SUMMARY); - DBUG_RETURN(m_error); - } + err= restore_triggers_and_events(); // logs errors + if (err) + DBUG_RETURN(fatal_error(err)); /* FIXME: this call is here because object services doesn't clean the @@ -1242,6 +1190,12 @@ int Backup_restore_ctx::do_restore() close_thread_tables(m_thd); // Never errors m_thd->main_da.reset_diagnostics_area(); // Never errors + DBUG_PRINT("restore",("Done.")); + + err= read_summary(info, s); + if (err) + DBUG_RETURN(report_error(ER_BACKUP_READ_SUMMARY)); + /* Report validity point time and binlog position stored in the backup image (in the summary section). @@ -1259,8 +1213,7 @@ int Backup_restore_ctx::do_restore() } /** - Report stream open error by calling fatal_error, effectively moving - context object into error state. + Report stream open error and move context object into error state. @return error code given as input or the one stored in the context object if a fatal error has already been reported. @@ -1271,26 +1224,26 @@ int Backup_restore_ctx::report_stream_op int error= 0; switch (my_open_status) { case ER_OPTION_PREVENTS_STATEMENT: - error= fatal_error(ER_OPTION_PREVENTS_STATEMENT, "--secure-file-priv"); + error= report_error(ER_OPTION_PREVENTS_STATEMENT, "--secure-file-priv"); break; case ER_BACKUP_WRITE_LOC: /* For this error, use the actual value returned instead of the path complimented with backupdir. */ - error= fatal_error(ER_BACKUP_WRITE_LOC, location->str); + error= report_error(ER_BACKUP_WRITE_LOC, location->str); break; case ER_BACKUP_READ_LOC: /* For this error, use the actual value returned instead of the path complimented with backupdir. */ - error= fatal_error(ER_BACKUP_READ_LOC, location->str); + error= report_error(ER_BACKUP_READ_LOC, location->str); break; default: DBUG_ASSERT(FALSE); } - return error; + return fatal_error(error); } namespace backup { @@ -1435,6 +1388,7 @@ int bcat_reset(st_bstream_image_header * DBUG_ASSERT(catalogue); Restore_info *info= static_cast(catalogue); + Logger &log= info->m_log; /* Iterate over the list of snapshots read from the backup image (and stored @@ -1459,50 +1413,50 @@ int bcat_reset(st_bstream_image_header * if (!se || !hton) { - info->m_ctx.fatal_error(ER_BACKUP_CANT_FIND_SE, name_lex.str); + log.report_error(ER_BACKUP_CANT_FIND_SE, name_lex.str); return BSTREAM_ERROR; } if (!hton->get_backup_engine) { - info->m_ctx.fatal_error(ER_BACKUP_NO_NATIVE_BE, name_lex.str); + log.report_error(ER_BACKUP_NO_NATIVE_BE, name_lex.str); return BSTREAM_ERROR; } - info->m_snap[n]= new Native_snapshot(info->m_ctx, snap->version, se); + info->m_snap[n]= new Native_snapshot(log, snap->version, se); // reports errors break; } case BI_NODATA: - info->m_snap[n]= new Nodata_snapshot(info->m_ctx, snap->version); + info->m_snap[n]= new Nodata_snapshot(log, snap->version); // reports errors break; case BI_CS: - info->m_snap[n]= new CS_snapshot(info->m_ctx, snap->version); + info->m_snap[n]= new CS_snapshot(log, snap->version); // reports errors break; case BI_DEFAULT: - info->m_snap[n]= new Default_snapshot(info->m_ctx, snap->version); + info->m_snap[n]= new Default_snapshot(log, snap->version); // reports errors break; default: // note: we use convention that snapshots are counted starting from 1. - info->m_ctx.fatal_error(ER_BACKUP_UNKNOWN_BE, n + 1); + log.report_error(ER_BACKUP_UNKNOWN_BE, n + 1); return BSTREAM_ERROR; } if (!info->m_snap[n]) { - info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + log.report_error(ER_OUT_OF_RESOURCES); return BSTREAM_ERROR; } info->m_snap[n]->m_num= n + 1; - info->m_ctx.report_driver(info->m_snap[n]->name()); + log.report_driver(info->m_snap[n]->name()); } return BSTREAM_OK; @@ -1532,6 +1486,7 @@ int bcat_add_item(st_bstream_image_heade using namespace backup; Restore_info *info= static_cast(catalogue); + Logger &log= info->m_log; backup::String name_str(item->name.begin, item->name.end); @@ -1572,7 +1527,7 @@ int bcat_add_item(st_bstream_image_heade with error earlier. */ DBUG_ASSERT(it->snap_num >= info->snap_count()); - info->m_ctx.fatal_error(ER_BACKUP_WRONG_TABLE_BE, it->snap_num + 1); + log.report_error(ER_BACKUP_WRONG_TABLE_BE, it->snap_num + 1); return BSTREAM_ERROR; } @@ -1636,6 +1591,7 @@ void* bcat_iterator_get(st_bstream_image DBUG_ASSERT(catalogue); Backup_info *info= static_cast(catalogue); + backup::Logger &log= info->m_log; switch (type) { @@ -1654,7 +1610,7 @@ void* bcat_iterator_get(st_bstream_image Iterator *it= info->get_tablespaces(); if (!it) { - info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + log.report_error(ER_OUT_OF_RESOURCES); return NULL; } @@ -1666,7 +1622,7 @@ void* bcat_iterator_get(st_bstream_image Iterator *it= info->get_dbs(); if (!it) { - info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + log.report_error(ER_OUT_OF_RESOURCES); return NULL; } @@ -1679,7 +1635,7 @@ void* bcat_iterator_get(st_bstream_image if (!it) { - info->m_ctx.fatal_error(ER_BACKUP_CAT_ENUM); + log.report_error(ER_BACKUP_CAT_ENUM); return NULL; } @@ -1770,18 +1726,19 @@ void* bcat_db_iterator_get(st_bstream_im DBUG_ASSERT(dbi); Backup_info *info= static_cast(catalogue); + backup::Logger &log= info->m_log; Backup_info::Db *db = info->get_db(dbi->base.pos); if (!db) { - info->m_ctx.fatal_error(ER_BACKUP_UNKNOWN_OBJECT); + log.report_error(ER_BACKUP_UNKNOWN_OBJECT); return NULL; } backup::Image_info::Iterator *it= info->get_db_objects(*db); if (!it) { - info->m_ctx.fatal_error(ER_OUT_OF_RESOURCES); + log.report_error(ER_OUT_OF_RESOURCES); return NULL; } @@ -1834,7 +1791,8 @@ int bcat_create_item(st_bstream_image_he DBUG_ASSERT(item); Restore_info *info= static_cast(catalogue); - THD *thd= info->m_ctx.thd(); + Logger &log= info->m_log; + THD *thd= info->m_thd; int create_err= 0; switch (item->type) { @@ -1856,7 +1814,7 @@ int bcat_create_item(st_bstream_image_he */ default: - info->m_ctx.fatal_error(ER_BACKUP_UNKNOWN_OBJECT_TYPE); + log.report_error(ER_BACKUP_UNKNOWN_OBJECT_TYPE); return BSTREAM_ERROR; } @@ -1864,7 +1822,7 @@ int bcat_create_item(st_bstream_image_he if (!obj) { - info->m_ctx.fatal_error(ER_BACKUP_UNKNOWN_OBJECT); + log.report_error(ER_BACKUP_UNKNOWN_OBJECT); return BSTREAM_ERROR; } @@ -1883,7 +1841,7 @@ int bcat_create_item(st_bstream_image_he if (!sobj) { - info->m_ctx.fatal_error(create_err, desc); + log.report_error(create_err, desc); return BSTREAM_ERROR; } @@ -1925,7 +1883,7 @@ int bcat_create_item(st_bstream_image_he if (ts) { DBUG_PRINT("restore",(" tablespace has changed on the server - aborting")); - info->m_ctx.fatal_error(ER_BACKUP_TS_CHANGE, desc); + log.report_error(ER_BACKUP_TS_CHANGE, desc); return BSTREAM_ERROR; } } @@ -1947,9 +1905,8 @@ int bcat_create_item(st_bstream_image_he */ if (!obs::check_user_existence(thd, sobj->get_name())) { - info->m_ctx.write_message(log_level::WARNING, - ER(ER_BACKUP_GRANT_SKIPPED), - create_stmt); + log.report_error(log_level::WARNING, ER_BACKUP_GRANT_SKIPPED, + create_stmt); return BSTREAM_OK; } /* @@ -1969,14 +1926,14 @@ int bcat_create_item(st_bstream_image_he db_name.append(start, size); if (!info->has_db(db_name)) { - info->m_ctx.fatal_error(ER_BACKUP_GRANT_WRONG_DB, create_stmt); + log.report_error(ER_BACKUP_GRANT_WRONG_DB, create_stmt); return BSTREAM_ERROR; } } if (sobj->execute(thd)) { - info->m_ctx.fatal_error(create_err, desc); + log.report_error(create_err, desc); return BSTREAM_ERROR; } @@ -2047,11 +2004,12 @@ int bcat_get_item_create_query(st_bstrea ::String *buf= &(info->serialization_buf); buf->length(0); - if (obj->m_obj_ptr->serialize(info->m_ctx.thd(), buf)) + if (obj->m_obj_ptr->serialize(info->m_thd, buf)) { Image_info::Obj::describe_buf dbuf; - info->m_ctx.fatal_error(meta_err, obj->describe(dbuf)); + info->m_log.report_error(meta_err, obj->describe(dbuf)); + return BSTREAM_ERROR; } === modified file 'sql/backup/logger.cc' --- a/sql/backup/logger.cc 2008-10-27 13:06:21 +0000 +++ b/sql/backup/logger.cc 2008-11-05 12:40:24 +0000 @@ -28,7 +28,7 @@ namespace backup { the moment. Destinations which are not ready/initialized yet should be silently ignored. - @returns 0 on success. + @returns Reported error code. */ int Logger::write_message(log_level::value level, int error_code, const char *msg) @@ -36,7 +36,11 @@ int Logger::write_message(log_level::val char buf[ERRMSGSIZE + 30]; const char *out= msg; - if (m_state == READY || m_state == RUNNING) + /* + Note: m_type is meaningful only after a call to init() i.e., + if m_state != CREATED. + */ + if (m_state != CREATED) { my_snprintf(buf, sizeof(buf), "%s: %s", m_type == BACKUP ? "Backup" : "Restore" , msg); @@ -45,43 +49,61 @@ int Logger::write_message(log_level::val switch (level) { case log_level::ERROR: - if (m_save_errors) - { - error.code= error_code; - error.level= MYSQL_ERROR::WARN_LEVEL_ERROR; - error.msg= sql_strdup(msg); - } + { + // Report to server's error log sql_print_error(out); - if (m_push_errors) - push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, - error_code, msg); - DBUG_PRINT("backup_log",("[ERROR] %s", out)); - + + // Report to the client + + bool saved_value= m_thd->no_warnings_for_error; + m_thd->no_warnings_for_error= m_push_errors ? FALSE : TRUE; + my_printf_error(error_code, msg, MYF(0)); + m_thd->no_warnings_for_error= saved_value; + + m_error_reported= TRUE; + + // Report to backup logs + if (m_state == READY || m_state == RUNNING) { time_t ts = my_time(0); backup_log->error_num(error_code); - backup_log->write_progress(0, ts, ts, 0, 0, error_code, out); + backup_log->write_progress(0, ts, ts, 0, 0, error_code, msg); } - return 0; + // Report in the debug trace + + DBUG_PRINT("backup_log",("[ERROR] %s", out)); + + return error_code; + } case log_level::WARNING: + // Report to server's error log sql_print_warning(out); + + // Report to the client (push on warning stack) if (m_push_errors) push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, error_code, msg); + + // Report to the debug trace DBUG_PRINT("backup_log",("[Warning] %s", out)); - return 0; + + return error_code; case log_level::INFO: + // Report to server's error log sql_print_information(out); + + // Report to the debug trace DBUG_PRINT("backup_log",("[Info] %s", out)); - return 0; - default: return ERROR; + return error_code; + + default: DBUG_ASSERT(0); return ERROR; } } @@ -94,7 +116,7 @@ int Logger::write_message(log_level::val If the message contains placeholders, additional arguments provide values to be put there. - @returns 0 on success. + @returns Reported error code. */ int Logger::v_report_error(log_level::value level, int error_code, va_list args) { === modified file 'sql/backup/logger.h' --- a/sql/backup/logger.h 2008-10-27 13:06:21 +0000 +++ b/sql/backup/logger.h 2008-11-05 12:40:24 +0000 @@ -50,6 +50,7 @@ class Logger int report_error(log_level::value level, int error_code, ...); int report_error(const char *format, ...); int write_message(log_level::value level, const char *msg, ...); + int log_error(int error_code, ...); void report_start(time_t); void report_stop(time_t, bool); @@ -67,11 +68,8 @@ class Logger } void report_cleanup() { delete backup_log; } - void save_errors(); - void stop_save_errors(); - void clear_saved_errors(); - util::SAVED_MYSQL_ERROR *last_saved_error(); bool push_errors(bool); + bool error_reported() const; protected: @@ -83,24 +81,25 @@ class Logger int write_message(log_level::value level , int error_code, const char *msg); private: - util::SAVED_MYSQL_ERROR error; ///< Used to store saved errors. - bool m_save_errors; ///< Flag telling if errors should be saved. + bool m_push_errors; ///< Should errors be pushed on warning stack? + bool m_error_reported; ///< Has any error been reported? Backup_log *backup_log; ///< Backup log interface class. + + // Disable copying of Logger instances + Logger(const Logger&) { DBUG_ASSERT(0); }; }; inline Logger::Logger(THD *thd) - :m_type(BACKUP), m_state(CREATED), - m_thd(thd), m_save_errors(FALSE), m_push_errors(TRUE), backup_log(0) + :m_type(BACKUP), m_state(CREATED), m_thd(thd), m_push_errors(TRUE), + m_error_reported(FALSE), backup_log(0) {} inline Logger::~Logger() -{ - clear_saved_errors(); -} +{} inline int Logger::write_message(log_level::value level, const char *msg, ...) @@ -153,37 +152,20 @@ int Logger::report_error(const char *for return res; } -/// Request that all reported errors are saved in the logger. +/// Reports error without pushing it on server's error stack. inline -void Logger::save_errors() +int Logger::log_error(int error_code, ...) { - if (m_save_errors) - return; - clear_saved_errors(); - m_save_errors= TRUE; -} - -/// Stop saving errors. -inline -void Logger::stop_save_errors() -{ - if (!m_save_errors) - return; - m_save_errors= FALSE; -} + va_list args; + bool saved= push_errors(FALSE); + + va_start(args, error_code); + int res= v_report_error(log_level::ERROR, error_code, args); + va_end(args); -/// Delete all saved errors to free resources. -inline -void Logger::clear_saved_errors() -{ - memset(&error, 0, sizeof(error)); -} + push_errors(saved); -/// Return a pointer to most recent saved error. -inline -util::SAVED_MYSQL_ERROR *Logger::last_saved_error() -{ - return error.code ? &error : NULL; + return res; } /// Report start of an operation. @@ -312,13 +294,20 @@ int Logger::init(enum_type type, const c return 0; m_type= type; - m_state= READY; + mysql_reset_errors(m_thd, 0); // Never errors backup_log = new Backup_log(m_thd, (enum_backup_operation)type, query); backup_log->state(BUP_STARTING); + m_state= READY; DEBUG_SYNC(m_thd, "after_backup_log_init"); return 0; } +inline +bool Logger::error_reported() const +{ + return m_error_reported; +} + } // backup namespace #endif === modified file 'sql/backup/restore_info.h' --- a/sql/backup/restore_info.h 2008-05-05 15:06:40 +0000 +++ b/sql/backup/restore_info.h 2008-11-05 12:40:24 +0000 @@ -33,7 +33,7 @@ class Restore_info: public backup::Image { public: - Backup_restore_ctx &m_ctx; + backup::Logger &m_log; Restore_info(Backup_restore_ctx&); ~Restore_info(); @@ -47,15 +47,21 @@ class Restore_info: public backup::Image private: + THD *m_thd; + friend int backup::restore_table_data(THD*, Restore_info&, backup::Input_stream&); friend int ::bcat_add_item(st_bstream_image_header*, struct st_bstream_item_info*); + friend int ::bcat_create_item(st_bstream_image_header *catalogue, + struct st_bstream_item_info *item, + bstream_blob create_stmt, + bstream_blob other_meta_data); }; inline Restore_info::Restore_info(Backup_restore_ctx &ctx) - :m_ctx(ctx) + :m_log(ctx), m_thd(ctx.thd()) {} inline @@ -82,7 +88,7 @@ Restore_info::add_ts(const ::String &nam Ts *ts= Image_info::add_ts(name, pos); if (!ts) - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_TS, name.ptr()); + m_log.report_error(ER_BACKUP_CATALOG_ADD_TS, name.ptr()); return ts; } @@ -95,7 +101,7 @@ Restore_info::add_db(const ::String &nam Db *db= Image_info::add_db(name, pos); if (!db) - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_DB, name.ptr()); + m_log.report_error(ER_BACKUP_CATALOG_ADD_DB, name.ptr()); return db; } @@ -109,7 +115,7 @@ Restore_info::add_table(Image_info::Db & Table *t= Image_info::add_table(db, name, snap, pos); if (!t) - m_ctx.fatal_error(ER_BACKUP_CATALOG_ADD_TABLE, db.name().ptr(), name.ptr()); + m_log.report_error(ER_BACKUP_CATALOG_ADD_TABLE, db.name().ptr(), name.ptr()); return t; } === modified file 'sql/backup/stream.cc' --- a/sql/backup/stream.cc 2008-10-15 20:00:48 +0000 +++ b/sql/backup/stream.cc 2008-11-05 12:40:24 +0000 @@ -645,10 +645,4 @@ bool Input_stream::rewind() return ret ? init() : FALSE; } -/// Move to next chunk in the stream. -int Input_stream::next_chunk() -{ - return bstream_next_chunk(this); -} - } // backup namespace === modified file 'sql/backup/stream.h' --- a/sql/backup/stream.h 2008-10-15 20:00:48 +0000 +++ b/sql/backup/stream.h 2008-11-05 12:40:24 +0000 @@ -98,7 +98,7 @@ class Stream: public fd_stream ::String *m_path; int m_flags; ///< flags used when opening the file size_t m_block_size; - Logger m_log; + Logger &m_log; friend int stream_write(void*, bstream_blob*, bstream_blob); friend int stream_read(void*, bstream_blob*, bstream_blob); @@ -139,8 +139,6 @@ class Input_stream: bool close(); bool rewind(); - int next_chunk(); - private: int check_magic_and_version(); @@ -181,6 +179,10 @@ result_t read_header(Image_info &info, Input_stream &s) { int ret= bstream_rd_header(&s, static_cast(&info)); + + if (ret != BSTREAM_ERROR) + ret= bstream_next_chunk(&s); + return ret == BSTREAM_ERROR ? ERROR : OK; } @@ -189,6 +191,10 @@ result_t read_catalog(Image_info &info, Input_stream &s) { int ret= bstream_rd_catalogue(&s, static_cast(&info)); + + if (ret != BSTREAM_ERROR) + ret= bstream_next_chunk(&s); + return ret == BSTREAM_ERROR ? ERROR : OK; } @@ -197,6 +203,10 @@ result_t read_meta_data(Image_info &info, Input_stream &s) { int ret= bstream_rd_meta_data(&s, static_cast(&info)); + + if (ret != BSTREAM_ERROR) + ret= bstream_next_chunk(&s); + return ret == BSTREAM_ERROR ? ERROR : OK; } === modified file 'sql/share/errmsg.txt' --- a/sql/share/errmsg.txt 2008-10-28 18:14:14 +0000 +++ b/sql/share/errmsg.txt 2008-11-05 12:40:24 +0000 @@ -6416,3 +6416,6 @@ ER_MASTER_BLOCKING_SLAVES eng "The master is not allowing slave connections." ER_RESTORE_ON_MASTER eng "A restore operation was initiated on the master." + +ER_BACKUP_VP_FAILED + eng "Could not create validity point of the image"