From: Rafal Somla Date: November 28 2008 9:57am Subject: Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2746) Bug#38426 List-Archive: http://lists.mysql.com/commits/60132 Message-Id: <492FC07C.8090503@sun.com> MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset=UTF-8 Content-Transfer-Encoding: 7BIT Hi Jorgen, STATUS ------ Patch approved. COMMENTARY ---------- See minor remarks/suggestions below. Jorgen Loland wrote: > #At file:///localhome/jl208045/mysql/mysql-6.0-backup-38426/ > > 2746 Jorgen Loland 2008-11-28 > Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver > > Before, the Default backup driver was used when backing up a partitioned table. However, the Snapshot driver is capable of backing up partitioned tables if the underlying storage engine is InnoDB or Falcon. > > With this patch, the Snapshot backup driver is used if the underlying storage engine of a partitioned table is InnoDB or Falcon > added: > mysql-test/suite/backup_engines/r/backup_partition.result > mysql-test/suite/backup_engines/t/backup_partition.test > modified: > sql/backup/backup_aux.h > sql/backup/backup_info.cc > > per-file messages: > mysql-test/suite/backup_engines/r/backup_partition.result > Test for checking that backup of partitioned tables uses correct backup driver > mysql-test/suite/backup_engines/t/backup_partition.test > Test for checking that backup of partitioned tables uses correct backup driver > sql/backup/backup_aux.h > Add inline method get_hton_se which returns storage engine type of a handlerton > sql/backup/backup_info.cc > Modify method get_storage_engine to return underlying storage engine of partitioned tables iff the storage engine is InnoDB or Falcon. ... > === added file 'mysql-test/suite/backup_engines/t/backup_partition.test' > --- a/mysql-test/suite/backup_engines/t/backup_partition.test 1970-01-01 00:00:00 +0000 > +++ b/mysql-test/suite/backup_engines/t/backup_partition.test 2008-11-28 09:05:46 +0000 > @@ -0,0 +1,68 @@ > +# > +# This test is designed to test that BACKUP selects the best available > +# driver for partitioned tables. The storage engines should use these > +# drivers: > +# > +# Partitioned Falcon table -> Snapshot > +# Partitioned InnoDB table -> Snapshot > +# All other partitioned tables -> Default > +# > + > +# Find expected backup driver for the current storage engine. > + > +let $exp_drv= Default; > +if (`select @@storage_engine = 'falcon'`) { let $exp_drv= Snapshot; } > +if (`select @@storage_engine = 'innodb'`) { let $exp_drv= Snapshot; } > + > +# Uncomment the line below and run the test with --force flag > +# if you want to verify that expected driver is correct > +#echo $exp_drv; > + > +--echo # > +--echo # Creating databases > +--echo # > +CREATE DATABASE backup_part; > + > +CREATE TABLE backup_part.t1 ( > + t1_autoinc INTEGER NOT NULL AUTO_INCREMENT, > + t1_uuid CHAR(36), > + PRIMARY KEY (t1_autoinc) > +); > +ALTER TABLE backup_part.t1 PARTITION BY HASH(t1_autoinc); > + > +INSERT INTO backup_part.t1(t1_uuid) VALUES ('aaa'),('bbb'),('ccc'),('ddd'); > + > +--echo # Show content > +SELECT * FROM backup_part.t1 ORDER BY t1_autoinc; > + > +--echo # > +--echo # Perform backup > +--replace_column 1 # > +let $bup_id= `BACKUP DATABASE backup_part TO 'partition.bak'`; > + > +--echo # > +--echo # Show drivers. > + > +--echo > +# Count of expected driver for this backup_id should be 1 > +# Replace exp_drv and backup_id in query because it will vary > +replace_regex /'.*'.*/'FILTERED DRIVER' AND backup_id=FILTERED ID/ ; > +--eval SELECT count(*) FROM mysql.backup_history WHERE drivers LIKE '$exp_drv' AND backup_id=$bup_id You could do it without "replace_regex" as follows: --eval SELECT drivers LIKE '$exp_drv' FROM mysql.backup_history WHERE backup_id=$bup_id; or even: --eval SELECT drivers = '$exp_drv' FROM mysql.backup_history WHERE backup_id=$bup_id; > +--echo > + > +--echo # > +--echo # Restore and show content > +--echo # > + > +--replace_column 1 # > +RESTORE FROM 'partition.bak' OVERWRITE; > +--echo > +SELECT * FROM backup_part.t1 ORDER BY t1_autoinc; > + > +--echo # > +--echo # Cleanup > +--echo # > + > +DROP DATABASE backup_part; > + > +--remove_file $MYSQLTEST_VARDIR/master-data/partition.bak ... > === modified file 'sql/backup/backup_info.cc' > --- a/sql/backup/backup_info.cc 2008-11-05 09:41:15 +0000 > +++ b/sql/backup/backup_info.cc 2008-11-28 09:05:46 +0000 > @@ -8,6 +8,7 @@ > */ > > #include "../mysql_priv.h" > +#include "../ha_partition.h" > > #include "backup_info.h" > #include "backup_kernel.h" > @@ -34,6 +35,50 @@ storage_engine_ref get_storage_engine(TH > if (table) > { > se= plugin_ref_to_se_ref(table->s->db_plugin); > + > + /* > + Further check for underlying storage engine is needed > + if table is partitioned > + */ > + > + storage_engine_ref se_tmp= NULL; > + > + if (table->part_info) > + { Wrong indentation: { shuld be directly below if > + partition_info *p_info= table->part_info; > + List_iterator p_it(p_info->partitions); > + partition_element *p_el; > + > + while ((p_el= p_it++)) > + { > + if (!se_tmp) > + { > + se_tmp= get_hton_se(p_el->engine_type); > + ::handlerton *h= se_hton(se_tmp); > + > + /* > + Only snapshot driver supports partitioning. Use > + default driver for all non-MVCC storage engines. > + */ > + if (h->start_consistent_snapshot == NULL) > + { > + se_tmp= NULL; The assignment is redundant. > + goto close; > + } > + > + continue; > + } > + > + // use default driver if partitions have different storage engines > + if (se_tmp != get_hton_se(p_el->engine_type)) > + goto close; > + }; > + > + se= se_tmp; > + } > + > + close: > + > ::intern_close_table(table); > my_free(table, MYF(0)); > } > > Rafal