Re: bk commit into 6.0 tree (cbell:1.2613) WL#3572

[Rafal Somla]

Hi Chuck,

Nice and clean patch. I have a couple of requests and suggestions before finally 
accepting it. See inlined comments for more details.

- A suggestion to simplify get_data() using a state-less implementation since 
this should be enough.

- A request to set buf.table_no= 0 in get_data() method.

- A request to change the code used for the new snapshot type in backup image 
format.

- A request to improve comments which resemble too much the ones used for the 
default backup engine and are not appropriate for the no-data backup engine.

Rafal

cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 6.0 repository of cbell.  When cbell does a push these changes
> will be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> 
> ChangeSet@stripped, 2008-04-24 11:08:34-04:00, cbell@mysql_cab_desk. +13 -0
>   WL#3572 : Online backup: No-data engine backup and restore
>   
>   This patch implements a no data driver for backup and restore of tables
>   whose engine does not store data. 
> 
>   mysql-test/r/backup_nodata_driver.result@stripped, 2008-04-24 11:08:29-04:00,
> cbell@mysql_cab_desk. +200 -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     New result file.
> 
>   mysql-test/r/backup_nodata_driver.result@stripped, 2008-04-24 11:08:29-04:00,
> cbell@mysql_cab_desk. +0 -0
> 
>   mysql-test/t/backup_nodata_driver.test@stripped, 2008-04-24 11:08:30-04:00,
> cbell@mysql_cab_desk. +165 -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Test for no data driver.
> 
>   mysql-test/t/backup_nodata_driver.test@stripped, 2008-04-24 11:08:30-04:00,
> cbell@mysql_cab_desk. +0 -0
> 
>   sql/backup/CMakeLists.txt@stripped, 2008-04-24 11:08:21-04:00, cbell@mysql_cab_desk. +1
> -1
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added new driver code to cmake file.
> 
>   sql/backup/Makefile.am@stripped, 2008-04-24 11:08:22-04:00, cbell@mysql_cab_desk. +2
> -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added new driver code to make file.
> 
>   sql/backup/backup_info.cc@stripped, 2008-04-24 11:08:23-04:00, cbell@mysql_cab_desk. +11
> -3
>     WL#3573 : Online backup: No-data engine backup and restore
>     
>     Added code to add a snapshot to the list of default drivers.
> 
>   sql/backup/be_default.h@stripped, 2008-04-24 11:08:23-04:00, cbell@mysql_cab_desk. +18
> -2
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added a more detailed accept method that rejects no data engines.
> 
>   sql/backup/be_nodata.cc@stripped, 2008-04-24 11:08:28-04:00, cbell@mysql_cab_desk. +149
> -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     No data driver for processing tables whose engines do not store data.
>     These include:
>     
>     DB_TYPE_MRG_MYISAM
>     DB_TYPE_BLACKHOLE_DB
>     DB_TYPE_FEDERATED_DB
>     DB_TYPE_EXAMPLE_DB
>     
> 
>   sql/backup/be_nodata.cc@stripped, 2008-04-24 11:08:28-04:00, cbell@mysql_cab_desk. +0
> -0
> 
>   sql/backup/be_nodata.h@stripped, 2008-04-24 11:08:28-04:00, cbell@mysql_cab_desk. +174
> -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Header file for no data driver.
> 
>   sql/backup/be_nodata.h@stripped, 2008-04-24 11:08:28-04:00, cbell@mysql_cab_desk. +0 -0
> 
>   sql/backup/image_info.h@stripped, 2008-04-24 11:08:24-04:00, cbell@mysql_cab_desk. +2
> -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added no data enum.
> 
>   sql/backup/kernel.cc@stripped, 2008-04-24 11:08:25-04:00, cbell@mysql_cab_desk. +6 -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added no data driver to catalog setup code.
> 
>   sql/backup/stream_v1.c@stripped, 2008-04-24 11:08:26-04:00, cbell@mysql_cab_desk. +10
> -7
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added nodata driver to stream types and comments.
> 
>   sql/backup/stream_v1.h@stripped, 2008-04-24 11:08:26-04:00, cbell@mysql_cab_desk. +1 -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added nodata driver to stream types and comments.
> 
>   storage/example/ha_example.cc@stripped, 2008-04-24 11:08:27-04:00,
> cbell@mysql_cab_desk. +1 -0
>     WL#3572 : Online backup: No-data engine backup and restore
>     
>     Added missing DB_TYPE for example storage engine.
> 
> diff -Nrup a/mysql-test/r/backup_nodata_driver.result
> b/mysql-test/r/backup_nodata_driver.result
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/r/backup_nodata_driver.result	2008-04-24 11:08:29 -04:00
> @@ -0,0 +1,200 @@
> +DROP DATABASE IF EXISTS bup_nodata;
> +DROP DATABASE IF EXISTS bup_data;
> +Creating tables
> +CREATE DATABASE bup_nodata;
> +CREATE DATABASE bup_data;
> +CREATE TABLE bup_data.myisam1 (a int, b char(30)) ENGINE=MYISAM;
> +CREATE TABLE bup_data.myisam2 (a int, b char(30)) ENGINE=MYISAM;
> +CREATE TABLE bup_data.myisam3 (a int, b char(30)) ENGINE=MYISAM;
> +CREATE TABLE bup_data.f1 (
> +`id` int(20) NOT NULL,
> +`group` int NOT NULL default 0,
> +`a\\b` int NOT NULL default 0,
> +`a\\` int NOT NULL default 0,
> +`name` varchar(32) NOT NULL default ''
> +    )
> +DEFAULT CHARSET=latin1;
> +CREATE TABLE bup_nodata.merge1 (a int, b char(30))
> +ENGINE=MERGE UNION=(bup_data.myisam1, bup_data.myisam2, bup_data.myisam3);
> +CREATE TABLE bup_nodata.f1 (
> +`id` int(20) NOT NULL,
> +`group` int NOT NULL default 0,
> +`a\\b` InT NOT NULL default 0,
> +`a\\` int NOT NULL default 0,
> +`name` varchar(32) NOT NULL default ''
> +    )
> +ENGINE="FEDERATED" DEFAULT CHARSET=latin1
> +CONNECTION='mysql://root@stripped:MASTER_PORT/bup_data/f1';
> +CREATE TABLE bup_nodata.b1 (a int, b int, c char(10)) ENGINE=BLACKHOLE;
> +CREATE TABLE bup_nodata.e1 (
> +Period smallint(4) unsigned zerofill DEFAULT '0000' NOT NULL,
> +Vapor_period smallint(4) unsigned DEFAULT '0' NOT NULL
> +) ENGINE=example;
> +Inserting data
> +INSERT INTO bup_data.myisam1 VALUES (11, 'table 1');
> +INSERT INTO bup_data.myisam1 VALUES (12, 'table 1');
> +INSERT INTO bup_data.myisam1 VALUES (13, 'table 1');
> +INSERT INTO bup_data.myisam2 VALUES (21, 'table 2');
> +INSERT INTO bup_data.myisam2 VALUES (22, 'table 2');
> +INSERT INTO bup_data.myisam2 VALUES (23, 'table 2');
> +INSERT INTO bup_data.myisam3 VALUES (31, 'table 3');
> +INSERT INTO bup_data.myisam3 VALUES (32, 'table 3');
> +INSERT INTO bup_data.myisam3 VALUES (33, 'table 3');
> +INSERT INTO bup_data.f1 (id, name) VALUES (1, 'foo');
> +INSERT INTO bup_data.f1 (id, name) VALUES (2, 'fee');
> +INSERT INTO bup_data.f1 (id, `group`) VALUES (3, 42);
> +INSERT INTO bup_data.f1 (id, `a\\b`) VALUES (4, 23);
> +INSERT INTO bup_data.f1 (id, `a\\`) VALUES (5, 1);
> +show data
> +SHOW FULL TABLES FROM bup_data;
> +Tables_in_bup_data	Table_type
> +f1	BASE TABLE
> +myisam1	BASE TABLE
> +myisam2	BASE TABLE
> +myisam3	BASE TABLE
> +SHOW FULL TABLES FROM bup_nodata;
> +Tables_in_bup_nodata	Table_type
> +b1	BASE TABLE
> +e1	BASE TABLE
> +f1	BASE TABLE
> +merge1	BASE TABLE
> +SELECT * FROM bup_nodata.merge1;
> +a	b
> +11	table 1
> +12	table 1
> +13	table 1
> +21	table 2
> +22	table 2
> +23	table 2
> +31	table 3
> +32	table 3
> +33	table 3
> +SELECT * FROM bup_nodata.f1;
> +id	group	a\\b	a\\	name
> +1	0	0	0	foo
> +2	0	0	0	fee
> +3	42	0	0	
> +4	0	23	0	
> +5	0	0	1	
> +SELECT * FROM bup_nodata.b1;
> +a	b	c
> +SELECT * FROM bup_nodata.e1;
> +Period	Vapor_period
> +BACKUP DATABASE bup_data TO 'bup_data.bak';
> +backup_id
> +#
> +BACKUP DATABASE bup_nodata TO 'bup_nodata.bak';
> +backup_id
> +#
> +show data
> +SHOW FULL TABLES FROM bup_data;
> +Tables_in_bup_data	Table_type
> +f1	BASE TABLE
> +myisam1	BASE TABLE
> +myisam2	BASE TABLE
> +myisam3	BASE TABLE
> +SHOW FULL TABLES FROM bup_nodata;
> +Tables_in_bup_nodata	Table_type
> +b1	BASE TABLE
> +e1	BASE TABLE
> +f1	BASE TABLE
> +merge1	BASE TABLE
> +SELECT * FROM bup_nodata.merge1;
> +a	b
> +11	table 1
> +12	table 1
> +13	table 1
> +21	table 2
> +22	table 2
> +23	table 2
> +31	table 3
> +32	table 3
> +33	table 3
> +SELECT * FROM bup_nodata.f1;
> +id	group	a\\b	a\\	name
> +1	0	0	0	foo
> +2	0	0	0	fee
> +3	42	0	0	
> +4	0	23	0	
> +5	0	0	1	
> +SELECT * FROM bup_nodata.b1;
> +a	b	c
> +SELECT * FROM bup_nodata.e1;
> +Period	Vapor_period
> +DROP DATABASE bup_data;
> +show data
> +SHOW FULL TABLES FROM bup_nodata;
> +Tables_in_bup_nodata	Table_type
> +b1	BASE TABLE
> +e1	BASE TABLE
> +f1	BASE TABLE
> +merge1	BASE TABLE
> +SELECT * FROM bup_nodata.merge1;
> +ERROR 42S02: Table 'bup_data.myisam1' doesn't exist
> +SELECT * FROM bup_nodata.f1;
> +Got one of the listed errors
> +SELECT * FROM bup_nodata.b1;
> +a	b	c
> +SELECT * FROM bup_nodata.e1;
> +Period	Vapor_period
> +DROP DATABASE bup_nodata;
> +Restoring nodata database.
> +RESTORE FROM 'bup_nodata.bak';
> +backup_id
> +#
> +show data
> +SHOW FULL TABLES FROM bup_nodata;
> +Tables_in_bup_nodata	Table_type
> +b1	BASE TABLE
> +e1	BASE TABLE
> +f1	BASE TABLE
> +merge1	BASE TABLE
> +SELECT * FROM bup_nodata.merge1;
> +ERROR 42S02: Table 'bup_data.myisam1' doesn't exist
> +SELECT * FROM bup_nodata.f1;
> +Got one of the listed errors
> +SELECT * FROM bup_nodata.b1;
> +a	b	c
> +SELECT * FROM bup_nodata.e1;
> +Period	Vapor_period
> +Restoring data database.
> +RESTORE FROM 'bup_data.bak';
> +backup_id
> +#
> +show data
> +SHOW FULL TABLES FROM bup_data;
> +Tables_in_bup_data	Table_type
> +f1	BASE TABLE
> +myisam1	BASE TABLE
> +myisam2	BASE TABLE
> +myisam3	BASE TABLE
> +SHOW FULL TABLES FROM bup_nodata;
> +Tables_in_bup_nodata	Table_type
> +b1	BASE TABLE
> +e1	BASE TABLE
> +f1	BASE TABLE
> +merge1	BASE TABLE
> +SELECT * FROM bup_nodata.merge1;
> +a	b
> +11	table 1
> +12	table 1
> +13	table 1
> +21	table 2
> +22	table 2
> +23	table 2
> +31	table 3
> +32	table 3
> +33	table 3
> +SELECT * FROM bup_nodata.f1;
> +id	group	a\\b	a\\	name
> +1	0	0	0	foo
> +2	0	0	0	fee
> +3	42	0	0	
> +4	0	23	0	
> +5	0	0	1	
> +SELECT * FROM bup_nodata.b1;
> +a	b	c
> +SELECT * FROM bup_nodata.e1;
> +Period	Vapor_period
> +DROP DATABASE bup_data;
> +DROP DATABASE bup_nodata;
> diff -Nrup a/mysql-test/t/backup_nodata_driver.test
> b/mysql-test/t/backup_nodata_driver.test
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/backup_nodata_driver.test	2008-04-24 11:08:30 -04:00
> @@ -0,0 +1,165 @@
> +#
> +# This test tests the backup using no data engines.
> +# It was made a separate test due to the possibility that some
> +# of the engines used may not be available on all platforms or
> +# builds.
> +#
> +# The test creates a database with tables using all of the known
> +# no data engines. It runs a backup then a restore and compares
> +# the results.
> +#
> +--source include/not_embedded.inc
> +--source include/have_federated_db.inc
> +--source include/have_exampledb.inc
> +
> +--disable_warnings
> +DROP DATABASE IF EXISTS bup_nodata;
> +DROP DATABASE IF EXISTS bup_data;
> +--enable_warnings
> +
> +# Create data
> +--echo Creating tables
> +CREATE DATABASE bup_nodata;
> +
> +CREATE DATABASE bup_data;
> +
> +CREATE TABLE bup_data.myisam1 (a int, b char(30)) ENGINE=MYISAM;
> +
> +CREATE TABLE bup_data.myisam2 (a int, b char(30)) ENGINE=MYISAM;
> +
> +CREATE TABLE bup_data.myisam3 (a int, b char(30)) ENGINE=MYISAM;
> +
> +CREATE TABLE bup_data.f1 (
> +    `id` int(20) NOT NULL,
> +    `group` int NOT NULL default 0,
> +    `a\\b` int NOT NULL default 0,
> +    `a\\` int NOT NULL default 0,
> +    `name` varchar(32) NOT NULL default ''

Any particular reason for such column names?

> +    )
> +  DEFAULT CHARSET=latin1;
> +
> +CREATE TABLE bup_nodata.merge1 (a int, b char(30))
> +  ENGINE=MERGE UNION=(bup_data.myisam1, bup_data.myisam2, bup_data.myisam3);
> +
> +--replace_result $MASTER_MYPORT MASTER_PORT
> +eval CREATE TABLE bup_nodata.f1 (
> +    `id` int(20) NOT NULL,
> +    `group` int NOT NULL default 0,
> +    `a\\\\b` InT NOT NULL default 0,
> +    `a\\\\` int NOT NULL default 0,
> +    `name` varchar(32) NOT NULL default ''
> +    )
> +  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
> +  CONNECTION='mysql://root@stripped:$MASTER_MYPORT/bup_data/f1';
> +
> +CREATE TABLE bup_nodata.b1 (a int, b int, c char(10)) ENGINE=BLACKHOLE;
> +
> +CREATE TABLE bup_nodata.e1 (
> +  Period smallint(4) unsigned zerofill DEFAULT '0000' NOT NULL,
> +  Vapor_period smallint(4) unsigned DEFAULT '0' NOT NULL
> +) ENGINE=example;
> +
> +# Insert some data (for merge and federated to ensure proper working tables)
> +--echo Inserting data
> +INSERT INTO bup_data.myisam1 VALUES (11, 'table 1');
> +INSERT INTO bup_data.myisam1 VALUES (12, 'table 1');
> +INSERT INTO bup_data.myisam1 VALUES (13, 'table 1');
> +INSERT INTO bup_data.myisam2 VALUES (21, 'table 2');
> +INSERT INTO bup_data.myisam2 VALUES (22, 'table 2');
> +INSERT INTO bup_data.myisam2 VALUES (23, 'table 2');
> +INSERT INTO bup_data.myisam3 VALUES (31, 'table 3');
> +INSERT INTO bup_data.myisam3 VALUES (32, 'table 3');
> +INSERT INTO bup_data.myisam3 VALUES (33, 'table 3');
> +
> +INSERT INTO bup_data.f1 (id, name) VALUES (1, 'foo');
> +INSERT INTO bup_data.f1 (id, name) VALUES (2, 'fee');
> +INSERT INTO bup_data.f1 (id, `group`) VALUES (3, 42);
> +INSERT INTO bup_data.f1 (id, `a\\b`) VALUES (4, 23);
> +INSERT INTO bup_data.f1 (id, `a\\`) VALUES (5, 1);
> +
> +# Show the data
> +--echo show data
> +SHOW FULL TABLES FROM bup_data;
> +SHOW FULL TABLES FROM bup_nodata;
> +
> +SELECT * FROM bup_nodata.merge1;
> +SELECT * FROM bup_nodata.f1;
> +SELECT * FROM bup_nodata.b1;
> +SELECT * FROM bup_nodata.e1;
> +
> +# Do the backup of the bup_data DB.
> +--replace_column 1 #
> +BACKUP DATABASE bup_data TO 'bup_data.bak';
> +
> +# Do the backup of the bup_nodata DB.
> +--replace_column 1 #
> +BACKUP DATABASE bup_nodata TO 'bup_nodata.bak';
> +
> +# Show the data
> +--echo show data
> +SHOW FULL TABLES FROM bup_data;
> +SHOW FULL TABLES FROM bup_nodata;
> +
> +SELECT * FROM bup_nodata.merge1;
> +SELECT * FROM bup_nodata.f1;
> +SELECT * FROM bup_nodata.b1;
> +SELECT * FROM bup_nodata.e1;
> +
> +# Now drop the data database and show what is left
> +
> +DROP DATABASE bup_data;
> +
> +# Show the data
> +--echo show data
> +SHOW FULL TABLES FROM bup_nodata;
> +
> +# The merge and federated tables should have errors since data is missing.
> +--error ER_NO_SUCH_TABLE
> +SELECT * FROM bup_nodata.merge1;
> +--error ER_CONNECT_TO_FOREIGN_DATA_SOURCE,ER_QUERY_ON_FOREIGN_DATA_SOURCE
> +SELECT * FROM bup_nodata.f1;
> +SELECT * FROM bup_nodata.b1;
> +SELECT * FROM bup_nodata.e1;
> +
> +DROP DATABASE bup_nodata;
> +
> +# Now restore the nodata database and see if it is the same as above.
> +--echo Restoring nodata database.
> +--replace_column 1 #
> +RESTORE FROM 'bup_nodata.bak';
> +
> +# Show the data
> +--echo show data
> +SHOW FULL TABLES FROM bup_nodata;
> +
> +# The merge and federated tables should have errors since data is missing.
> +--error ER_NO_SUCH_TABLE
> +SELECT * FROM bup_nodata.merge1;
> +--error ER_CONNECT_TO_FOREIGN_DATA_SOURCE,ER_QUERY_ON_FOREIGN_DATA_SOURCE
> +SELECT * FROM bup_nodata.f1;
> +SELECT * FROM bup_nodata.b1;
> +SELECT * FROM bup_nodata.e1;
> +
> +# Now restore the data database and see that all is well.
> +--echo Restoring data database.
> +--replace_column 1 #
> +RESTORE FROM 'bup_data.bak';
> +
> +# Show the data
> +--echo show data
> +SHOW FULL TABLES FROM bup_data;
> +SHOW FULL TABLES FROM bup_nodata;
> +
> +SELECT * FROM bup_nodata.merge1;
> +SELECT * FROM bup_nodata.f1;
> +SELECT * FROM bup_nodata.b1;
> +SELECT * FROM bup_nodata.e1;
> +
> +DROP DATABASE bup_data;
> +DROP DATABASE bup_nodata;
> +
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_data.bak
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_nodata.bak
> +

