List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:October 27 2008 3:31pm
Subject:Re: bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364
View as plain text  
STATUS
======
Changes requested, patch not approved.

([x] refers to inline comments in the patch)

REQUESTS
========

1. With this patch, test sp-code fails for me on Solaris x86 (I have not
    tested other platforms, yet).  I get the following error diff:

+++ 
/export/home/tmp/oysteing/mysql/mysql-6.0-backup-review/mysql-test/r/sp-code.reject 
Mon Oct 27 15:45:08 2008
@@ -155,11 +155,11 @@
  0      stmt 9 "drop temporary table if exists sudoku..."
  1      stmt 1 "create temporary table sudoku_work ( ..."
  2      stmt 1 "create temporary table sudoku_schedul..."
-3      stmt 89 "call sudoku_init()"
+3      stmt 90 "call sudoku_init()"
  4      jump_if_not 7(8) p_naive@0
  5      stmt 4 "update sudoku_work set cnt = 0 where ..."
  6      jump 8
-7      stmt 89 "call sudoku_count()"
+7      stmt 90 "call sudoku_count()"
  8      stmt 6 "insert into sudoku_schedule (row,col)..."
  9      set v_scounter@2 0
  10     set v_i@3 1

2. Is it really necessary to require bin logging for this test to be
    run?  If not, I think it should not be required since it limits the
    number of configurations for which the test is run.

3. For the same reason as above, I do not see why requiring InnoDb
    support is necessary.  The tested functionality seems to be
    unrelated to which storage engine used.

4. The new test backup_logs_purge generates large amounts of output in
    master.err due to turning on tracing.  Is it not possible to turn
    on and off the debug flags without turning on tracing?  (I think
    you can use SET SESSION debug="+d,..." and "-d,...")

5. Fix typos [1], [2], [3], [11]

6. I think we should have tests that also test commands that will
    delete either none or all the entries in log tables.

7. I think we need a test that verify that content of files are
    actually purged.

8. scan_backup_log: I think the function name should reflect that log
    entries are actually deleted, and not just scanned.  Also, the
    documentation does not reflect the actual implementation since it
    is not required that datetime_val=0 for entries to be deleted by
    id. (In fact, this method is called with both backup_id and
    datatime_val set, see [4]).  I guess the easiest is to change the
    doc here.  Documentation also seems to imply that it is possible to
    use this method to delete by timestamp from process table, but I am
    not convinced that will work.  (Since ET_OBH_FIELD_START_TIME is
    used to find time field).

9. scan_backup_log, error handling: The call to ha_rnd_end()
    overwrites the result value from the original error.  Don't you
    risk ignoring errors here?

10. Do we really a specific error message for datetime errors for this
     command?  Please, consider whether there is a more generic message
     that can be used for such issues.

11. Comment [9] seems to be misplaced.  Please, move or remove.

12. Please explain the check for backupdir [10].


SUGGESTIONS
===========

1. I am not sure I agree that it should give errors to purge backup
    logs to id/table if logging to table is not enabled.  I suggest
    just ignoring the command since tables are empty anyway.

2. scan_backup_log: I think this implementation is a bit confusing
    since deletion by id and time is handled differently, one is
    cascading into the progress table the other is not.  I think you
    are trying to do too much with a single function, and I would
    suggest you use separate methods for these purposes.  If you are
    concerned about duplicating code, you could make a method for
    sharing the table_list setup.  (However, duplicating code seems to
    be the "name of the game" in log.cc.)

3. Log_to_csv_event_handler::purge_backup_logs_before_id: See [5], [6].

4. Log_to_csv_event_handler::purge_backup_logs_before_date: It is a
    bit unclear whether the Note in doc header is an interface note or
    an implementation spec.  From an interface point of view the
    cascading part is of no interest, purge_backup_logs_before_id and
    purge_backup_logs_before_date behaves the same way.  Both deletes
    entries in history and progress table.  Hence, I do not understand
    why one of them need a specific note.

5. Error messages:  Please consider comments [7], [8].

6. Please, consider adding an HA_EXTRA_DISALLOW_LOG_DELETE option too.
    I think it would be easier to follow if this LOG_DELETE was
    completly independent of MARK_AS_LOG_TABLE.  It seems strange to
    mark a table as a log table several times, just in order to get the
    side effect of disallowing deletes.


Chuck Bell wrote:
 > #At file:///C:/source/bzr/mysql-6.0-bug-33364/
 >
 >  2717 Chuck Bell	2008-10-24
 >       BUG#33364 Online Backup: Purge statement missing
 >
 >       This patch adds the following commands:
 >
 >       PURGE BACKUP LOGS;
 >       PURGE BACKUP LOGS TO <id>;
 >       PURGE BACKUP LOGS BEFORE <datetime>;
 > added:
 >   mysql-test/suite/backup/r/backup_logs_purge.result
 >   mysql-test/suite/backup/t/backup_logs_purge.test
 > modified:
 >   include/my_base.h
 >   sql/backup/kernel.cc
 >   sql/log.cc
 >   sql/log.h
 >   sql/mysqld.cc
 >   sql/share/errmsg.txt
 >   sql/sql_lex.h
 >   sql/sql_parse.cc
 >   sql/sql_yacc.yy
 >   storage/csv/ha_tina.cc
 >   storage/csv/ha_tina.h
 >
 > per-file messages:
 >   include/my_base.h
 >     Added property to allow delete from logs (table only).
 >   mysql-test/suite/backup/r/backup_logs_purge.result
 >     New result file.
 >   mysql-test/suite/backup/t/backup_logs_purge.test
 >     New test for purge commands.
 >   sql/backup/kernel.cc
 >     Added debug sync point for test.
 >   sql/log.cc
 >     Added methods to allow purging of backup logs.
 >   sql/log.h
 >     Added method declarations to allow purging of backup logs.
 >   sql/mysqld.cc
 >     Added new command enums for 'the big switch'.
 >   sql/share/errmsg.txt
 >     New error messages.
 >   sql/sql_lex.h
 >     Added new command enums for 'the big switch'.
 >   sql/sql_parse.cc
 >     Added code to execute the PURGE BACKUP commands.
 >   sql/sql_yacc.yy
 >     Added new commands to parser.
 >   storage/csv/ha_tina.cc
 >     Added code to allow delete from log tables.
 >   storage/csv/ha_tina.h
 >     Added new attribute to allow delete from logs.
 > === modified file 'include/my_base.h'
 > --- a/include/my_base.h	2008-07-11 13:36:54 +0000
 > +++ b/include/my_base.h	2008-10-24 21:01:03 +0000
 > @@ -206,7 +206,8 @@ enum ha_extra_function {
 >    HA_EXTRA_IS_ATTACHED_CHILDREN,
 >    HA_EXTRA_DETACH_CHILDREN,
 >    HA_EXTRA_ORDERBY_LIMIT,
 > -  HA_EXTRA_NO_ORDERBY_LIMIT
 > +  HA_EXTRA_NO_ORDERBY_LIMIT,
 > +  HA_EXTRA_ALLOW_LOG_DELETE
 >  };
 >
 >  /* Compatible option, to be deleted in 6.0 */
 >
 > === added file 'mysql-test/suite/backup/r/backup_logs_purge.result'
 > --- a/mysql-test/suite/backup/r/backup_logs_purge.result	1970-01-01 
00:00:00 +0000
 > +++ b/mysql-test/suite/backup/r/backup_logs_purge.result	2008-10-24 
