Re: bzr commit into mysql-6.0-backup branch (Rafal.Somla:2734) Bug#40262

[Rafal Somla]

Hi,

Thanks for the review. I'll apply the requested/suggested changes before pushing.

Øystein Grøvlen wrote:
> STATUS
> ======
> 
> Approved when a long line has been fixed.
> 
> REQUESTS
> ========
> 
> 1. Long line: [3]

OK.

> 
> SUGGESTIONS
> ===========
> 
> 1. Please, consider whether sleep is necessary [1]

See my arguments below. If you see a better way, I'm all ears.
> 
> 2. Use a single query to check all vp info [2]
> 

Yes, nice idea.

> 
> Rafal Somla wrote:
>> === modified file 'mysql-test/suite/backup/t/backup.test'
>> --- a/mysql-test/suite/backup/t/backup.test    2008-10-07 17:15:44 +0000
>> +++ b/mysql-test/suite/backup/t/backup.test    2008-11-17 14:43:10 +0000
>> @@ -193,6 +193,52 @@ DROP DATABASE db3;
>>  SET DEBUG_SYNC= 'RESET';
>>  
>>  #
>> +# Check that PTR data (such as VP time and binlog positon) is 
>> correctly stored and read
>> +# when there are no tables to backup (BUG#40262). +#
>> +
>> +--error 0,1
>> +--remove_file $MYSQLTEST_VARDIR/master-data/db1.bkp
>> +CREATE DATABASE db1;
>> +
>> +--replace_column 1 #
>> +BACKUP DATABASE db1 TO 'db1.bkp';
>> +# get backup_id of the BACKUP operation. +SELECT MAX(backup_id) FROM 
>> mysql.backup_history INTO @bid;
>> +
>> +# store VP time and binlog position
>> +
>> +SELECT validity_point_time FROM mysql.backup_history
>> +WHERE backup_id = @bid INTO @vp_time;
>> +SELECT binlog_file FROM mysql.backup_history
>> +WHERE backup_id = @bid INTO @vp_file;
>> +SELECT binlog_pos FROM mysql.backup_history
>> +WHERE backup_id = @bid INTO @vp_pos;
>> +
>> +DROP DATABASE db1;
>> +
>> +# wait few seconds so that restore time != backup time
>> +--sleep 2
> 
> [1]  I do not understand the purpose of sleep here.  Why does restore 
> time have to be different from backup time.

If restore time is identical to backup time, then it could happen 
(theoretically) that even with incorrect code the validity_point_time = @vp_time 
test below passes. For example if backup_history.validity_point_time is not set 
to the value read from backup image but to the time when RESTORE was executed.

> 
>> +
>> +--replace_column 1 #
>> +RESTORE FROM 'db1.bkp';
>> +# determine id of RESTORE operation
>> +SELECT MAX(backup_id) FROM mysql.backup_history INTO @bid;
>> +
>> +# check that VP info was correctly read and reported
>> +
>> +SELECT validity_point_time = @vp_time FROM mysql.backup_history
>> +WHERE backup_id = @bid; +SELECT binlog_file = @vp_file FROM 
>> mysql.backup_history
>> +WHERE backup_id = @bid; +SELECT binlog_pos = @vp_pos FROM 
>> mysql.backup_history
>> +WHERE backup_id = @bid; 
> 
> [2] All three criteria could be checked with a single select statement.
> 

Will do - nice idea.

Rafal