Nice test. And amazing that it works :)

> diff -Nrup a/sql/backup/CMakeLists.txt b/sql/backup/CMakeLists.txt
> --- a/sql/backup/CMakeLists.txt	2008-03-04 11:08:40 -05:00
> +++ b/sql/backup/CMakeLists.txt	2008-04-24 11:08:21 -04:00
> @@ -26,7 +26,7 @@ SET(BACKUP_SOURCES stream.cc logger.cc k
>                 image_info.cc backup_info.cc data_backup.cc
>                 be_default.cc buffer_iterator.cc
>                 be_snapshot.cc be_thread.cc backup_progress.cc
> -               backup_test.cc)
> +               backup_test.cc be_nodata.cc)
>  
>  IF(NOT SOURCE_SUBLIBS)
>    ADD_LIBRARY(backup ${BACKUP_SOURCES})

> diff -Nrup a/sql/backup/Makefile.am b/sql/backup/Makefile.am
> --- a/sql/backup/Makefile.am	2008-03-04 11:06:22 -05:00
> +++ b/sql/backup/Makefile.am	2008-04-24 11:08:22 -04:00
> @@ -34,6 +34,7 @@ libbackup_la_SOURCES = \
>    data_backup.cc \
>    be_default.cc \
>    be_snapshot.cc \
> +  be_nodata.cc \
>    buffer_iterator.cc \
>    be_thread.cc \
>    backup_progress.cc \
> @@ -62,6 +63,7 @@ noinst_HEADERS = \
>    be_native.h \
>    be_default.h \
>    be_snapshot.h \
> +  be_nodata.h \
>    buffer_iterator.h \
>    be_thread.h \
>    backup_progress.h \