21:01:03 +0000
 > @@ -0,0 +1,342 @@
 > +SET @@global.log_backup_output = 'FILE,TABLE';
 > +PURGE BACKUP LOGS;
 > +
 > +Now starting real tests
 > +
 > +DROP DATABASE IF EXISTS backup_logs;
 > +CREATE DATABASE backup_logs;
 > +Create table and populate with data.
 > +CREATE TABLE backup_logs.t1 (a char(30)) ENGINE=MYISAM;
 > +CREATE TABLE backup_logs.t2 (a char(30)) ENGINE=INNODB;
 > +CREATE TABLE backup_logs.t3 (a char(30)) ENGINE=MEMORY;
 > +INSERT INTO backup_logs.t1 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("04 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("05 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("06 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("07 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("04 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("05 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("06 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("04 Test #1 - progress");
 > +SET SESSION debug="d,set_backup_id";
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +backup_id
 > +500
 > +SET SESSION debug="d";
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +backup_id
 > +501
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +backup_id
 > +502
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +backup_id
 > +503
 > +Do restore of database
 > +RESTORE from 'backup1.bak';
 > +backup_id
 > +504
 > +Do restore of database
 > +RESTORE from 'backup2.bak';
 > +backup_id
 > +505
 > +Do restore of database
 > +RESTORE from 'backup3.bak';
 > +backup_id
 > +506
 > +Do restore of database
 > +RESTORE from 'backup4.bak';
 > +backup_id
 > +507
 > +Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +backup_id
 > +500
 > +501
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +backup_id
 > +500
 > +501
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +8
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +36
 > +Purge all backup log data prior to id = 504.
 > +PURGE BACKUP LOGS TO 502;
 > +Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +backup_id
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +backup_id
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +6
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +24
 > +Purge all backup log data.
 > +PURGE BACKUP LOGS;
 > +Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +backup_id
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +backup_id
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +0
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +0
 > +Cleanup the logs and images for later testing.
 > +PURGE BACKUP LOGS;
 > +SET @@time_zone = '+00:00';
 > +SET SESSION debug="d,set_backup_id";
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +backup_id
 > +500
 > +SET SESSION debug="d";
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +backup_id
 > +501
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +backup_id
 > +502
 > +SET SESSION debug="d,set_log_time";
 > +Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +backup_id
 > +503
 > +SET SESSION debug="d";
 > +Do restore of database
 > +RESTORE from 'backup1.bak';
 > +backup_id
 > +504
 > +Do restore of database
 > +RESTORE from 'backup2.bak';
 > +backup_id
 > +505
 > +Do restore of database
 > +RESTORE from 'backup3.bak';
 > +backup_id
 > +506
 > +SET SESSION debug="d,set_log_time";
 > +Do restore of database
 > +RESTORE from 'backup4.bak';
 > +backup_id
 > +507
 > +SET SESSION debug="d";
 > +Display the results.
 > +SELECT backup_id FROM mysql.backup_history;
 > +backup_id
 > +500
 > +501
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +backup_id
 > +500
 > +501
 > +502
 > +503
 > +504
 > +505
 > +506
 > +507
 > +SET @now_time= now();
 > +PURGE BACKUP LOGS BEFORE @now_time;
 > +SELECT backup_id FROM mysql.backup_history;
 > +backup_id
 > +503
 > +507
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +backup_id
 > +503
 > +507
 > +SET @@time_zone = @@global.time_zone;
 > +PURGE BACKUP LOGS;
 > +Perform backup
 > +SET SESSION debug="d,set_backup_id";
 > +BACKUP DATABASE backup_logs TO '../bup_logs_dir.bak';
 > +backup_id
 > +500
 > +SET SESSION debug="d";
 > +Ensure backup image file went to the correct location
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +1
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +6
 > +PURGE BACKUP LOGS TO 501;
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +0
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +0
 > +PURGE BACKUP LOGS BEFORE 123123;
 > +ERROR HY000: The datetime specified is invalid for the 'PURGE BACKUP 
LOGS BEFORE' command.
 > +SET @@global.log_backup_output = 'FILE';
 > +PURGE BACKUP LOGS TO 99999999;
 > +ERROR HY000: The operation could not be completed because the 
log_backup_output option does not include writing to tables.
 > +PURGE BACKUP LOGS BEFORE @now_time;
 > +ERROR HY000: The operation could not be completed because the 
log_backup_output option does not include writing to tables.
 > +SET @@global.log_backup_output = 'TABLE';
 > +Test backup and purge concurrent execution.
 > +PURGE BACKUP LOGS;
 > +Get rid of any lingering image files.
 > +SET DEBUG_SYNC= 'RESET';
 > +con2: Do some backups to add entries in logs.
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +backup_id
 > +#
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +backup_id
 > +#
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +backup_id
 > +#
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +backup_id
 > +#
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +4
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +24
 > +SET SESSION debug="d,set_backup_id";
 > +con2: Activate sync points for the backup statement.
 > +SET DEBUG_SYNC= 'before_backup_done SIGNAL ready WAIT_FOR proceed';
 > +BACKUP DATABASE backup_logs to 'backup5.bak';
 > +con1: Wait for the backup to be ready.
 > +SET DEBUG_SYNC= 'now WAIT_FOR ready';
 > +PURGE BACKUP LOGS;
 > +SET DEBUG_SYNC= 'now SIGNAL proceed';
 > +backup_id
 > +#
 > +SET SESSION debug="d";
 > +There should be one row in this table: the backup id from last
 > +backup (500).
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +1
 > +SELECT backup_id, command FROM mysql.backup_history;
 > +backup_id	command
 > +500	BACKUP DATABASE backup_logs to 'backup5.bak'
 > +There should be one row in this table: the backup id from last
 > +backup (500). We should only see the complete progress
 > +statement because all others were deleted while backup was
 > +in progress.
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +1
 > +SELECT * FROM mysql.backup_progress;
 > +backup_id	500
 > +object	backup kernel
 > +start_time	#
 > +stop_time	#
 > +total_bytes	0
 > +progress	0
 > +error_num	0
 > +notes	complete
 > +Now do the same test for restore.
 > +RESTORE FROM 'backup1.bak';
 > +backup_id
 > +#
 > +RESTORE FROM 'backup2.bak';
 > +backup_id
 > +#
 > +RESTORE FROM 'backup3.bak';
 > +backup_id
 > +#
 > +RESTORE FROM 'backup4.bak';
 > +backup_id
 > +#
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +5
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +13
 > +con2: Activate sync points for the backup statement.
 > +SET DEBUG_SYNC= 'before_restore_done SIGNAL ready WAIT_FOR proceed';
 > +RESTORE FROM 'backup5.bak';
 > +con1: Wait for the backup to be ready.
 > +SET DEBUG_SYNC= 'now WAIT_FOR ready';
 > +PURGE BACKUP LOGS;
 > +SET DEBUG_SYNC= 'now SIGNAL proceed';
 > +backup_id
 > +#
 > +There should be one row in this table: the backup id from last
 > +backup (505).
 > +SELECT count(*) FROM mysql.backup_history;
 > +count(*)
 > +1
 > +SELECT backup_id, command FROM mysql.backup_history;
 > +backup_id	command
 > +505	RESTORE FROM 'backup5.bak'
 > +There should be one row in this table: the backup id from last
 > +restore (505). We should only see the complete progress
 > +statement because all others were deleted while restore was
 > +in progress.
 > +SELECT count(*) FROM mysql.backup_progress;
 > +count(*)
 > +1
 > +SELECT * FROM mysql.backup_progress;
 > +backup_id	505
 > +object	backup kernel
 > +start_time	#
 > +stop_time	#
 > +total_bytes	0
 > +progress	0
 > +error_num	0
 > +notes	complete
 > +SET DEBUG_SYNC= 'RESET';
 > +DROP DATABASE backup_logs;
 > +PURGE BACKUP LOGS;
 >
 > === added file 'mysql-test/suite/backup/t/backup_logs_purge.test'
 > --- a/mysql-test/suite/backup/t/backup_logs_purge.test	1970-01-01 
00:00:00 +0000
 > +++ b/mysql-test/suite/backup/t/backup_logs_purge.test	2008-10-24 
21:01:03 +0000
 > @@ -0,0 +1,383 @@
 > +#
 > +# This test includes tests for ensuring the backup logs can be purged
 > +# of data.
 > +#
 > +
 > +--source include/have_log_bin.inc
 > +--source include/have_innodb.inc
 > +--source include/not_embedded.inc
 > +--source include/have_debug_sync.inc
 > +--source include/have_debug.inc
 > +
 > +connect (con1,localhost,root,,);
 > +connect (con2,localhost,root,,);
 > +
 > +connection con1;
 > +
 > +SET @@global.log_backup_output = 'FILE,TABLE';
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +--echo
 > +--echo Now starting real tests
 > +--echo
 > +
 > +# Create a database and some data to work with.
 > +
 > +--disable_warnings
 > +DROP DATABASE IF EXISTS backup_logs;
 > +--enable warnings
 > +CREATE DATABASE backup_logs;
 > +
 > +--echo Create table and populate with data.
 > +
 > +CREATE TABLE backup_logs.t1 (a char(30)) ENGINE=MYISAM;
 > +CREATE TABLE backup_logs.t2 (a char(30)) ENGINE=INNODB;
 > +CREATE TABLE backup_logs.t3 (a char(30)) ENGINE=MEMORY;
 > +
 > +INSERT INTO backup_logs.t1 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("04 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("05 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("06 Test #1 - progress");
 > +INSERT INTO backup_logs.t1 VALUES ("07 Test #1 - progress");
 > +
 > +INSERT INTO backup_logs.t2 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("04 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("05 Test #1 - progress");
 > +INSERT INTO backup_logs.t2 VALUES ("06 Test #1 - progress");
 > +
 > +INSERT INTO backup_logs.t3 VALUES ("01 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("02 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("03 Test #1 - progress");
 > +INSERT INTO backup_logs.t3 VALUES ("04 Test #1 - progress");
 > +
 > +#
 > +# Now test read of backupid with known id using debug insertion
 > +#
 > +SET SESSION debug="d,set_backup_id";
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +
 > +SET SESSION debug="d";
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup1.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup2.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup3.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup4.bak';
 > +
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup1.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup2.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup3.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup4.bak
 > +
 > +--echo Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +--echo Purge all backup log data prior to id = 504.
 > +PURGE BACKUP LOGS TO 502;

[1] Typo in comment. (504)

 > +
 > +--echo Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +--echo Purge all backup log data.
 > +PURGE BACKUP LOGS;
 > +
 > +--echo Display results from backup logs
 > +SELECT backup_id FROM mysql.backup_history;
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +--echo Cleanup the logs and images for later testing.
 > +PURGE BACKUP LOGS;
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup1.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup2.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup3.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup4.bak
 > +
 > +SET @@time_zone = '+00:00';
 > +
 > +SET SESSION debug="d,set_backup_id";
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +
 > +SET SESSION debug="d";
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +
 > +SET SESSION debug="d,set_log_time";
 > +
 > +--echo Do backup of database
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +
 > +SET SESSION debug="d";
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup1.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup2.bak';
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup3.bak';
 > +
 > +SET SESSION debug="d,set_log_time";
 > +
 > +--echo Do restore of database
 > +RESTORE from 'backup4.bak';
 > +
 > +SET SESSION debug="d";
 > +
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup1.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup2.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup3.bak
 > +--file_exists $MYSQLTEST_VARDIR/master-data/backup4.bak
 > +
 > +--echo Display the results.
 > +SELECT backup_id FROM mysql.backup_history;
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +
 > +#
 > +# We need to sleep to allow time to pass in order to ensure the time
 > +# compare method in log.cc has two different times to compare.
 > +#
 > +real_sleep 3;
 > +
 > +SET @now_time= now();
 > +
 > +PURGE BACKUP LOGS BEFORE @now_time;
 > +
 > +SELECT backup_id FROM mysql.backup_history;
 > +SELECT DISTINCT backup_id FROM mysql.backup_progress;
 > +
 > +SET @@time_zone = @@global.time_zone;
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +#
 > +# Test purge of logs for locations other than backupdir.
 > +#
 > +--echo Perform backup
 > +SET SESSION debug="d,set_backup_id";
 > +
 > +BACKUP DATABASE backup_logs TO '../bup_logs_dir.bak';
 > +
 > +SET SESSION debug="d";
 > +
 > +--echo Ensure backup image file went to the correct location
 > +--file_exists $MYSQLTEST_VARDIR/bup_logs_dir.bak
 > +
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +PURGE BACKUP LOGS TO 501;
 > +
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +#
 > +# Test error conditions.
 > +#
 > +
 > +--error ER_BACKUP_PURGE_DATETIME
 > +PURGE BACKUP LOGS BEFORE 123123;
 > +
 > +SET @@global.log_backup_output = 'FILE';
 > +
 > +--error ER_BACKUP_LOG_OUTPUT
 > +PURGE BACKUP LOGS TO 99999999;
 > +
 > +--error ER_BACKUP_LOG_OUTPUT
 > +PURGE BACKUP LOGS BEFORE @now_time;
 > +
 > +SET @@global.log_backup_output = 'TABLE';
 > +
 > +#
 > +# Now test what happens when PURGE is run at the same
 > +# time as a backup or restore is run.
 > +#
 > +
 > +--echo Test backup and purge concurrent execution.
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +--echo Get rid of any lingering image files.
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup1.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup2.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup3.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup4.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/bup_logs_dir.bak
 > +
 > +SET DEBUG_SYNC= 'RESET';
 > +
 > +connection con1;
 > +
 > +--echo con2: Do some backups to add entries in logs.
 > +--replace_column 1 #
 > +BACKUP DATABASE backup_logs to 'backup1.bak';
 > +--replace_column 1 #
 > +BACKUP DATABASE backup_logs to 'backup2.bak';
 > +--replace_column 1 #
 > +BACKUP DATABASE backup_logs to 'backup3.bak';
 > +--replace_column 1 #
 > +BACKUP DATABASE backup_logs to 'backup4.bak';
 > +
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +connection con2;
 > +
 > +SET SESSION debug="d,set_backup_id";
 > +
 > +#
 > +# Stop backup after it has written rows to the logs but before
 > +# it writes the history log and last complete log entry.
 > +#
 > +--echo con2: Activate sync points for the backup statement.
 > +SET DEBUG_SYNC= 'before_backup_done SIGNAL ready WAIT_FOR proceed';
 > +SEND BACKUP DATABASE backup_logs to 'backup5.bak';
 > +
 > +connection con1;
 > +
 > +--echo con1: Wait for the backup to be ready.
 > +SET DEBUG_SYNC= 'now WAIT_FOR ready';
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +SET DEBUG_SYNC= 'now SIGNAL proceed';
 > +
 > +connection con2;
 > +--replace_column 1 #
 > +reap;
 > +
 > +SET SESSION debug="d";
 > +
 > +connection con1;
 > +
 > +--echo There should be one row in this table: the backup id from last
 > +--echo backup (500).
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT backup_id, command FROM mysql.backup_history;
 > +
 > +--echo There should be one row in this table: the backup id from last
 > +--echo backup (500). We should only see the complete progress
 > +--echo statement because all others were deleted while backup was
 > +--echo in progress.
 > +SELECT count(*) FROM mysql.backup_progress;
 > +--replace_column 3 # 4 #
 > +--query_vertical SELECT * FROM mysql.backup_progress
 > +
 > +--echo Now do the same test for restore.
 > +
 > +--replace_column 1 #
 > +RESTORE FROM 'backup1.bak';
 > +--replace_column 1 #
 > +RESTORE FROM 'backup2.bak';
 > +--replace_column 1 #
 > +RESTORE FROM 'backup3.bak';
 > +--replace_column 1 #
 > +RESTORE FROM 'backup4.bak';
 > +
 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT count(*) FROM mysql.backup_progress;
 > +
 > +connection con2;
 > +
 > +#
 > +# Stop restore after it has written rows to the logs but before
 > +# it writes the history log and last complete log entry.
 > +#
 > +--echo con2: Activate sync points for the backup statement.
 > +SET DEBUG_SYNC= 'before_restore_done SIGNAL ready WAIT_FOR proceed';
 > +SEND RESTORE FROM 'backup5.bak';
 > +
 > +connection con1;
 > +
 > +--echo con1: Wait for the backup to be ready.
 > +SET DEBUG_SYNC= 'now WAIT_FOR ready';
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +SET DEBUG_SYNC= 'now SIGNAL proceed';
 > +
 > +connection con2;
 > +--replace_column 1 #
 > +reap;
 > +
 > +connection con1;
 > +
 > +--echo There should be one row in this table: the backup id from last
 > +--echo backup (505).

[2] Typo. Should be restore, not backup.

 > +SELECT count(*) FROM mysql.backup_history;
 > +SELECT backup_id, command FROM mysql.backup_history;
 > +
 > +--echo There should be one row in this table: the backup id from last
 > +--echo restore (505). We should only see the complete progress
 > +--echo statement because all others were deleted while restore was
 > +--echo in progress.
 > +SELECT count(*) FROM mysql.backup_progress;
 > +--replace_column 3 # 4 #
 > +--query_vertical SELECT * FROM mysql.backup_progress
 > +
 > +#
 > +# Cleanup.
 > +#
 > +
 > +SET DEBUG_SYNC= 'RESET';
 > +
 > +DROP DATABASE backup_logs;
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup1.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup2.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup3.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup4.bak
 > +--error 0,1
 > +--remove_file $MYSQLTEST_VARDIR/master-data/backup5.bak
 > +
 > +PURGE BACKUP LOGS;
 > +
 > +
 >
 > === modified file 'sql/backup/kernel.cc'
 > --- a/sql/backup/kernel.cc	2008-10-21 16:35:31 +0000
 > +++ b/sql/backup/kernel.cc	2008-10-24 21:01:03 +0000
 > @@ -1243,6 +1243,8 @@ int Backup_restore_ctx::do_restore()
 >
 >    report_stats_post(info);                      // Never errors
 >
 > +  DEBUG_SYNC(m_thd, "before_restore_done");
 > +
 >    DBUG_RETURN(0);
 >  }
 >
 >
 > === modified file 'sql/log.cc'
 > --- a/sql/log.cc	2008-10-21 10:44:29 +0000
 > +++ b/sql/log.cc	2008-10-24 21:01:03 +0000
 > @@ -856,6 +856,11 @@ bool Log_to_csv_event_handler::
 >    if (history_data->start)
 >    {
 >      MYSQL_TIME time;
 > +    /*
 > +      Set time ahead a few hours to allow backup purge test to test
 > +      PURGE BACKUP LOGS BEFORE command.
 > +    */
 > +    DBUG_EXECUTE_IF("set_log_time", history_data->start= my_time(0) 
+ 100000;);
 >      my_tz_OFFSET0->gmt_sec_to_TIME(&time, 
(my_time_t)history_data->start);
 >
 >      table->field[ET_OBH_FIELD_START_TIME]->set_notnull();
 > @@ -1096,6 +1101,319 @@ err:
 >    return result;
 >  }
 >
 > +/**
 > +  Delete the current row from a log table.
 > +
 > +  This method is used to delete a row from a log that is being
 > +  accessed as a table. The row to be deleted is the current
 > +  row that the handler is pointing to via tbl->record[0].
 > +
 > +  Note: The handler must be positioned correctly and the
 > +        tbl->record[0] must be fetched by the appropriate
 > +        method (e.g., hdl->next()). T
 > +
 > +  @param[IN]  hdl   Table handler.
 > +  @param[IN]  tbl   Table class.
 > +
 > +  @returns Result of delete operation
 > +*/
 > +static bool delete_current_log_row(handler *hdl, TABLE *tbl)
 > +{
 > +  bool result= 0;
 > +
 > +  /*
 > +    Tell the handler to allow deletes for this log table
 > +    by turning off log table flag.
 > +  */
 > +  hdl->extra(HA_EXTRA_ALLOW_LOG_DELETE);
 > +  result= hdl->ha_delete_row(tbl->record[0]);
 > +  hdl->extra(HA_EXTRA_MARK_AS_LOG_TABLE);
 > +  return result;
 > +}
 > +
 > +/**
 > +  Perform a table scan of the backup log tables for deleting rows.
 > +
 > +  This method is a helper method designed to delete rows from the
 > +  backup logs when the destination includes writing to tables.
 > +
 > +  The method is designed to work with either the backup_history or
 > +  backup_progress log. The log is specified using the @c log_name
 > +  parameter and show be either BACKUP_HISTORY_LOG_NAME or

[3] Typo. I guess you mean "should be either ..."

 > +  BACKUP_PROGRESS_LOG_NAME.
 > +
 > +  There are three ways this method can be used. The choice of which of
 > +  the operations to run depends on the values of the parameters as 
stated.
 > +  1. Delete rows whose backup_id is < a given value. This is used
 > +     for @c PURGE BACKUP LOGS TO <@c backup_id>.
 > +     To use this method, set @c backup_id to the value passed from the
 > +     command, @c delete_exact_row = FALSE, and @c datetime_val = 0.
 > +  2. Delete rows in the log where backuip_id is == to a given value.
 > +     This is used when cascading deletes from the backup_history log
 > +     to the backup_progress log.
 > +     To use this method, set @c backup_id to the value of the rows 
needing
 > +     deletion, @c delete_exact_row = TRUE, and @c datetime_val = 0.
 > +  3. Delete rows whose start_time < a given value. This is used for
 > +     @c PURGE BACKUP LOGS BEFORE <datetimeval>.
 > +     To use this method, set @c backup_id = 0, @c delete_exact_row = 
FALSE,
 > +     and @c datetime_val = value passed from the command.
 > +
 > +  @param[IN]  THD                 The current thread
 > +  @param[IN]  log_name            is used to determine which log to 
process
 > +                                  (backup_history or backup_progress).
 > +  @param[IN]  backup_id           value specified on the command for 
deleting
 > +                                  rows by id or value for exact match
 > +  @param[IN]  datetime_val        value specified on the command for 
deleting
 > +                                  rows by date
 > +  @param[IN]  delete_exact_row    if TRUE, use the comparison for 
the log
 > +                                  column backup_id_in_row == @c 
backup_id else
 > +                                  use the comparison backup_id_in_row <
 > +                                  @c backup_id.
 > +  @param[OUT] num                 Number of rows affected.
 > +
 > +  @returns TRUE if error
 > +*/
 > +static bool scan_backup_log(THD *thd,
 > +                            LEX_STRING log_name,
 > +                            ulonglong backup_id,
 > +                            my_time_t datetime_val,
 > +                            bool delete_exact_row,
 > +                            int *rows)
 > +{
 > +  TABLE_LIST table_list;
 > +  handler *hdl;
 > +  TABLE *tbl;
 > +  bool res= FALSE;
 > +  uint result= 0;
 > +  int num_rows= 0;
 > +  Open_tables_state open_tables_backup;
 > +  MYSQL_TIME tmp;
 > +
 > +  DBUG_ASSERT((my_strcasecmp(system_charset_info, log_name.str,
 > +               BACKUP_HISTORY_LOG_NAME.str) == 0) ||
 > +              (my_strcasecmp(system_charset_info, log_name.str,
 > +               BACKUP_PROGRESS_LOG_NAME.str) == 0));
 > +
 > +  /*
 > +    Setup table list for open.
 > +  */
 > +  bzero(&table_list, sizeof(TABLE_LIST));
 > +  table_list.alias= table_list.table_name= log_name.str;
 > +  table_list.table_name_length= log_name.length;
 > +
 > +  table_list.lock_type= TL_WRITE_CONCURRENT_INSERT;
 > +
 > +  table_list.db= MYSQL_SCHEMA_NAME.str;
 > +  table_list.db_length= MYSQL_SCHEMA_NAME.length;
 > +
 > +  tbl= open_performance_schema_table(thd, &table_list,
 > +                                       &open_tables_backup);
 > +  hdl= tbl->file;
 > +  hdl->extra(HA_EXTRA_MARK_AS_LOG_TABLE);
 > +
 > +  /*
 > +    Get time zone conversion of value to compare for
 > +    PURGE BACKUP LOGS BEFORE <datetime>.
 > +  */
 > +  bzero(&tmp, sizeof(MYSQL_TIME));
 > +  thd->variables.time_zone->gmt_sec_to_TIME(&tmp, datetime_val);
 > +
 > +  /*
 > +    Begin table scan.
 > +  */
 > +  result= hdl->ha_rnd_init(1);
 > +  result= hdl->rnd_next(tbl->record[0]);
 > +  while (result != HA_ERR_END_OF_FILE)
 > +  {
 > +    // backup_id is field number 0 in both logs.
 > +    ulonglong id= tbl->field[0]->val_int();
 > +    /*
 > +      Delete by id.
 > +    */
 > +    if (backup_id)
 > +    {
 > +      /*
 > +        Here we delete a specific row. For example, a specific
 > +        row in the backup_history log as the result of a
 > +        cascade delete initiated from backup_history log.
 > +      */
 > +      if (delete_exact_row)
 > +      {
 > +        if (id == backup_id) // delete log row
 > +        {
 > +          result= delete_current_log_row(hdl, tbl);
 > +          if (result)
 > +            goto error;
 > +        }
 > +      }
 > +      /*
 > +        Here we execute the delete all rows 'to' a specific
 > +        id, e.g. PURGE BACKUP LOGS TO 17 -- we delete id < 17.
 > +      */
 > +      else if (id < backup_id)
 > +      {
 > +        result= delete_current_log_row(hdl, tbl);
 > +        if (result)
 > +          goto error;
 > +        num_rows++;
 > +      }
 > +    }
 > +    /*
 > +      Delete by date
 > +    */
 > +    else
 > +    {
 > +      MYSQL_TIME time;
 > +
 > +      tbl->field[ET_OBH_FIELD_START_TIME]->get_date(&time, 
TIME_NO_ZERO_DATE);
 > +      if (my_time_compare(&time, &tmp) < 0)
 > +      {
 > +        /*
 > +          When deleting by date for the backup_history table,
 > +          we must also delete rows from the backup_progress table
 > +          for this backup id.
 > +        */
 > +        if ((my_strcasecmp(system_charset_info, log_name.str,
 > +             BACKUP_HISTORY_LOG_NAME.str) == 0) &&
 > +            opt_backup_progress_log &&
 > +            (log_backup_output_options & LOG_TABLE))
 > +        {
 > +          int r= 0;
 > +
 > +          scan_backup_log(thd, BACKUP_PROGRESS_LOG_NAME,
 > +                          id, datetime_val, TRUE, &r);

[4] This calls scan_backup_log with both id and datetime_val != 0.
     The behavior in this case is not specified in the documentation.

 > +        }
 > +        result= delete_current_log_row(hdl, tbl);
 > +        if (result)
 > +          goto error;
 > +        num_rows++;
 > +      }
 > +    }
 > +    result= hdl->rnd_next(tbl->record[0]);
 > +  }
 > +error:
 > +  result= hdl->ha_rnd_end();
 > +  *rows= num_rows;
 > +  close_performance_schema_table(thd, &open_tables_backup);
 > +  return result;
 > +}
 > +
 > +/**
 > +  Remove all rows from the backup logs.
 > +
 > +  This method removes (via truncate) all data from the backup logs. 
It checks
 > +  if the backup logs are turned on and if the 
log_backup_output_option is set
 > +  to include writing to tables. If these conditions are true, each log
 > +  (backup_history, backup_progress) is truncated.
 > +
 > +  @param[IN] THD  current thread
 > +
 > +  @returns TRUE = success, FALSE = failed
 > +*/
 > +bool Log_to_csv_event_handler::purge_backup_logs(THD *thd)
 > +{
 > +  TABLE_LIST tables;
 > +  bool res= FALSE;
 > +
 > +  if (opt_backup_history_log && (log_backup_output_options &
LOG_TABLE))
 > +  {
 > +    // need to truncate the table.
 > +    tables.init_one_table("mysql", strlen("mysql"),
 > +                          "backup_history", strlen("backup_history"),
 > +                          "backup_history", TL_READ);
 > +    alloc_mdl_locks(&tables, thd->mem_root);
 > +    res= mysql_truncate(thd, &tables, 1);
 > +    close_thread_tables(thd);
 > +    if (res)
 > +      goto err;
 > +  }
 > +  if (opt_backup_progress_log && (log_backup_output_options & 
LOG_TABLE))
 > +  {
 > +    // need to truncate the table.
 > +    tables.init_one_table("mysql", strlen("mysql"),
 > +                          "backup_progress", strlen("backup_progress"),
 > +                          "backup_progress", TL_READ);
 > +    alloc_mdl_locks(&tables, thd->mem_root);
 > +    res= mysql_truncate(thd, &tables, 1);
 > +    close_thread_tables(thd);
 > +  }
 > +err:
 > +  return res;
 > +}
 > +
 > +/**
 > +  Purge backup logs of rows less than backup_id passed.
 > +
 > +  This method walks the backup log tables deleting all rows whose
 > +  backup id is less than @c backup_id passed.
 > +
 > +  @param[IN]  THD                 The current thread
 > +  @param[IN]  backup_id           Value for backup id
 > +  @param[OUT] num                 Number of rows affected.
 > +
 > +  @retval num  Number of rows affected.
 > +
 > +  @returns TRUE if error
 > +*/
 > +bool
 > +Log_to_csv_event_handler::purge_backup_logs_before_id(THD *thd,
 > +                                                      ulonglong 
backup_id,
 > +                                                      int *rows)
 > +{
 > +  bool res= FALSE;
 > +  my_time_t t= 0;
 > +
 > +  if (opt_backup_history_log && (log_backup_output_options &
LOG_TABLE))
 > +  {
 > +    res= scan_backup_log(thd, BACKUP_HISTORY_LOG_NAME,
 > +                         backup_id, t, FALSE, rows);

[5] Personally, I think 0 would be more readable than t as a parameter
here.

 > +    if (res)
 > +      goto err;
 > +  }
 > +  if (opt_backup_progress_log && (log_backup_output_options & 
LOG_TABLE))
 > +  {
 > +    int r= 0;   //ignore this for progress log
 > +
 > +    res= scan_backup_log(thd, BACKUP_PROGRESS_LOG_NAME,
 > +                         backup_id, t, FALSE, &r);

[6] Ditto.

 > +  }
 > +err:
 > +  return res;
 > +}
 > +
 > +/**
 > +  Purge backup logs of rows previous to start date passed.
 > +
 > +  This method walks the backup log tables deleting all rows whose
 > +  start column value is less than @c t passed.
 > +
 > +  Note: This method uses cascading delete functionality to remove
 > +  rows from the backup_progress log when rows from the
 > +  backup_history log are removed. Thus, only one call to delete
 > +  rows from the logs is necessary.
 > +
 > +  @param[IN]  THD                 The current thread
 > +  @param[IN]  t                   Value for start datetime
 > +  @param[OUT] num                 Number of rows affected.
 > +
 > +  @retval num  Number of rows affected.
 > +
 > +  @returns TRUE if error
 > +*/
 > +bool
 > +Log_to_csv_event_handler::purge_backup_logs_before_date(THD *thd,
 > +                                                        my_time_t t,
 > +                                                        int *rows)
 > +{
 > +  bool res= FALSE;
 > +
 > +  if (opt_backup_history_log && (log_backup_output_options &
LOG_TABLE))
 > +    res= scan_backup_log(thd, BACKUP_HISTORY_LOG_NAME,
 > +                         0, t, FALSE, rows);
 > +  return res;
 > +}
 > +
 > +
 >  bool Log_to_csv_event_handler::
 >    log_error(enum loglevel level, const char *format, va_list args)
 >  {
 > @@ -1256,12 +1574,55 @@ void Log_to_file_event_handler::flush_ba
 >      reopen log files
 >
 >      Where TRUE means perform open on history file (backup_history) and
 > -    FALSE means perform open on the progress file (backkup_progress).
 > +    FALSE means perform open on the progress file (backup_progress).
 >    */
 >    mysql_backup_history_log.reopen_file(TRUE);
 >    mysql_backup_progress_log.reopen_file(FALSE);
 >  }
 >
 > +/**
 > +  Purge the backup logs of all data.
 > +
 > +  This method truncates the backup log files. It will only do so if the
 > +  log_backup_output_options includes logging to FILE. It also checks to
 > +  see if each log is turned on (backup_history and backup_progress) and
 > +  if so, truncates it. Truncate in this case means resetting the size
 > +  to 0 bytes using my_chsize().
 > +
 > +  @returns TRUE = success, FALSE = failed.
 > +*/
 > +bool Log_to_file_event_handler::purge_backup_logs()
 > +{
 > +  bool res= FALSE;
 > +
 > +  if (opt_backup_history_log && (log_backup_output_options & LOG_FILE))
 > +  {
 > +    MYSQL_BACKUP_LOG *backup_log= 
logger.get_backup_history_log_file_handler();
 > +
 > +    pthread_mutex_lock(backup_log->get_log_lock());
 > +    res= my_sync(backup_log->get_file(), MYF(MY_WME));
 > +    if (!res)
 > +      res= my_chsize(backup_log->get_file(), 0, 0, MYF(MY_WME));
 > +    pthread_mutex_unlock(backup_log->get_log_lock());
 > +    if (res)
 > +      goto err;
 > +  }
 > +  if (opt_backup_progress_log && (log_backup_output_options &
LOG_FILE))
 > +  {
 > +    MYSQL_BACKUP_LOG *backup_log= 
logger.get_backup_progress_log_file_handler();
 > +
 > +    pthread_mutex_lock(backup_log->get_log_lock());
 > +    res= my_sync(backup_log->get_file(), MYF(MY_WME));
 > +    if (!res)
 > +      res= my_chsize(backup_log->get_file(), 0, 0, MYF(MY_WME));
 > +    pthread_mutex_unlock(backup_log->get_log_lock());
 > +    if (res)
 > +      goto err;
 > +  }
 > +err:
 > +  return res;
 > +}
 > +
 >  void Log_to_file_event_handler::cleanup()
 >  {
 >    mysql_log.cleanup();
 > @@ -1411,6 +1772,91 @@ bool LOGGER::flush_backup_logs(THD *thd)
 >    return rc;
 >  }
 >
 > +/**
 > +  Delete contents of backup logs.
 > +
 > +  This method deletes the data from the backup logs. This applies to 
both
 > +  log file and table destinations.
 > +
 > +  @param[IN]  thd  The current thread.
 > +
 > +  @returns TRUE if error.
 > +*/
 > +bool LOGGER::purge_backup_logs(THD *thd)
 > +{
 > +  my_bool res= FALSE;
 > +
 > +  DBUG_ENTER("LOGGER::purge_backup_logs");
 > +  /*
 > +    Here we need to truncate the files if writing to files.
 > +  */
 > +  res= file_log_handler->purge_backup_logs();
 > +  if (res)
 > +    goto err;
 > +
 > +  /*
 > +    We also need to truncate the table if writing to tables.
 > +  */
 > +  res= table_log_handler->purge_backup_logs(thd);
 > +
 > +err:
 > +  DBUG_RETURN(res);
 > +}
 > +
 > +/**
 > +  Delete contents of backup logs where backup id is less than a 
given id.
 > +
 > +  This method deletes the data from the backup logs where a backup 
id is
 > +  less than the backup id specified. This applies only to table 
destinations.
 > +
 > +  @param[IN]  thd        The current thread.
 > +  @param[IN]  backup_id  The backup id to compare rows to.
 > +  @param[OUT] rows       The number of rows affected.
 > +
 > +  @retval  num  The number of rows affected.
 > +
 > +  @returns TRUE if error.
 > +*/
 > +bool LOGGER::purge_backup_logs_before_id(THD *thd,
 > +                                         ulonglong backup_id,
 > +                                         int *rows)
 > +{
 > +  my_bool res= FALSE;
 > +
 > +  DBUG_ENTER("LOGGER::purge_backup_logs_before_id");
 > +
 > +  res= table_log_handler->purge_backup_logs_before_id(thd, 
backup_id, rows);
 > +
 > +  DBUG_RETURN(res);
 > +}
 > +
 > +/**
 > +  Delete backup rows less than a certain date.
 > +
 > +  This method scans the backup history log and deletes the rows for 
those
 > +  operations that were created (start column) previous to the date 
specified in
 > +  @c t.
 > +
 > +  @param[IN]  thd        The current thread.
 > +  @param[IN]  t          The date to compare rows to.
 > +  @param[OUT] rows       The number of rows affected.
 > +
 > +  @retval  rows  The number of rows affected.
 > +
 > +  @returns TRUE if error.
 > +*/
 > +bool LOGGER::purge_backup_logs_before_date(THD *thd,
 > +                                           my_time_t t,
 > +                                           int *rows)
 > +{
 > +  my_bool res= FALSE;
 > +
 > +  DBUG_ENTER("LOGGER::purge_backup_logs_before_date");
 > +
 > +  res= table_log_handler->purge_backup_logs_before_date(thd, t, rows);
 > +
 > +  DBUG_RETURN(res);
 > +}
 >
 >  /*
 >    Log slow query with all enabled log event handlers
 >
 > === modified file 'sql/log.h'
 > --- a/sql/log.h	2008-10-21 10:44:29 +0000
 > +++ b/sql/log.h	2008-10-24 21:01:03 +0000
 > @@ -268,6 +268,14 @@ private:
 >    time_t last_time;
 >  };
 >
 > +/*
 > +  Values for the type enum. This reflects the order of the enum
 > +  declaration in the PURGE BACKUP LOGS command.
 > +*/
 > +#define TYPE_ENUM_PURGE_BACKUP_LOGS       1
 > +#define TYPE_ENUM_PURGE_BACKUP_LOGS_ID    2
 > +#define TYPE_ENUM_PURGE_BACKUP_LOGS_DATE  3
 > +
 >  class MYSQL_BACKUP_LOG: public MYSQL_LOG
 >  {
 >  public:
 > @@ -307,6 +315,8 @@ public:
 >    }
 >    ulonglong get_next_backup_id();
 >    my_bool check_backup_logs(THD *thd);
 > +  File get_file() { return log_file.file; }
 > +  inline pthread_mutex_t* get_log_lock() { return &LOCK_log; }
 >
 >  private:
 >    bool write_int(uint num);
 > @@ -577,6 +587,13 @@ public:
 >                                     longlong progress,
 >                                     int error_num,
 >                                     const char *notes);
 > +  bool purge_backup_logs(THD *thd);
 > +  bool purge_backup_logs_before_id(THD *thd,
 > +                                   ulonglong backup_id,
 > +                                   int *rows);
 > +  bool purge_backup_logs_before_date(THD *thd,
 > +                                     my_time_t t,
 > +                                     int *rows);
 >
 >    int activate_log(THD *thd, uint log_type);
 >
 > @@ -630,6 +647,7 @@ public:
 >
 >    void flush();
 >    void flush_backup_logs();
 > +  bool purge_backup_logs();
 >    void init_pthread_objects();
 >    MYSQL_QUERY_LOG *get_mysql_slow_log() { return &mysql_slow_log; }
 >    MYSQL_QUERY_LOG *get_mysql_log() { return &mysql_log; }
 > @@ -684,6 +702,13 @@ public:
 >    void init_log_tables();
 >    bool flush_logs(THD *thd);
 >    bool flush_backup_logs(THD *thd);
 > +  bool purge_backup_logs(THD *thd);
 > +  bool purge_backup_logs_before_id(THD *thd,
 > +                                   ulonglong backup_id,
 > +                                   int *rows);
 > +  bool purge_backup_logs_before_date(THD *thd,
 > +                                     my_time_t t,
 > +                                     int *rows);
 >    /* Perform basic logger cleanup. this will leave e.g. error log 
open. */
 >    void cleanup_base();
 >    /* Free memory. Nothing could be logged after this function is 
called */
 >
 > === modified file 'sql/mysqld.cc'
 > --- a/sql/mysqld.cc	2008-10-13 14:10:00 +0000
 > +++ b/sql/mysqld.cc	2008-10-24 21:01:03 +0000
 > @@ -3242,6 +3242,7 @@ SHOW_VAR com_status_vars[]= {
 >    {"prepare_sql",          (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_PREPARE]), SHOW_LONG_STATUS},
 >    {"purge",                (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_PURGE]), SHOW_LONG_STATUS},
 >    {"purge_before_date",    (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_PURGE_BEFORE]), SHOW_LONG_STATUS},
 > +  {"purge_bup_log",        (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_PURGE_BUP_LOG]), SHOW_LONG_STATUS},
 >    {"release_savepoint",    (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_RELEASE_SAVEPOINT]), SHOW_LONG_STATUS},
 >    {"rename_table",         (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_RENAME_TABLE]), SHOW_LONG_STATUS},
 >    {"rename_user",          (char*) offsetof(STATUS_VAR, 
com_stat[(uint) SQLCOM_RENAME_USER]), SHOW_LONG_STATUS},
 >
 > === modified file 'sql/share/errmsg.txt'
 > --- a/sql/share/errmsg.txt	2008-10-15 06:48:36 +0000
 > +++ b/sql/share/errmsg.txt	2008-10-24 21:01:03 +0000
 > @@ -6404,4 +6404,11 @@ ER_BACKUP_BINLOG
 >          eng "Error on accessing binlog during BACKUP"
 >          nor "Lesing av binlog feilet under BACKUP"
 >          norwegian-ny "Lesing av binlog feila under BACKUP"
 > -
 > +ER_BACKUP_LOG_OUTPUT
 > +  eng "The operation could not be completed because the 
