#At file:///home/thava/repo/backup/ based on revid:charles.bell@stripped
2891 Thava Alagu 2009-11-18
BUG#47994 : Interruption of BACKUP command reported multiple times.
When backup session is interrupted at certain stages, multiple
duplicate warning messages are generated.
Added additional checks to catch interruption of the command
earlier to stop backup kernel from executing further queries.
Removed some duplicate pushing of warnings.
@ sql/si_objects.cc
Removed some redundant pushing of messages.
Still user may get some duplicate query interrupted messages.
Todo: The duplicate messages should be filtered out when they are
pushed into thread warnings stack.
modified:
mysql-test/suite/backup/r/backup_errors.result
mysql-test/suite/backup/r/backup_errors_compression.result
mysql-test/suite/backup/t/backup_errors.test
mysql-test/suite/backup_engines/r/backup_interruption.result
sql/backup/backup_info.cc
sql/backup/kernel.cc
sql/backup/logger.h
sql/share/errmsg-utf8.txt
sql/share/errmsg.txt
sql/si_logs.cc
sql/si_logs.h
sql/si_objects.cc
sql/si_objects.h
=== modified file 'mysql-test/suite/backup/r/backup_errors.result'
--- a/mysql-test/suite/backup/r/backup_errors.result 2009-10-21 12:15:53 +0000
+++ b/mysql-test/suite/backup/r/backup_errors.result 2009-11-17 19:43:39 +0000
@@ -595,6 +595,37 @@ SET SESSION DEBUG='-d';
#
# Test error handling by injecting errors into BACKUP
#
+# non-existent database
+DROP DATABASE IF EXISTS db1;
+DROP DATABASE IF EXISTS db2;
+CREATE DATABASE db1;
+USE db1;
+CREATE TABLE db1_tab (i int);
+INSERT INTO db1_tab VALUES (1), (2), (3);
+CREATE DATABASE db2;
+USE db2;
+CREATE TABLE db2_tab (v varchar(64));
+INSERT INTO db2_tab VALUES ("This is table db2_tab");
+#
+# Backup without error injection
+#
+SET SESSION DEBUG='-d';
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+backup_id
+#
+SHOW WARNINGS;
+Level Code Message
+#
+# Error injection in user privileges checking
+#
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_db';
+BACKUP DATABASE db1 TO 'db1.bak' OVERWRITE;
+ERROR HY000: Failed to perform access privileges check for user 'root'.
+SET SESSION DEBUG='-d';
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_all_dbs';
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+ERROR HY000: Failed to perform access privileges check for user 'root'.
+SET SESSION DEBUG='-d';
#
# Error injection in si_object.cc:get_database_stub
#
@@ -606,3 +637,4 @@ SET SESSION DEBUG='-d';
# Cleanup
#
DROP DATABASE db1;
+DROP DATABASE db2;
=== modified file 'mysql-test/suite/backup/r/backup_errors_compression.result'
--- a/mysql-test/suite/backup/r/backup_errors_compression.result 2009-10-23 15:41:56 +0000
+++ b/mysql-test/suite/backup/r/backup_errors_compression.result 2009-11-17 19:43:39 +0000
@@ -595,6 +595,37 @@ SET SESSION DEBUG='-d';
#
# Test error handling by injecting errors into BACKUP
#
+# non-existent database
+DROP DATABASE IF EXISTS db1;
+DROP DATABASE IF EXISTS db2;
+CREATE DATABASE db1;
+USE db1;
+CREATE TABLE db1_tab (i int);
+INSERT INTO db1_tab VALUES (1), (2), (3);
+CREATE DATABASE db2;
+USE db2;
+CREATE TABLE db2_tab (v varchar(64));
+INSERT INTO db2_tab VALUES ("This is table db2_tab");
+#
+# Backup without error injection
+#
+SET SESSION DEBUG='-d';
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+backup_id
+#
+SHOW WARNINGS;
+Level Code Message
+#
+# Error injection in user privileges checking
+#
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_db';
+BACKUP DATABASE db1 TO 'db1.bak' OVERWRITE;
+ERROR HY000: Failed to perform access privileges check for user 'root'.
+SET SESSION DEBUG='-d';
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_all_dbs';
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+ERROR HY000: Failed to perform access privileges check for user 'root'.
+SET SESSION DEBUG='-d';
#
# Error injection in si_object.cc:get_database_stub
#
@@ -606,3 +637,4 @@ SET SESSION DEBUG='-d';
# Cleanup
#
DROP DATABASE db1;
+DROP DATABASE db2;
=== modified file 'mysql-test/suite/backup/t/backup_errors.test'
--- a/mysql-test/suite/backup/t/backup_errors.test 2009-10-21 12:15:53 +0000
+++ b/mysql-test/suite/backup/t/backup_errors.test 2009-11-17 19:43:39 +0000
@@ -697,6 +697,45 @@ SET SESSION DEBUG='-d';
--echo # Test error handling by injecting errors into BACKUP
--echo #
+--echo # non-existent database
+--disable_warnings
+DROP DATABASE IF EXISTS db1;
+DROP DATABASE IF EXISTS db2;
+--enable_warnings
+
+CREATE DATABASE db1;
+USE db1;
+CREATE TABLE db1_tab (i int);
+INSERT INTO db1_tab VALUES (1), (2), (3);
+
+CREATE DATABASE db2;
+USE db2;
+CREATE TABLE db2_tab (v varchar(64));
+INSERT INTO db2_tab VALUES ("This is table db2_tab");
+
+--echo #
+--echo # Backup without error injection
+--echo #
+SET SESSION DEBUG='-d';
+--replace_column 1 #
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+SHOW WARNINGS;
+
+--echo #
+--echo # Error injection in user privileges checking
+--echo #
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_db';
+
+--error ER_BACKUP_USER_PRIV_CHECK
+BACKUP DATABASE db1 TO 'db1.bak' OVERWRITE;
+SET SESSION DEBUG='-d';
+
+SET SESSION DEBUG='+d,ER_BACKUP_USER_PRIV_CHECK_add_all_dbs';
+
+--error ER_BACKUP_USER_PRIV_CHECK
+BACKUP DATABASE * TO 'all.bak' OVERWRITE;
+SET SESSION DEBUG='-d';
+
--echo #
--echo # Error injection in si_object.cc:get_database_stub
--echo #
@@ -710,6 +749,7 @@ SET SESSION DEBUG='-d';
--echo #
DROP DATABASE db1;
+DROP DATABASE db2;
--remove_file $MYSQLD_DATADIR/errorinject.bak
=== modified file 'mysql-test/suite/backup_engines/r/backup_interruption.result'
--- a/mysql-test/suite/backup_engines/r/backup_interruption.result 2009-10-09 13:03:56 +0000
+++ b/mysql-test/suite/backup_engines/r/backup_interruption.result 2009-11-17 19:43:39 +0000
@@ -196,8 +196,6 @@ SHOW WARNINGS;
Level Code Message
Error <error-code> Query execution was interrupted
Error <error-code> Query execution was interrupted
-Error <error-code> Query execution was interrupted
-Error <error-code> Query execution was interrupted
Warning <error-code> Operation aborted
#
# Examine backup logs.
@@ -414,8 +412,6 @@ SHOW WARNINGS;
Level Code Message
Error <error-code> Query execution was interrupted
Error <error-code> Query execution was interrupted
-Error <error-code> Query execution was interrupted
-Error <error-code> Query execution was interrupted
Warning <error-code> Operation aborted
#
# Examine backup logs.
=== modified file 'sql/backup/backup_info.cc'
--- a/sql/backup/backup_info.cc 2009-10-23 15:41:56 +0000
+++ b/sql/backup/backup_info.cc 2009-11-17 19:43:39 +0000
@@ -674,7 +674,18 @@ backup::Image_info::Db* Backup_info::add
}
// Check to see if the user can see all of the objects in the database.
- if (obs::check_user_access(m_thd, name))
+ bool has_access= FALSE;
+ bool got_error= obs::check_user_access(m_thd, name, &has_access);
+
+ DBUG_EXECUTE_IF("ER_BACKUP_USER_PRIV_CHECK_add_db", got_error= TRUE;);
+
+ if (got_error)
+ {
+ m_log.report_error(ER_BACKUP_USER_PRIV_CHECK, m_thd->security_ctx->user);
+ return NULL;
+ }
+
+ if (!has_access)
{
m_log.report_error(ER_BACKUP_ACCESS_OBJS_INCOMPLETE, name->ptr());
return NULL;
@@ -802,7 +813,18 @@ int Backup_info::add_all_dbs()
int res= 0;
// Check to see if the user can see all of the databases.
- if (check_user_access(m_thd, 0))
+ bool has_access= FALSE;
+ bool got_error= obs::check_user_access(m_thd, 0, &has_access);
+
+ DBUG_EXECUTE_IF("ER_BACKUP_USER_PRIV_CHECK_add_all_dbs", got_error= TRUE;);
+
+ if (got_error)
+ {
+ m_log.report_error(ER_BACKUP_USER_PRIV_CHECK, m_thd->security_ctx->user);
+ return ERROR;
+ }
+
+ if (!has_access)
{
m_log.report_error(ER_BACKUP_ACCESS_DBS_INCOMPLETE);
return ERROR;
=== modified file 'sql/backup/kernel.cc'
--- a/sql/backup/kernel.cc 2009-10-23 15:41:56 +0000
+++ b/sql/backup/kernel.cc 2009-11-17 19:43:39 +0000
@@ -249,7 +249,7 @@ execute_backup_command(THD *thd,
/* Error condition insertion for ER_BACKUP_BACKUP_PREPARE. */
DBUG_EXECUTE_IF("ER_BACKUP_BACKUP_PREPARE_2", res= 1;);
- if (res || !info->is_valid())
+ if (context.report_killed() || res || !info->is_valid())
DBUG_RETURN(send_error(context, ER_BACKUP_BACKUP_PREPARE));
int count= info->db_count();
@@ -347,7 +347,17 @@ execute_backup_command(THD *thd,
static
int send_error(Backup_restore_ctx &context, int error_code, ...)
{
- if (!context.is_killed() && !context.error_reported())
+ using namespace backup;
+
+ /* Atmost one error should be reported. */
+ if (context.error_reported())
+ return error_code;
+
+ /* Report error state if not already done. */
+ context.report_state(BUP_ERRORS);
+
+ /* If the query has been interrupted, do not print this message. */
+ if (!context.is_killed())
{
char buf[MYSQL_ERRMSG_SIZE];
va_list args;
=== modified file 'sql/backup/logger.h'
--- a/sql/backup/logger.h 2009-10-21 13:32:24 +0000
+++ b/sql/backup/logger.h 2009-11-17 19:43:39 +0000
@@ -263,7 +263,7 @@ void Logger::report_start(time_t when)
report_error(log_level::INFO, m_type == BACKUP ? ER_BACKUP_BACKUP_START
: ER_BACKUP_RESTORE_START);
backup_log->start(when);
- backup_log->state(BUP_RUNNING);
+ backup_log->set_state(BUP_RUNNING);
}
@@ -329,21 +329,23 @@ void Logger::report_aborted(time_t when,
/**
- Report change of the state of operation
+ Report current state of operation only when there is state change.
+ This can be called multiple times without generating duplicate messages.
For possible states see definition of @c enum_backup_state
@todo Consider reporting state changes in the server error log (as info
entries).
+
*/
inline
void Logger::report_state(enum_backup_state state)
{
- DBUG_ASSERT(m_state == RUNNING || m_state == READY);
- DBUG_ASSERT(backup_log);
-
- backup_log->state(state);
+ if (!backup_log)
+ return;
+ if (backup_log->get_state() != state)
+ backup_log->set_state(state);
}
@@ -443,7 +445,7 @@ int Logger::init(enum_type type, const c
m_type= type;
backup_log = new Backup_log(m_thd, (enum_backup_operation)type, query);
- backup_log->state(BUP_STARTING);
+ backup_log->set_state(BUP_STARTING);
m_state= READY;
DEBUG_SYNC(m_thd, "after_backup_log_init");
return 0;
=== modified file 'sql/share/errmsg-utf8.txt'
--- a/sql/share/errmsg-utf8.txt 2009-10-12 09:08:34 +0000
+++ b/sql/share/errmsg-utf8.txt 2009-11-17 19:43:39 +0000
@@ -6562,3 +6562,5 @@ ER_BACKUP_ACCESS_OBJS_INCOMPLETE
eng "Insufficient privileges. You do not have privileges to backup database '%s'."
ER_WARN_I_S_SKIPPED_TABLE
eng "Table '%s'.'%s' was skipped since its definition is being modified by concurrent DDL statement."
+ER_BACKUP_USER_PRIV_CHECK
+ eng "Failed to perform access privileges check for user '%s'."
=== modified file 'sql/share/errmsg.txt'
--- a/sql/share/errmsg.txt 2009-10-26 14:02:26 +0000
+++ b/sql/share/errmsg.txt 2009-11-17 19:43:39 +0000
@@ -6562,3 +6562,5 @@ ER_BACKUP_ACCESS_OBJS_INCOMPLETE
eng "Insufficient privileges. You do not have privileges to backup database '%s'."
ER_WARN_I_S_SKIPPED_TABLE
eng "Table '%s'.'%s' was skipped since its definition is being modified by concurrent DDL statement."
+ER_BACKUP_USER_PRIV_CHECK
+ eng "Failed to perform access privileges check for user '%s'."
=== modified file 'sql/si_logs.cc'
--- a/sql/si_logs.cc 2009-05-21 13:17:37 +0000
+++ b/sql/si_logs.cc 2009-11-17 19:43:39 +0000
@@ -162,13 +162,24 @@ bool Backup_log::write_master_binlog_inf
@todo Consider reporting state changes in the server error log (as info
entries).
*/
-void Backup_log::state(enum_backup_state state)
+void Backup_log::set_state(enum_backup_state state)
{
m_op_hist.state= state;
logger.backup_progress_log_write(m_thd, m_op_hist.backup_id, "backup kernel", 0,
0, 0, 0, 0, get_state_string(state));
}
+/**
+ Get current state of operation.
+
+ For possible states see definition of @c enum_backup_state
+ */
+enum_backup_state Backup_log::get_state()
+{
+ return m_op_hist.state;
+}
+
+
/**
Report validity point creation time.
=== modified file 'sql/si_logs.h'
--- a/sql/si_logs.h 2009-11-16 18:37:18 +0000
+++ b/sql/si_logs.h 2009-11-17 19:43:39 +0000
@@ -133,7 +133,8 @@ public:
the backup_history log.
*/
ulonglong get_backup_id() { return m_op_hist.backup_id; }
- void state(enum_backup_state);
+ void set_state(enum_backup_state);
+ enum_backup_state get_state();
void error_num(int code) { m_op_hist.error_num= code; }
void binlog_start_pos(unsigned long int pos)
{
=== modified file 'sql/si_objects.cc'
--- a/sql/si_objects.cc 2009-11-09 10:56:12 +0000
+++ b/sql/si_objects.cc 2009-11-17 19:43:39 +0000
@@ -1310,11 +1310,6 @@ Iterator *create_row_set_iterator(THD *t
if (run_service_interface_sql(thd, &ed_connection, query, TRUE))
/* Query failed with an error */
return NULL;
- else if(ed_connection.get_warn_count())
- /* Push warnings to BACKUP's error stack. Calling method will
- decide if the warnings are a problem later when serializing the
- object. */
- thd->warning_info->append_warnings(thd, ed_connection.get_warn_list());
DBUG_ASSERT(ed_connection.get_field_count());
@@ -2561,9 +2556,9 @@ Obj_iterator *get_databases(THD *thd)
@param[in] THD Current thread
- @return uint Number of databases.
+ @return int Number of databases. Return value of -1 indicates failure.
*/
-uint get_num_dbs(THD *thd)
+int get_num_dbs(THD *thd)
{
Ed_connection ed_connection(thd);
Ed_result_set *ed_result_set;
@@ -2578,10 +2573,7 @@ uint get_num_dbs(THD *thd)
if (run_service_interface_sql(thd, &ed_connection,
s_stream.lex_string(), TRUE))
/* Query failed with an error */
- return NULL;
- else if(ed_connection.get_warn_count())
- /* Push warnings to BACKUP's error stack. */
- thd->warning_info->append_warnings(thd, ed_connection.get_warn_list());
+ return -1;
DBUG_ASSERT(ed_connection.get_field_count());
@@ -2607,9 +2599,9 @@ uint get_num_dbs(THD *thd)
@param[in] THD Current thread
@param[in] String Database name.
- @return uint Number of objects in the database.
+ @return int Number of objects in the database. Value of -1 means failure.
*/
-uint get_num_objects(THD *thd, const String *db_name)
+int get_num_objects(THD *thd, const String *db_name)
{
Ed_connection ed_connection(thd);
Ed_result_set *ed_result_set;
@@ -2635,10 +2627,7 @@ uint get_num_objects(THD *thd, const Str
if (run_service_interface_sql(thd, &ed_connection,
s_stream.lex_string(), TRUE))
/* Query failed with an error */
- return NULL;
- else if(ed_connection.get_warn_count())
- /* Push warnings to BACKUP's error stack. */
- thd->warning_info->append_warnings(thd, ed_connection.get_warn_list());
+ return -1;
DBUG_ASSERT(ed_connection.get_field_count());
@@ -2650,10 +2639,13 @@ uint get_num_objects(THD *thd, const Str
}
///////////////////////////////////////////////////////////////////////////
-bool check_user_access(THD *thd, const String *db_name)
+bool check_user_access(THD *thd, const String *db_name, bool *has_access)
{
- uint elev_count;
- uint user_count;
+ int elev_count;
+ int user_count;
+
+ DBUG_ASSERT(has_access);
+ *has_access= false; /* Reset access status. */
/*
Get value from user context without elevation.
@@ -2663,6 +2655,9 @@ bool check_user_access(THD *thd, const S
else
user_count= get_num_objects(thd, db_name);
+ if (user_count < 0)
+ return TRUE; /* Return error status */
+
/*
Peform global elevation with SELECT to get value of
someone with global SELECT (e.g. root).
@@ -2682,10 +2677,13 @@ bool check_user_access(THD *thd, const S
thd->security_ctx->master_access= saved_master_access;
thd->security_ctx->db_access= saved_db_access;
- /*
- Compare results and return.
- */
- return !(user_count == elev_count);
+ if (elev_count < 0)
+ return TRUE; /* Return error status */
+
+ /* User has proper access if both counts are same. */
+ *has_access= (user_count == elev_count);
+
+ return FALSE;
}
///////////////////////////////////////////////////////////////////////////
=== modified file 'sql/si_objects.h'
--- a/sql/si_objects.h 2009-11-04 14:18:21 +0000
+++ b/sql/si_objects.h 2009-11-17 19:43:39 +0000
@@ -185,18 +185,17 @@ Obj_iterator *get_databases(THD *thd);
2) If db_name != 0, the method shall check the user's visibility for all
objects in the database specified.
- @param[in] THD The current thread.
- @param[in] String The database name to check or 0
-
- @returns Information whether user can see all specified objects
- @retval TRUE
- if db_name != 0 : user does not have visibility to the database specified
- if db_name == 0 : user does not have visibility to all databases
- @retval FALSE
- if db_name != 0 : user does have visibility to the database specified
- if db_name == 0 : user does have visibility to all databases
+ @param[in] THD the current thread
+ @param[in] String the database name to check
+ @param[out] bool set to true/false as described below:
+ TRUE : User has the required access permission
+ FALSE : User does not have the required access permission
+
+ @return boolean value which indicates if an error has occured.
+ @retval TRUE indicates error
+ @retval FALSE indicates success
*/
-bool check_user_access(THD *thd, const String *db_name);
+bool check_user_access(THD *thd, const String *db_name, bool *has_access);
/**
Create an iterator over all base tables in a particular database.
Attachment: [text/bzr-bundle] bzr/thavamuni.alagu@sun.com-20091117194339-70kztwm8iydgooc0.bundle
| Thread |
|---|
| • bzr commit into mysql-6.0-backup branch (thavamuni.alagu:2891) Bug#47994 | thavamuni.alagu | 17 Nov |