> diff -Nrup a/sql/backup/backup_info.cc b/sql/backup/backup_info.cc
> --- a/sql/backup/backup_info.cc	2008-04-16 14:23:01 -04:00
> +++ b/sql/backup/backup_info.cc	2008-04-24 11:08:23 -04:00
> @@ -13,6 +13,7 @@
>  #include "be_native.h"
>  #include "be_default.h"
>  #include "be_snapshot.h"
> +#include "be_nodata.h"
>  
>  /// Return storage engine of a given table.
>  static
> @@ -201,10 +202,17 @@ Backup_info::Backup_info(Backup_restore_
>    bzero(m_snap, sizeof(m_snap));
>  
>    /* 
> -    Create default and CS snapshot objects and add them to the snapshots list.
> -    Note that the default snapshot should be the last element on that list, as a
> -    "catch all" entry. 
> +    Create nodata, default, and CS snapshot objects and add them to the 
> +    snapshots list. Note that the default snapshot should be the last 
> +    element on that list, as a "catch all" entry. 
>     */
> +
> +  snap= new Nodata_snapshot(m_ctx);  // reports errors
> +
> +  if (!snap || !snap->is_valid())
> +    return;
> +
> +  snapshots.push_back(snap);
>  
>    snap= new CS_snapshot(m_ctx); // reports errors
>  

> diff -Nrup a/sql/backup/be_default.h b/sql/backup/be_default.h
> --- a/sql/backup/be_default.h	2008-03-04 11:06:22 -05:00
> +++ b/sql/backup/be_default.h	2008-04-24 11:08:23 -04:00
> @@ -222,8 +222,24 @@ class Default_snapshot: public Snapshot_
>    const char* name() const
>    { return "Default"; }
>  
> -  bool accept(const Table_ref&, const storage_engine_ref)
> -  { return TRUE; }; // accept all tables
> +  bool accept(const backup::Table_ref &,const storage_engine_ref e)
> +  { 
> +    bool accepted= TRUE;
> +    handlerton *h= se_hton(e);
> +
> +    /*
> +      Do not accept nodata engines.
> +    */
> +    switch (h->db_type) {
> +    case DB_TYPE_MRG_MYISAM:
> +    case DB_TYPE_BLACKHOLE_DB:
> +    case DB_TYPE_FEDERATED_DB:
> +    case DB_TYPE_EXAMPLE_DB:
> +      accepted= FALSE;
> +      break;
> +    }
> +    return (accepted);
> +  }; 
>  
>    result_t get_backup_driver(Backup_driver* &ptr)
>    { return (ptr= new default_backup::Backup(m_tables, ::current_thd,

> diff -Nrup a/sql/backup/be_nodata.cc b/sql/backup/be_nodata.cc
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/sql/backup/be_nodata.cc	2008-04-24 11:08:28 -04:00
> @@ -0,0 +1,149 @@
> +/* Copyright (C) 2004-2007 MySQL AB
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; version 2 of the License.
> +
> +   This program is distributed in the hope that it will be useful, 
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
> +*/
> +
> +/**
> +  @file 
> + 
> +  @brief Contains the nodata backup algorithm driver.
> + 
> +  This file contains the nodata backup algorithm (also called a "driver"
> +  in the online backup terminology. The nodata backup algorithm may be
> +  used in place of an engine-specific driver if one does not exist or if
> +  chosen by the user.
> + 
> +  The nodata backup algorithm is a blocking algorithm that locks all of
> +  the tables given at the start of the backup/restore process. Once all of
> +  the data is backed up or restored, the locks are removed. The nodata
> +  backup is a row-level backup and therefore does not backup the indexes
> +  or any of the engine-specific files.

The comment is good for the default driver but doesn't fit to the no-data one.

> +*/
> +#include "../mysql_priv.h"
> +#include "backup_engine.h"
> +#include "be_nodata.h"
> +#include "backup_aux.h"
> +#include "rpl_record.h"
> +
> +namespace nodata_backup {
> +
> +using backup::byte;
> +using backup::result_t;
> +using backup::version_t;
> +using backup::Table_list;
> +using backup::Table_ref;
> +using backup::Buffer;
> +
> +using namespace backup;
> +
> +Engine::Engine(THD *t_thd)
> +{
> +  m_thd= t_thd;
> +}
> +
> +/**
> +  Create a nodata backup backup driver.
> +  
> +  Given a list of tables to be backed-up, create instance of backup
> +  driver which will create backup image of these tables.
> +  
> +  @param[IN]  tables list of tables to be backed-up.
> +  @param[OUT] eng    pointer to backup driver instance.
> +  
> +  @retval  ERROR  if cannot create backup driver class.
> +  @retval  OK     on success.
> +*/
> +result_t Engine::get_backup(const uint32, const Table_list &tables,
> +                            Backup_driver* &drv)
> +{
> +  DBUG_ENTER("Engine::get_backup");
> +  Backup *ptr= new nodata_backup::Backup(tables);
> +  if (!ptr)
> +    DBUG_RETURN(ERROR);
> +  drv= ptr;
> +  DBUG_RETURN(OK);
> +}
> +
> +/**
> +  @brief Get the data for a row in the table.
> +
> +  This method is the main method used in the backup operation. It is
> +  responsible for reading a row from the table and placing the data in
> +  the buffer (buf.data) and setting the correct attributes for processing
> +  (e.g., buf.size = size of record data).
> +  
> +*/
> +result_t Backup::get_data(Buffer &buf)
> +{
> +  DBUG_ENTER("Nodata_backup::get_data)");

Add "buf.table_no= 0".

> +  buf.last= TRUE;
> +  buf.size= 0;
> +  switch (m_driver_state) {
> +  case ND_INITIALIZE:
> +  {
> +    m_driver_state= ND_LOCK;
> +    DBUG_RETURN(READY);
> +  }
> +  case ND_LOCK:
> +  {
> +    m_driver_state= ND_DONE;
> +    DBUG_RETURN(READY);
> +  }
> +  default:
> +    DBUG_RETURN(DONE);
> +  }
> +}

After consulting the backup scheduler code, I think that a state-less version of 
get_data() which always returns DONE should work ok. Can you try this? If it 
really works we will have much simpler omplementation.

I.e., the get_data() method should simply do this:

  buf.table_no= 0;
  buf.size= 0;
  buf.last= TRUE;
  DBUG_RETURN(DONE);

Note: the READY answer will be given by prepare() and this is enough for the 
scheduler.

> +
> +/**
> +   Create a nodata backup restore driver.
> +  
> +   Given a list of tables to be restored, create instance of restore
> +   driver which will restore these tables from a backup image.
> +  
> +   @param[IN]  version  version of the backup image.
> +   @param[IN]  tables   list of tables to be restored.
> +   @param[OUT] eng      pointer to restore driver instance.
> +  
> +   @retval ERROR  if cannot create restore driver class.
> +   @retval OK     on success.
> +*/
> +result_t Engine::get_restore(version_t, const uint32, 
> +                             const Table_list &tables, Restore_driver*
> &drv)
> +{
> +  DBUG_ENTER("Engine::get_restore");
> +  Restore *ptr= new nodata_backup::Restore(tables, m_thd);
> +  if (!ptr)
> +    DBUG_RETURN(ERROR);
> +  drv= ptr;
> +  DBUG_RETURN(OK);
> +}
> +
> +/**
> +   @brief Restore the data for a row in the table.
> +  
> +   This method is the main method used in the restore operation. It is
> +   responsible for writing a row to the table.

We don't write any rows here.

> +  
> +*/
> +result_t Restore::send_data(Buffer &buf)
> +{
> +  DBUG_ENTER("Nodata_backup::send_data)");
> +  buf.last= TRUE;
> +  buf.size= 0;
> +  DBUG_RETURN(DONE);
> +}
> +
> +} /* nodata_backup namespace */
> +
> +

> diff -Nrup a/sql/backup/be_nodata.h b/sql/backup/be_nodata.h
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/sql/backup/be_nodata.h	2008-04-24 11:08:28 -04:00
> @@ -0,0 +1,174 @@
> +#ifndef _NODATA_BACKUP_H
> +#define _NODATA_BACKUP_H
> +
> +#include <backup_engine.h>
> +#include <backup/image_info.h>  // to define default backup image class
> +#include <backup/buffer_iterator.h>
> +#include <backup/be_thread.h>
> +
> +namespace nodata_backup {
> +
> +using backup::byte;
> +using backup::result_t;
> +using backup::version_t;
> +using backup::Table_list;
> +using backup::Table_ref;
> +using backup::Buffer;
> +
> +/**
> + * @class Engine
> + *
> + * @brief Encapsulates nodata online backup/restore functionality.
> + *
> + * This class is used to initiate the nodata backup algorithm, which is used
> + * by the backup kernel to create a backup image of data stored in any
> + * engine that does not have a native backup driver. It may also be used as
> + * an option by the user.
> + *
> + * Using this class, the caller can create an instance of the nodata backup
> + * backup and restore class. The backup class is used to backup data for a
> + * list of tables. The restore class is used to restore data from a
> + * previously created nodata backup image.

Hmm, this comment doesn't read well. Please formulate it more adequately to the 
no-data engine. E.g., we don't save any table data with this engine.

> + */
> +class Engine: public Backup_engine
> +{
> +  public:
> +    Engine(THD *t_thd);
> +
> +    /*
> +      Return version of backup images created by this engine.
> +    */
> +    const version_t version() const { return 0; };
> +    result_t get_backup(const uint32, const Table_list &tables, 
> +                        Backup_driver* &drv);
> +    result_t get_restore(const version_t ver, const uint32, const Table_list
> &tables,
> +                         Restore_driver* &drv);
> +
> +    /*
> +     Free any resources allocated by the nodata backup engine.
> +    */
> +    void free() { delete this; }
> +
> +private:
> +    THD *m_thd;
> +};
> +
> +/**
> + * @class Backup
> + *
> + * @brief Contains the nodata backup algorithm backup functionality.
> + *
> + * The backup class is a row-level backup mechanism designed to perform
> + * a table scan on each table reading the rows and saving the data to the
> + * buffer from the backup algorithm.

Above comment doesn't fit to no-data engine...

Btw. Comment format violates our coding guidelines... Ooops! ;)

> + *
> + * @see <backup driver> and <backup thread driver>
> + */
> +class Backup: public Backup_driver
> +{
> +  public:
> +    enum has_data_info { YES, WAIT, EOD };

Would be nice to have documentation for this enum.

> +    Backup(const backup::Table_list &tables):
> +    Backup_driver(tables) { m_driver_state= ND_INITIALIZE; };
> +    virtual ~Backup() {}; 
> +    size_t size()  { return 0; };
> +    size_t init_size() { return 0; };
> +    result_t begin(const size_t) { return backup::OK; };
> +    result_t end() { return backup::OK; };
> +    result_t get_data(Buffer &buf);
> +    result_t lock() { return backup::OK; };
> +    result_t unlock() { return backup::OK; };
> +    result_t cancel() { return backup::OK; };
> +    void free() { delete this; };
> +    result_t prelock() { return backup::OK; };
> +  private:
> +    enum driver_state { ND_INITIALIZE, ND_LOCK, ND_DONE };
> +    driver_state m_driver_state;
> +    bool m_initialized;
> +};
> +
> +/**
> + * @class Restore
> + *
> + * @brief Contains the nodata backup algorithm restore functionality.
> + *
> + * The restore class is a row-level backup mechanism designed to restore
> + * data for each table by writing the data for the rows from the
> + * buffer given by the backup algorithm.
> + *

Above comment is good for the default driver but doesn't fit to no-data driver.

> + * @see <restore driver>
> + */
> +class Restore: public Restore_driver
> +{
> +  public:
> +    Restore(const Table_list &tables, THD *t_thd):
> +        Restore_driver(tables) {};
> +    virtual ~Restore() {};
> +    result_t begin(const size_t) { return backup::OK; };
> +    result_t end() { return backup::OK; };
> +    result_t send_data(Buffer &buf);
> +    result_t cancel() { return backup::OK; };
> +    void free() { delete this; };
> +};
> +} // nodata_backup namespace
> +
> +
> +/*********************************************************************
> +
> +  Nodata snapshot class
> +
> + *********************************************************************/
> +
> +namespace backup {
> +
> +class Logger;
> +
> +class Nodata_snapshot: public Snapshot_info
> +{
> + public:
> +
> +  Nodata_snapshot(Logger&) :Snapshot_info(1) // current version number is 1
> +  {}
> +  Nodata_snapshot(Logger&, const version_t ver) :Snapshot_info(ver)
> +  {}
> +
> +  enum_snap_type type() const
> +  { return NODATA_SNAPSHOT; }
> +
> +  const char* name() const
> +  { return "Nodata"; }
> +
> +  bool accept(const backup::Table_ref &,const storage_engine_ref e)
> +  { 
> +    bool accepted= FALSE;
> +    handlerton *h= se_hton(e);
> +
> +    /*
> +      Accept only nodata engines.
> +    */
> +    switch (h->db_type) {
> +    case DB_TYPE_MRG_MYISAM:
> +    case DB_TYPE_BLACKHOLE_DB:
> +    case DB_TYPE_FEDERATED_DB:
> +    case DB_TYPE_EXAMPLE_DB:
> +      accepted= TRUE;
> +      break;
> +    }
> +    return (accepted);
> +  }; 
> +
> +  result_t get_backup_driver(Backup_driver* &ptr)
> +  { return (ptr= new nodata_backup::Backup(m_tables)) ? OK : ERROR; }
> +
> +  result_t get_restore_driver(Restore_driver* &ptr)
> +  { return (ptr= new nodata_backup::Restore(m_tables,::current_thd)) ? OK : ERROR;
> }
> +
> +  bool is_valid(){ return TRUE; };
> +
> +};
> +
> +} // backup namespace
> +
> +
> +#endif
> +

> diff -Nrup a/sql/backup/image_info.h b/sql/backup/image_info.h
> --- a/sql/backup/image_info.h	2008-04-17 06:44:07 -04:00
> +++ b/sql/backup/image_info.h	2008-04-24 11:08:24 -04:00
> @@ -206,6 +206,8 @@ class Snapshot_info
>    enum enum_snap_type {
>      /** snapshot created by native backup engine. */
>      NATIVE_SNAPSHOT= BI_NATIVE,
> +    /** snapshot created by No data backup driver. */
> +    NODATA_SNAPSHOT= BI_NODATA,
>      /** Snapshot created by built-in, blocking backup engine. */
>      DEFAULT_SNAPSHOT= BI_DEFAULT,
>      /** Snapshot created by built-in CS backup engine. */

> diff -Nrup a/sql/backup/kernel.cc b/sql/backup/kernel.cc
> --- a/sql/backup/kernel.cc	2008-04-17 09:28:22 -04:00
> +++ b/sql/backup/kernel.cc	2008-04-24 11:08:25 -04:00
> @@ -67,6 +67,7 @@
>  #include "be_native.h"
>  #include "be_default.h"
>  #include "be_snapshot.h"
> +#include "be_nodata.h"
>  #include "ddl_blocker.h"
>  #include "backup_progress.h"
>  
> @@ -1008,6 +1009,11 @@ int bcat_reset(st_bstream_image_header *
>                                                                // reports errors
>        break;
>      }
> +
> +    case BI_NODATA:
> +      info->m_snap[n]= new Nodata_snapshot(info->m_ctx, snap->version);
> +                                                              // reports errors
> +      break;
>  
>      case BI_CS:
>        info->m_snap[n]= new CS_snapshot(info->m_ctx, snap->version);

> diff -Nrup a/sql/backup/stream_v1.c b/sql/backup/stream_v1.c
> --- a/sql/backup/stream_v1.c	2008-04-17 06:42:27 -04:00
> +++ b/sql/backup/stream_v1.c	2008-04-24 11:08:26 -04:00
> @@ -421,8 +421,9 @@ int bstream_rd_header(backup_stream *s, 
>    [image type] is encoded as follows:
>  
>    - 0 = snapshot created by native backup driver (BI_NATIVE),
> -  - 1 = snapshot created by built-in blocking driver (BI_DEFAULT),
> -  - 2 = snapshot created using created by built-in driver using consistent
> +  - 1 = snapshot created by built-in no data driver (BI_NODATA),
> +  - 2 = snapshot created by built-in blocking driver (BI_DEFAULT),
> +  - 3 = snapshot created using created by built-in driver using consistent
>        read transaction (BI_CS).
>

Better use 3 for the no-data snapshot. Otherwise we might have to step the 
backup image format version number since we have released the code which 
understands 2 as CS driver.

>    Format of [backup engine info] depends on snapshot type. It is empty for the
> @@ -444,9 +445,10 @@ int bstream_wr_snapshot_info(backup_stre
>  
>    switch (info->type) {
>    case BI_NATIVE:  ret= bstream_wr_byte(s,0); break;
> -  case BI_DEFAULT: ret= bstream_wr_byte(s,1); break;
> -  case BI_CS:      ret= bstream_wr_byte(s,2); break;
> -  default:         ret= bstream_wr_byte(s,3); break;
> +  case BI_NODATA:  ret= bstream_wr_byte(s,1); break;
> +  case BI_DEFAULT: ret= bstream_wr_byte(s,2); break;
> +  case BI_CS:      ret= bstream_wr_byte(s,3); break;
> +  default:         ret= bstream_wr_byte(s,4); break;
>    }
>  
>    CHECK_WR_RES(bstream_wr_int2(s,info->version));
> @@ -493,8 +495,9 @@ int bstream_rd_image_info(backup_stream 
>  
>    switch (type) {
>    case 0: type= BI_NATIVE; break;
> -  case 1: type= BI_DEFAULT; break;
> -  case 2: type= BI_CS; break;
> +  case 1: type= BI_NODATA; break;
> +  case 2: type= BI_DEFAULT; break;
> +  case 3: type= BI_CS; break;
>    default: return BSTREAM_ERROR;
>    }
>  

> diff -Nrup a/sql/backup/stream_v1.h b/sql/backup/stream_v1.h
> --- a/sql/backup/stream_v1.h	2008-04-17 06:42:27 -04:00
> +++ b/sql/backup/stream_v1.h	2008-04-24 11:08:26 -04:00
> @@ -113,6 +113,7 @@ struct st_bstream_engine_info
>  /** Types of table data snapshots. */
>  enum enum_bstream_snapshot_type {
>    BI_NATIVE,  /**< created by native backup driver of a storage engine */
> +  BI_NODATA,  /**< created by built-in driver using no data driver */

I think "created by built-in no data backup driver" will read better.

>    BI_DEFAULT, /**< created by built-in blocking backup driver */
>    BI_CS       /**< created by built-in driver using consistent read transaction
> */
>  };
> diff -Nrup a/storage/example/ha_example.cc b/storage/example/ha_example.cc
> --- a/storage/example/ha_example.cc	2008-02-24 08:12:15 -05:00
> +++ b/storage/example/ha_example.cc	2008-04-24 11:08:27 -04:00
> @@ -138,6 +138,7 @@ static int example_init_func(void *p)
>    example_hton->state=   SHOW_OPTION_YES;
>    example_hton->create=  example_create_handler;
>    example_hton->flags=   HTON_CAN_RECREATE;
> +  example_hton->db_type= DB_TYPE_EXAMPLE_DB;
>  
>    DBUG_RETURN(0);
>  }
> 
>