log_backup_output option does not include writing to tables."

[7] You are the native speaker, but I found this message a bit
strange.  Why "could not be completed"?  Sounds like it was started
and left in an uncompleted state.  Is option the right term for
log_backup_output? I also find "does not include writing to tables a
bit strange.  How about "The PURGE operation could not be executed
because logging to tables has not been enabled."?

 > +ER_BACKUP_PURGE_DATETIME
 > +  eng "The datetime specified is invalid for the '%-.64s' command."
 > +ER_BACKUP_LOGS_DELETED
 > +  eng "Backup log entries deleted: "

[8] Is the colon intentional?  Will something more be added by another
     statement?

 > +ER_BACKUP_LOGS_TRUNCATED
 > +  eng "All backup logs entries have been deleted"
 >
 > === modified file 'sql/sql_lex.h'
 > --- a/sql/sql_lex.h	2008-10-07 22:05:23 +0000
 > +++ b/sql/sql_lex.h	2008-10-24 21:01:03 +0000
 > @@ -89,7 +89,7 @@ enum enum_sql_command {
 >    SQLCOM_BEGIN, SQLCOM_CHANGE_MASTER,
 >    SQLCOM_RENAME_TABLE,
 >    SQLCOM_RESET, SQLCOM_PURGE, SQLCOM_PURGE_BEFORE, SQLCOM_SHOW_BINLOGS,
 > -  SQLCOM_SHOW_OPEN_TABLES,
 > +  SQLCOM_PURGE_BUP_LOG, SQLCOM_SHOW_OPEN_TABLES,
 >    SQLCOM_HA_OPEN, SQLCOM_HA_CLOSE, SQLCOM_HA_READ,
 >    SQLCOM_SHOW_SLAVE_HOSTS, SQLCOM_DELETE_MULTI, SQLCOM_UPDATE_MULTI,
 >    SQLCOM_SHOW_BINLOG_EVENTS, SQLCOM_SHOW_NEW_MASTER, SQLCOM_DO,
 > @@ -1529,6 +1529,7 @@ struct LEX: public Query_tables_list
 >    LEX_STRING backup_dir;				/* For RESTORE/BACKUP */
 >    bool backup_compression;
 >    char* to_log;                                 /* For PURGE MASTER 
LOGS TO */
 > +  ulonglong backup_id;     /* For PURGE BACKUP LOGS */
 >    char* x509_subject,*x509_issuer,*ssl_cipher;
 >    String *wild;
 >    sql_exchange *exchange;
 >
 > === modified file 'sql/sql_parse.cc'
 > --- a/sql/sql_parse.cc	2008-10-08 11:46:49 +0000
 > +++ b/sql/sql_parse.cc	2008-10-24 21:01:03 +0000
 > @@ -2100,6 +2100,97 @@ mysql_execute_command(THD *thd)
 >      res = purge_master_logs_before_date(thd, (ulong)it->val_int());
 >      break;
 >    }
 > +  /*
 > +    Check log status to see if the command applies.
 > +  */

