Re: bzr commit into mysql-6.0-backup branch (rsomla:2688) Bug#38261

[Rafal Somla]

Thanks for looking at the patch!

Øystein Grøvlen wrote:
> 
> 1. You have added select statements after each backup, but what does
>    these statements test?  For restore, rollback is used to test that
>    restore transaction was automatically committed.  Maybe that could
>    be done for backup, too?
> 

These selects also test if transaction has been commited. Since we insert data 
inside the transaction, select should show it only if transaction is commited.

The advantage over testing commit with ROLLBACK is that we can continue with 
transaction commited (although not sure if that is really needed here).

> 2. You have only added tests for one storage engine/backup driver,
>    while there a restore tests for a combination of drivers and
>    autocommit on/off.  Is that sufficient?  Should the behavior of
>    other engines also be tested?  For example, parts of the test would
>    fail on Falcon since it does not allow restore of objects locked by
>    other transactions.
>

Jorgen has a similar concern. I'll ponder and see if I can rewrite this test to 
use "combinations", as Hema did in her tests. Then the same test will be run for 
the two transactional engines we have in the server. Do you agree that it is a 
good idea?


> SUGGESTIONS
> -----------
> 
> 1. Maybe a comment should be added to close() method to explain why
>    commit is not needed
> 

Hmm, I'm reluctant to comment code which is *not* there. I'll see if I can find 
a good place explaining how commits are handled by BACKUP/RESTORE commands.

Rafal