[9] Is this the right place for this comment?  I think this comment
somewhere inside the case statement.

 > +  case SQLCOM_PURGE_BUP_LOG:
 > +  {
 > +    char buff[256];
 > +    int num= 0;
 > +    res= 0;
 > +
 > +    if (check_global_access(thd, SUPER_ACL))
 > +      goto error;
 > +
 > +    /*
 > +      If we are attempting to purge to a specified date or backup_id, we
 > +      must ensure the backup history log is turned on and is
 > +      being written to a table.
 > +    */
 > +    if (((lex->type == TYPE_ENUM_PURGE_BACKUP_LOGS_ID) ||
 > +         (lex->type == TYPE_ENUM_PURGE_BACKUP_LOGS_DATE)) &&
 > +         (opt_backup_history_log && !(log_backup_output_options & 
LOG_TABLE)))
 > +    {
 > +      my_error(ER_BACKUP_LOG_OUTPUT, MYF(0), ER(ER_BACKUP_LOG_OUTPUT));
 > +      goto error;
 > +    }
 > +
 > +    /*
 > +      Check the type of purge command and process accordingly.
 > +    */
 > +    switch (lex->type) {
 > +    case TYPE_ENUM_PURGE_BACKUP_LOGS:
 > +    {
 > +      if (sys_var_backupdir.value_length > 0)
 > +        res= logger.purge_backup_logs(thd);
 > +      break;

[10] Please explain this test for backupdir.  Why is it necessarty?
What does this mean?  Is the command ignored if the backupdir is not
set?  Is the user informed?

 > +    }
 > +    case TYPE_ENUM_PURGE_BACKUP_LOGS_ID:
 > +    {
 > +      res= logger.purge_backup_logs_before_id(thd, 
thd->lex->backup_id, &num);
 > +      break;
 > +    }
 > +    case TYPE_ENUM_PURGE_BACKUP_LOGS_DATE:
 > +    {
 > +      Item *it;
 > +
 > +      /*
 > +        Perform additional error checking for the
 > +        PURGE BACKUP LOGS BEFORE <date> command.
 > +      */
 > +      it= (Item *)lex->value_list.head();
 > +      if ((!it->fixed && it->fix_fields(lex->thd, &it)) ||
 > +          it->check_cols(1))
 > +      {
 > +        my_error(ER_WRONG_ARGUMENTS, MYF(0), "PURGE BACKUP BEFORE");

[11] I guess it is supposed to be "PURGE BACKUP LOGS BEFORE"
By the way, why is this error checking done here?  It seems to me to
better if this was done inside logger.  This switch is more than long
enough as it is.

 > +        goto error;
 > +      }
 > +      it= new Item_func_unix_timestamp(it);
 > +
 > +      /*
 > +        it is OK to only emulate fix_fields, because we need only
 > +        value of constant
 > +      */
 > +      it->quick_fix_field();
 > +
 > +      if ((ulong)it->val_int() == 0)
 > +      {
 > +        my_error(ER_BACKUP_PURGE_DATETIME, MYF(0), "PURGE BACKUP 
LOGS BEFORE");
 > +        goto error;
 > +      }
 > +
 > +      my_time_t t= (ulong)it->val_int();
 > +
 > +      /*
 > +        Validate datetime.
 > +      */
 > +      res= logger.purge_backup_logs_before_date(thd, t, &num);
 > +      break;
 > +    }
 > +    }
 > +
 > +    /*
 > +      Check result.
 > +    */
 > +    if (res)
 > +      goto error;
 > +    if (lex->type == TYPE_ENUM_PURGE_BACKUP_LOGS)
 > +      my_sprintf(buff, (buff, "%s.", ER(ER_BACKUP_LOGS_TRUNCATED)));
 > +    else
 > +      my_sprintf(buff, (buff, "%s %d.", ER(ER_BACKUP_LOGS_DELETED), 
num));
 > +    my_ok(thd, num, 0, buff);
 > +    break;
 > +  }
 >  #endif
 >    case SQLCOM_SHOW_WARNS:
 >    {
 >
 > === modified file 'sql/sql_yacc.yy'
 > --- a/sql/sql_yacc.yy	2008-10-02 14:03:57 +0000
 > +++ b/sql/sql_yacc.yy	2008-10-24 21:01:03 +0000
 > @@ -10813,10 +10813,32 @@ purge:
 >              lex->type=0;
 >              lex->sql_command = SQLCOM_PURGE;
 >            }
 > -          purge_options
 > -          {}
 > -        ;
 > +          purge_options {}
 > +          | PURGE BACKUP_SYM LOGS_SYM
 > +          {
 > +            LEX *lex=Lex;
 > +            lex->type=TYPE_ENUM_PURGE_BACKUP_LOGS;
 > +            lex->sql_command = SQLCOM_PURGE_BUP_LOG;
 > +          }
 > +          purge_bup_log_option {}
 > +          ;
 >
 > +purge_bup_log_option:
 > +          {}
 > +          | TO_SYM NUM_literal
 > +          {
 > +            LEX *lex= Lex;
 > +            lex->backup_id= (ulonglong)$2->val_int();
 > +            lex->type=TYPE_ENUM_PURGE_BACKUP_LOGS_ID;
 > +          }
 > +          | BEFORE_SYM expr
 > +          {
 > +            LEX *lex= Lex;
 > +            lex->value_list.empty();
 > +            lex->value_list.push_front($2);
 > +            lex->type=TYPE_ENUM_PURGE_BACKUP_LOGS_DATE;
 > +          }
 > +          ;
 >  purge_options:
 >            master_or_binary LOGS_SYM purge_option
 >          ;
 >
 > === modified file 'storage/csv/ha_tina.cc'
 > --- a/storage/csv/ha_tina.cc	2008-08-23 00:18:35 +0000
 > +++ b/storage/csv/ha_tina.cc	2008-10-24 21:01:03 +0000
 > @@ -80,7 +80,6 @@ static handler *tina_create_handler(hand
 >                                      TABLE_SHARE *table,
 >                                      MEM_ROOT *mem_root);
 >
 > -
 > 
/*****************************************************************************
 >   ** TINA tables
 > 
*****************************************************************************/
 > @@ -169,6 +168,7 @@ static TINA_SHARE *get_share(const char
 >      share->update_file_opened= FALSE;
 >      share->tina_write_opened= FALSE;
 >      share->data_file_version= 0;
 > +    share->allow_log_delete= FALSE;
 >      strmov(share->table_name, table_name);
 >      fn_format(share->data_file_name, table_name, "", CSV_EXT,
 >                MY_REPLACE_EXT|MY_UNPACK_FILENAME);
 > @@ -995,8 +995,13 @@ int ha_tina::delete_row(const uchar * bu
 >    share->rows_recorded--;
 >    pthread_mutex_unlock(&share->mutex);
 >
 > -  /* DELETE should never happen on the log table */
 > -  DBUG_ASSERT(!share->is_log_table);
 > +  /*
 > +     DELETE should never happen on the log table
 > +     UNLESS it is a backup log table in which case extra() should be
 > +     called with HA_EXTRA_ALLOW_LOG_DELETE then reset with
 > +     HA_EXTRA_MARK_AS_LOG_TABLE.
 > +  */
 > +  DBUG_ASSERT(!share->is_log_table || share->allow_log_delete);
 >
 >    DBUG_RETURN(0);
 >  }
 > @@ -1169,6 +1174,13 @@ int ha_tina::extra(enum ha_extra_functio
 >   {
 >     pthread_mutex_lock(&share->mutex);
 >     share->is_log_table= TRUE;
 > +   share->allow_log_delete= FALSE;
 > +   pthread_mutex_unlock(&share->mutex);
 > + }
 > + else if (operation == HA_EXTRA_ALLOW_LOG_DELETE)
 > + {
 > +   pthread_mutex_lock(&share->mutex);
 > +   share->allow_log_delete= TRUE;
 >     pthread_mutex_unlock(&share->mutex);
 >   }
 >    DBUG_RETURN(0);
 >
 > === modified file 'storage/csv/ha_tina.h'
 > --- a/storage/csv/ha_tina.h	2008-06-28 11:00:59 +0000
 > +++ b/storage/csv/ha_tina.h	2008-10-24 21:01:03 +0000
 > @@ -50,6 +50,7 @@ typedef struct st_tina_share {
 >    bool crashed;             /* Meta file is crashed */
 >    ha_rows rows_recorded;    /* Number of rows in tables */
 >    uint data_file_version;   /* Version of the data file used */
 > +  bool allow_log_delete;
 >  } TINA_SHARE;
 >
 >  struct tina_set {
 >
 >

Thread
bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364Chuck Bell24 Oct
  • Re: bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364Rafal Somla27 Oct
  • Re: bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364Øystein Grøvlen27 Oct
    • RE: bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364Chuck Bell27 Oct
      • Re: bzr commit into mysql-6.0 branch (cbell:2717) Bug#33364Øystein Grøvlen28 Oct