RE: bk commit into 5.2 tree (cbell:1.2610) BUG#31383

[Chuck Bell]

Hi Andrei, 

> -----Original Message-----
> From: Andrei Elkin [mailto:aelkin@stripped] 
> Sent: Monday, October 22, 2007 4:12 AM
> To: cbell@stripped
> Cc: commits@stripped
> Subject: Re: bk commit into 5.2 tree (cbell:1.2610) BUG#31383
> 
> Chuck, hi.
> 
> I started reviewing but got stumbled at some point which is 
> actually at the very beginning.  Could you please answer to 
> my questions for me to proceed further.

NP

> 
> > Below is the list of changes that have just been committed into a 
> > local
> > 5.2 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, 2007-10-19 18:00:24-04:00, 
> cbell@mysql_cab_desk. +14 -0
> >   BUG#31383 : Consistent Snapshot driver not consistent
> >   
> 
> 
> I think you jump to quickly to what has been done so that I 
> did not get what was the problem.
> 
> Is the idea in to release the kernel from waiting for too 
> long at locking?
> If so what else?

No, actually what happened was the consistent snapshot driver failed to
correctly initiate the consistent read which meant that the consistent
snapshot backed up all records -- even those altered/inserted after the
consistent read transaction was started.

The real problem was the conflict between the requirements of the default
driver and consistent snapshot driver WRT how and when tables are opened --
see the the bug report for more details.

The only way to solve the problem was to run either the default or
consistent snapshot table access portions in a separate thread. I chose the
default driver.

> 
> >   This patch changes the default driver to use a separate thread to
> >   open and lock tables.
> 
> >   The default driver opens tables on the prelock() call from the
> >   kernel. The snapshot
> 
> I am wondering about `prelock' name. What is behind the name 
> is actually the real locking. What more to lock remains by 
> the kernel when your new thread locked all tables?
> 

See the online backup documentation on wiki forge. It is basically a prepare
phase call.


> >   driver initiates the CS read on the lock() call from the 
> kernel and opens tables in the first
> >   call to get_data() after the lock is taken. This is due 
> to the fact that the default driver's
> >   validity point is at open_and_lock_tables() while the 
> snapshot driver's validity point
> >   is at the start of the transaction.
> >
> >   mysql-test/r/backup_snapshot.result@stripped, 2007-10-19 
> 18:00:10-04:00, cbell@mysql_cab_desk. +16 -8
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     New result file.
> >
> >   mysql-test/t/backup_snapshot.test@stripped, 2007-10-19 
> 18:00:10-04:00, cbell@mysql_cab_desk. +23 -4
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Test modified to use the get_lock, release_lock 
> functions to stop the CS driver in
> >     progress allowing simultaneous inserts from the second 
> connection. This patch makes this test
> >     accurately test the CS driver.
> >
> 
> good description, thanks.
> 
> >   sql/backup/CMakeLists.txt@stripped, 2007-10-19 18:00:13-04:00, 
> cbell@mysql_cab_desk. +4 -1
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added the new be_thread source file and dependency for backup.
> >
> >   sql/backup/Makefile.am@stripped, 2007-10-19 18:00:14-04:00, 
> cbell@mysql_cab_desk. +4 -2
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added the new be_thread source file and dependency for backup.
> >
> >   sql/backup/be_default.cc@stripped, 2007-10-19 18:00:14-04:00, 
> cbell@mysql_cab_desk. +65 -0
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added new methods to support using a separate thread to 
> open and lock tables for
> >     backup.
> >
> >   sql/backup/be_default.h@stripped, 2007-10-19 18:00:15-04:00, 
> cbell@mysql_cab_desk. +32 -29
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added new methods to support using a separate thread to 
> open and lock tables for
> >     backup.
> >
> >   sql/backup/be_snapshot.cc@stripped, 2007-10-19 18:00:16-04:00, 
> cbell@mysql_cab_desk. +13 -0
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Modifies the CS driver to open and close its own tables 
> while executing in the
> >     kernel's thread.
> >
> >   sql/backup/be_snapshot.h@stripped, 2007-10-19 18:00:17-04:00, 
> cbell@mysql_cab_desk. +4 -1
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added methods and variables for opening and closing tables.
> >
> >   sql/backup/be_thread.cc@stripped, 2007-10-19 18:00:19-04:00, 
> cbell@mysql_cab_desk. +125 -0
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added new error message for error handling in threads 
> in default and snapshot drivers.
> >
> >   sql/backup/be_thread.cc@stripped, 2007-10-19 18:00:19-04:00, 
> > cbell@mysql_cab_desk. +0 -0
> >
> >   sql/backup/be_thread.h@stripped, 2007-10-19 18:00:20-04:00, 
> cbell@mysql_cab_desk. +35 -0
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     New source file for mutex initialization and helper 
> methods for using a thread to open
> >     and lock tables in default and snapshot drivers.
> >
> >   sql/backup/be_thread.h@stripped, 2007-10-19 18:00:20-04:00, 
> > cbell@mysql_cab_desk. +0 -0
> >
> >   sql/backup/data_backup.cc@stripped, 2007-10-19 
> 18:00:17-04:00, cbell@mysql_cab_desk. +0 -31
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Removed code to call open and lock tables from kernel.
> >
> >   sql/item_func.cc@stripped, 2007-10-19 18:00:11-04:00, 
> cbell@mysql_cab_desk. +3 -1
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added debug_sync_point + lock name to the state of the 
> process list.
> >
> >   sql/share/errmsg.txt@stripped, 2007-10-19 18:00:18-04:00, 
> cbell@mysql_cab_desk. +3 -0
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added new error message for error handling in threads 
> in the default driver.
> >
> >   sql/sql_class.h@stripped, 2007-10-19 18:00:12-04:00, 
> cbell@mysql_cab_desk. +2 -1
> >     BUG#31383 : Consistent Snapshot driver not consistent
> >     
> >     Added a new thread for backup.
> >
> > diff -Nrup a/mysql-test/r/backup_snapshot.result 
> b/mysql-test/r/backup_snapshot.result
> > --- a/mysql-test/r/backup_snapshot.result	2007-07-02 
> 13:42:57 -04:00
> > +++ b/mysql-test/r/backup_snapshot.result	2007-10-19 
> 18:00:10 -04:00
> > @@ -17,15 +17,13 @@ word
> >  SELECT COUNT(*) FROM bup_snapshot.t1;
> >  COUNT(*)
> >  10
> > -con1: Backing up database.
> > +con2: Getting lock on driver.
> > +SELECT get_lock("backup_snapshot", 100); 
> get_lock("backup_snapshot", 
> > +100)
> > +1
> > +con1: Backing up database. Spawn this and continue...
> >  BACKUP DATABASE bup_snapshot TO "bup_snapshot.bak"; -Backup Summary
> > - header     =       22 bytes
> > - meta-data  =       95 bytes
> > - data       =      260 bytes
> > -              --------------
> > - total             377 bytes
> > -con2: Inserting new data.
> > +con2: Wait until backup pauses then insert new data.
> >  INSERT INTO bup_snapshot.t1 VALUES("- Dave Mathews");  INSERT INTO 
> > bup_snapshot.t1 VALUES("- Yes");  INSERT INTO bup_snapshot.t1 
> > VALUES("- Jethro Tull"); @@ -39,6 +37,16 @@ word  SELECT 
> COUNT(*) FROM 
> > bup_snapshot.t1;
> >  COUNT(*)
> >  12
> > +con2: Release lock on driver.
> > +SELECT release_lock("backup_snapshot");
> > +release_lock("backup_snapshot")
> > +1
> > +Backup Summary
> > + header     =       22 bytes
> > + meta-data  =       95 bytes
> > + data       =      260 bytes
> > +              --------------
> > + total             377 bytes
> >  con1: Dropping the database
> >  DROP TABLE bup_snapshot.t1;
> >  con1: Restoring the database
> > diff -Nrup a/mysql-test/t/backup_snapshot.test 
> b/mysql-test/t/backup_snapshot.test
> > --- a/mysql-test/t/backup_snapshot.test	2007-07-02 
> 13:42:57 -04:00
> > +++ b/mysql-test/t/backup_snapshot.test	2007-10-19 
> 18:00:10 -04:00
> > @@ -41,15 +41,29 @@ INSERT INTO bup_snapshot.t1 VALUES ("10 
>  SELECT * 
> > FROM bup_snapshot.t1 WHERE word LIKE '-%';  SELECT COUNT(*) FROM 
> > bup_snapshot.t1;
> >  
> > +connection con2;
> > +
> > +--echo con2: Getting lock on driver.
> > +SELECT get_lock("backup_snapshot", 100);
> > +
> >  # While a consistent snapshot backup is executed,  # no external 
> > inserts should be visible to the transaction.
> >  
> > ---echo con1: Backing up database.
> > -BACKUP DATABASE bup_snapshot TO "bup_snapshot.bak";
> > +connection con1;
> > +
> > +--echo con1: Backing up database. Spawn this and continue...
> > +send BACKUP DATABASE bup_snapshot TO "bup_snapshot.bak";
> >  
> >  connection con2;
> >  
> > ---echo con2: Inserting new data.
> > +--echo con2: Wait until backup pauses then insert new data.
> > +
> > +# Must wait to know when backup has entered lock.
> > +let $wait_condition = SELECT state = "debug_sync_point: 
> backup_snapshot"
> > +                      FROM INFORMATION_SCHEMA.PROCESSLIST
> > +                      WHERE info LIKE "backup database %"; 
> --source 
> > +include/wait_condition.inc
> > +
> >  INSERT INTO bup_snapshot.t1 VALUES("- Dave Mathews");  INSERT INTO 
> > bup_snapshot.t1 VALUES("- Yes");  INSERT INTO bup_snapshot.t1 
> > VALUES("- Jethro Tull"); @@ -59,8 +73,13 @@ DELETE FROM 
> > bup_snapshot.t1 WHERE word L  SELECT * FROM bup_snapshot.t1 
> WHERE word 
> > LIKE '-%';  SELECT COUNT(*) FROM bup_snapshot.t1;
> >  
> > +--echo con2: Release lock on driver.
> > +SELECT release_lock("backup_snapshot");
> > +
> >  connection con1;
> >  
> > +reap;
> > +
> >  # Now restore the database and then check to make sure the 
> new rows  
> > # were not backed up.
> >  
> > @@ -76,6 +95,6 @@ SELECT COUNT(*) FROM bup_snapshot.t1;
> >  
> >  DROP DATABASE bup_snapshot;
> >  
> > ---exec rm $MYSQLTEST_VARDIR/master-data/bup_snapshot.bak
> > +remove_file $MYSQLTEST_VARDIR/master-data/bup_snapshot.bak;
> >  
> >  
> > diff -Nrup a/sql/backup/CMakeLists.txt b/sql/backup/CMakeLists.txt
> > --- a/sql/backup/CMakeLists.txt	2007-10-03 12:56:57 -04:00
> > +++ b/sql/backup/CMakeLists.txt	2007-10-19 18:00:13 -04:00
> > @@ -25,8 +25,11 @@ INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/
> >  SET(BACKUP_SOURCES stream.cc logger.cc string_pool.cc
> >                 archive.cc meta_backup.cc data_backup.cc
> >                 sql_backup.cc be_default.cc buffer_iterator.cc
> > -               be_snapshot.cc)
> > +               be_snapshot.cc be_thread.cc)
> >  
> >  IF(NOT SOURCE_SUBLIBS)
> >    ADD_LIBRARY(backup ${BACKUP_SOURCES})  ENDIF(NOT SOURCE_SUBLIBS)
> > +
> > +ADD_DEPENDENCIES(backup mysys)
> > +
> > diff -Nrup a/sql/backup/Makefile.am b/sql/backup/Makefile.am
> > --- a/sql/backup/Makefile.am	2007-10-05 09:12:28 -04:00
> > +++ b/sql/backup/Makefile.am	2007-10-19 18:00:14 -04:00
> > @@ -33,7 +33,8 @@ libbackup_la_SOURCES = \
> >  	sql_backup.cc \
> >        be_default.cc \
> >        be_snapshot.cc \
> > -      buffer_iterator.cc
> > +      buffer_iterator.cc \
> > +      be_thread.cc
> >  
> >  noinst_HEADERS = \
> >  	api_types.h \
> > @@ -50,7 +51,8 @@ noinst_HEADERS = \
> >      meta_backup.h \
> >        be_default.h \
> >        be_snapshot.h \
> > -      buffer_iterator.h
> > +      buffer_iterator.h \
> > +      be_thread.h
> >  
> >  DEFS = \
> >  	-DMYSQL_SERVER \
> > diff -Nrup a/sql/backup/be_default.cc b/sql/backup/be_default.cc
> > --- a/sql/backup/be_default.cc	2007-07-02 13:42:58 -04:00
> > +++ b/sql/backup/be_default.cc	2007-10-19 18:00:14 -04:00
> > @@ -126,6 +126,55 @@ Backup::Backup(const Table_list &tables,  }
> >  
> >  /**
> > +   @brief Lock tables in driver.
> > +
> > +   This method creates a new THD for use in the new 
> thread. It calls
> > +   the method to open and lock the tables.
> > +  */
> > +void Backup::lock_driver_tables()
> > +{
> > +  DBUG_ENTER("Default_backup::lock_tables");
> > +  lock_thd= create_new_thd();
> > +  lock_thread_tables(&lock_thd, &lock_state, tables_in_backup);
> > +  lock_called= TRUE;
> > +  DBUG_VOID_RETURN;
> > +}
> > +
> > +/**
> > +  * @brief Entry point for the locking thread.
> > +  *
> > +  * The method calls the class method lock_driver_tables which
> > +  * handles the open and locking of tables for the driver.
> > +  */
> > +pthread_handler_t default_thread_for_locking(void *arg) {
> > +  my_thread_init();
> > +  DBUG_ENTER("Default_backup - lock_tables_in_separate_thread");
> > +  (static_cast<Backup *>(arg))->lock_driver_tables();
> > +  my_thread_end();
> > +  pthread_exit(0);
> > +  DBUG_RETURN(0);
> > +}
> > +
> > +/**
> > +  * @brief Prelock call to setup locking.
> > +  *
> > +  * Launches a separate thread ("locking thread") which will lock
> > +  * tables. Locking in a separate thread is needed to have a 
> > +non-blocking
> > +  * prelock() (given that thr_lock() is blocking).
> > +  */
> > +result_t Backup::prelock()
> > +{
> > +  DBUG_ENTER("Default_backup::open_tables");
> > +  lock_state= LOCK_IN_PROGRESS;
> > +  if (pthread_create(&th, &connection_attrib,
> > +                     default_thread_for_locking, this))
> > +    SET_STATE_TO_ERROR_AND_DBUG_RETURN;
> > +  pthread_join(th, 0);
> > +  DBUG_RETURN(backup::OK);
> > +}
> > +
> > +/**
> >    * @brief Start table read.
> >    *
> >    * This method saves the handler for the table and 
> initializes the 
> > @@ -198,6 +247,22 @@ int Backup::next_table()
> >      }
> >    }
> >    DBUG_RETURN(0);
> > +}
> > +
> > +/**
> > + * @brief End backup process.
> > + *
> > + * This method unlocks all of the tables.
> > + *
> > + * @retval backup::OK    all tables unlocked.
> > + */
> > +result_t Backup::end()
> > +{
> > +  DBUG_ENTER("Default_backup::end");
> > +  close_thread_tables(lock_thd);
> > +  net_end(&lock_thd->net);
> > +  delete lock_thd;
> > +  DBUG_RETURN(OK);
> >  }
> >  
> >  /**
> > diff -Nrup a/sql/backup/be_default.h b/sql/backup/be_default.h
> > --- a/sql/backup/be_default.h	2007-07-02 13:42:58 -04:00
> > +++ b/sql/backup/be_default.h	2007-10-19 18:00:15 -04:00
> > @@ -6,6 +6,8 @@
> >  #include "archive.h"
> >  #include "buffer_iterator.h"
> >  #include "backup_aux.h"
> > +#include "mysql_priv.h"
> > +#include "be_thread.h"
> >  
> >  namespace default_backup {
> >  
> > @@ -85,25 +87,27 @@ class Backup: public Backup_driver
> >    public:
> >      enum has_data_info { YES, WAIT, EOD };
> >      Backup(const Table_list &tables, THD *t_thd, 
> thr_lock_type lock_type);
> > -    virtual ~Backup() { backup::free_table_list(all_tables); }; 
> > +    virtual ~Backup() { backup::free_table_list(all_tables); };
> >      size_t size()  { return UNKNOWN_SIZE; };
> >      size_t init_size() { return 0; };
> > -    result_t  begin(const size_t) { return backup::OK; };
> > -    result_t end() { return backup::OK; };
> > +    result_t begin(const size_t) { return backup::OK; };
> > +    result_t end();
> >      result_t get_data(Buffer &buf);
> > -    result_t lock() 
> > -    {
> > -      lock_called= TRUE;
> > -      return backup::OK; 
> > -    };
> > +    result_t lock() { return backup::OK; };
> >      result_t unlock() { return backup::OK; };
> >      result_t cancel() { return backup::OK; };
> >      TABLE_LIST *get_table_list() { return all_tables; }
> >      void free() { delete this; };
> > +    void lock_driver_tables();
> > +    result_t prelock();
> >  
> >   protected:
> > -    my_bool lock_called;           ///< Checks to see if 
> locks have been reached.
> > -    THD *m_thd;                    ///< Pointer to current 
> thread struct.
> > +    my_bool lock_called;            ///< Checks to see if 
> locks have been reached.
> > +    THD *m_thd;                     ///< Pointer to 
> current thread struct.
> > +    TABLE_LIST *tables_in_backup;   ///< List of tables 
> used in backup.
> > +    THD *lock_thd;                  ///< Locking thread pointer
> > +    LOCK_STATE lock_state;          ///< Current locking 
> thread state
> > +    pthread_t th;                   ///< Thread pointer 
> used for open tables
> 
> A name like `lock_thread' would be more informative and 
> connecting to a thread imo.
> 
> 
> >  
> >    private:
> >      /*
> 
> > @@ -112,30 +116,29 @@ class Backup: public Backup_driver
> >        set in the code as a response to conditions or flow of data.
> >      */
> >      typedef enum {
> > -      INITIALIZE,                  ///< Indicates time to 
> initialize read
> > -      GET_NEXT_TABLE,              ///< Open next table in the list
> > -      READ_RCD,                    ///< Reading rows from 
> table mode
> > -      READ_RCD_BUFFER,             ///< Buffer records mode
> > -      CHECK_BLOBS,                 ///< See if record has blobs
> > -      READ_BLOB,                   ///< Reading blobs from 
> record mode
> > -      READ_BLOB_BUFFER             ///< Buffer blobs mode
> > +      INITIALIZE,                   ///< Indicates time to 
> initialize read
> > +      GET_NEXT_TABLE,               ///< Open next table 
> in the list
> > +      READ_RCD,                     ///< Reading rows from 
> table mode
> > +      READ_RCD_BUFFER,              ///< Buffer records mode
> > +      CHECK_BLOBS,                  ///< See if record has blobs
> > +      READ_BLOB,                    ///< Reading blobs 
> from record mode
> > +      READ_BLOB_BUFFER              ///< Buffer blobs mode
> >      } BACKUP_MODE;
> 
> Not to call to restore, but please try to avoid such hunks 
> having only white space difference. You know that simply 
> hurts concentration.
> 
> >  
> >      result_t start_tbl_read(TABLE *tbl);
> >      result_t end_tbl_read();
> >      int next_table();
> > -    BACKUP_MODE mode;              ///< Indicates which 
> mode the code is in
> > -    int tbl_num;                   ///< The index of the 
> current table.
> > -    TABLE *cur_table;              ///< The table 
> currently being read.
> > -    handler *hdl;                  ///< Pointer to table handler.
> > -    uint *cur_blob;                ///< The current blob field.
> > -    uint *last_blob_ptr;           ///< Position of last 
> blob field.
> > -    MY_BITMAP *read_set;           ///< The file read set.
> > -    Buffer_iterator rec_buffer;    ///< Buffer iterator 
> for windowing records
> > -    Buffer_iterator blob_buffer;   ///< Buffer iterator 
> for windowing BLOB fields
> > -    byte *ptr;                     ///< Pointer to blob 
> data from record.
> > -    TABLE_LIST *all_tables;        ///< Reference to list 
> of tables used.
> > -    TABLE_LIST *tables_in_backup;  ///< List of tables 
> used in backup.
> > +    BACKUP_MODE mode;               ///< Indicates which 
> mode the code is in
> > +    int tbl_num;                    ///< The index of the 
> current table.
> > +    TABLE *cur_table;               ///< The table 
> currently being read.
> > +    handler *hdl;                   ///< Pointer to table handler.
> > +    uint *cur_blob;                 ///< The current blob field.
> > +    uint *last_blob_ptr;            ///< Position of last 
> blob field.
> > +    MY_BITMAP *read_set;            ///< The file read set.
> > +    Buffer_iterator rec_buffer;     ///< Buffer iterator 
> for windowing records
> > +    Buffer_iterator blob_buffer;    ///< Buffer iterator 
> for windowing BLOB fields
> > +    byte *ptr;                      ///< Pointer to blob 
> data from record.
> > +    TABLE_LIST *all_tables;         ///< Reference to list 
> of tables used.
> >  };
> >  
> >  /**
> > diff -Nrup a/sql/backup/be_snapshot.cc b/sql/backup/be_snapshot.cc
> > --- a/sql/backup/be_snapshot.cc	2007-07-02 13:42:59 -04:00
> > +++ b/sql/backup/be_snapshot.cc	2007-10-19 18:00:16 -04:00
> > @@ -88,6 +88,7 @@ result_t Backup::end()  {
> >    DBUG_ENTER("Snapshot_backup::end");
> >    end_active_trans(m_thd);
> > +  close_thread_tables(m_thd);
> >    DBUG_RETURN(OK);
> >  }
> >  
> > @@ -112,6 +113,18 @@ result_t Backup::unlock()  {
> >    DBUG_ENTER("Snapshot_backup::unlock()");
> >    DBUG_RETURN(OK);
> > +}
> > +
> > +result_t Backup::get_data(Buffer &buf) {
> > +  if (!tables_open && lock_called)
> > +  {
> > +    open_and_lock_tables(m_thd, tables_in_backup);
> > +    tables_open= TRUE;
> > +  }
> > +  if (lock_called)
> > +    BACKUP_SYNC("backup_snapshot");
> > +  return(default_backup::Backup::get_data(buf));
> >  }
> >  
> >  /**
> > diff -Nrup a/sql/backup/be_snapshot.h b/sql/backup/be_snapshot.h
> > --- a/sql/backup/be_snapshot.h	2007-07-02 13:42:59 -04:00
> > +++ b/sql/backup/be_snapshot.h	2007-10-19 18:00:17 -04:00
> > @@ -59,13 +59,16 @@ class Backup: public default_backup::Bac  {
> >    public:
> >      Backup(const Table_list &tables, THD *t_thd): 
> > -      default_backup::Backup(tables, t_thd, TL_READ) {};
> > +      default_backup::Backup(tables, t_thd, TL_READ) { 
> tables_open= 
> > + FALSE; };
> >      virtual ~Backup() {};
> >      result_t begin(const size_t) { return backup::OK; };
> >      result_t end();
> > +    result_t get_data(Buffer &buf);
> >      result_t prelock() { return backup::READY; }
> >      result_t lock();
> >      result_t unlock();
> > +  private:
> > +    my_bool tables_open;   ///< Indicates if tables are open
> >  };
> >  
> >  /**
> > diff -Nrup a/sql/backup/be_thread.cc b/sql/backup/be_thread.cc
> > --- /dev/null	Wed Dec 31 16:00:00 196900
> > +++ b/sql/backup/be_thread.cc	2007-10-19 18:00:19 -04:00
> > @@ -0,0 +1,125 @@
> > +/* 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 thread methods for online backup.
> > +  *
> > +  * The methods in this class are used to initialize the mutexes
> > +  * for the backup threads. Helper methods are included to make 
> > +thread
> > +  * calls easier for the driver code.
> > +  */
> > +
> > +#include "be_thread.h"
> > +
> > +/**
> > +  *  @brief Creates a new THD object.
> > +  *
> > +  * Creates a new THD object for use in running the open and lock
> > +  * method as a separate thread.
> > +  *
> > +  * @returns Pointer to new THD object or 0 if error.
> > +  */
> > +THD *create_new_thd()
> > +{
> > +  THD *thd;
> > +  DBUG_ENTER("Online backup::Create new THD object");
> > +
> > +  thd= new THD;
> > +  if (unlikely(!thd))
> > +  {
> > +    delete thd;
> > +    DBUG_RETURN(0);
> > +  }
> > +
> > +  thd->thread_stack = (char*)&thd; // remember where our stack is  
> > + pthread_mutex_lock(&LOCK_thread_count);
> > +  thd->thread_id= thread_id++;
> > +  pthread_mutex_unlock(&LOCK_thread_count);
> > +  if (unlikely(thd->store_globals())) // for a proper MEM_ROOT  {
> > +    delete thd;
> > +    DBUG_RETURN(0);
> > +  }
> > +
> > +  thd->init_for_queries(); // opening tables needs a proper LEX
> > +  thd->command= COM_DAEMON;
> > +  thd->system_thread= SYSTEM_THREAD_BACKUP;
> > +  thd->version= refresh_version;
> > +  thd->set_time();
> > +  thd->main_security_ctx.host_or_ip= "";
> > +  thd->client_capabilities= 0;
> > +  thd->mysys_var= 0;
> > +  my_net_init(&thd->net, 0);
> > +  thd->main_security_ctx.master_access= ~0;
> > +  thd->main_security_ctx.priv_user= 0;
> > +  thd->real_id= pthread_self();
> > +  /*
> > +    Making this thread visible to SHOW PROCESSLIST is useful for
> > +    troubleshooting a backup job (why does it stall etc).
> > +  */
> > +  pthread_mutex_lock(&LOCK_thread_count);
> > +  threads.append(thd);
> > +  pthread_mutex_unlock(&LOCK_thread_count);
> > +  lex_start(thd);
> > +  mysql_reset_thd_for_next_command(thd);
> > +  DBUG_RETURN(thd);
> > +}
> > +
> > +/**
> > +  * @brief Creates a validity point by locking all tables.
> > +  *
> > +  * This method calls the open and lock tables for a driver. This 
> > +method
> > +  * is called from a separate thread and therefore can allow 
> > +non-blocking
> > +  * calls to open_and_lock_tables().
> > +  */
> > +void lock_thread_tables(THD **lock_thd,
> > +                        LOCK_STATE *lock_state,
> > +                        TABLE_LIST *tables_in_backup) {
> > +  DBUG_ENTER("Default_backup::lock_tables");
> > +
> > +  /*
> > +    Make sure we have tables to open.
> > +  */
> > +  if (!tables_in_backup)
> > +  {
> > +    DBUG_PRINT("info",("Online backup locking error no 
> tables to lock"));
> > +    *lock_state= LOCK_ERROR;
> > +    DBUG_VOID_RETURN;
> > +  }
> > +
> > +  /*
> > +    As locking tables can be a long operation, we need to support
> > +    cancellability during that time. So we publish our THD 
> to the thread which
> > +    created us.
> > +  */
> > +  if (open_and_lock_tables(*lock_thd, tables_in_backup))  {
> > +    DBUG_PRINT("info",("Online backup locking thread dying"));
> > +    close_thread_tables(*lock_thd);
> > +    DBUG_VOID_RETURN;
> > +  }
> > +
> > +  /*
> > +    Set lock state to acquired.
> > +  */
> > +  DBUG_PRINT("info",("Online backup locking thread got locks"));
> > +  *lock_state= LOCK_ACQUIRED;
> > +  DBUG_VOID_RETURN;
> > +}
> > +
> > diff -Nrup a/sql/backup/be_thread.h b/sql/backup/be_thread.h
> > --- /dev/null	Wed Dec 31 16:00:00 196900
> > +++ b/sql/backup/be_thread.h	2007-10-19 18:00:20 -04:00
> > @@ -0,0 +1,35 @@
> > +#ifndef _BACKUP_THREAD_H
> > +#define _BACKUP_THREAD_H
> > +
> > +#include "../mysql_priv.h"
> > +
> > +/**
> > +   Locking of tables goes through several states.
> > +*/
> > +typedef enum {
> > +  LOCK_NOT_STARTED,
> > +  LOCK_IN_PROGRESS,
> > +  LOCK_ACQUIRED,
> > +  LOCK_ERROR
> > +} LOCK_STATE;
> > +
> > +#define SET_STATE_TO_ERROR_AND_DBUG_RETURN {               
>                   \
> > +    int state= ERROR;                                      
>                  \
> > +    DBUG_PRINT("error",("driver got an error at 
> %s:%d",__FILE__,__LINE__)); \
> > +    DBUG_RETURN(backup::ERROR); }
> > +
> > +/*
> > +  This method a new THD object for the drivers.
> > +*/
> > +THD *create_new_thd();
> > +
> > +/*
> > +  This method is used from a separate thread to allow non-blocking 
> > +calls
> > +  to open_and_lock_tables().
> > +*/
> > +void lock_thread_tables(THD **lock_thd,
> > +                        LOCK_STATE *lock_state,
> > +                        TABLE_LIST *tables_in_backup);
> > +
> > +#endif
> > +
> > diff -Nrup a/sql/backup/data_backup.cc b/sql/backup/data_backup.cc
> > --- a/sql/backup/data_backup.cc	2007-09-11 05:29:29 -04:00
> > +++ b/sql/backup/data_backup.cc	2007-10-19 18:00:17 -04:00
> > @@ -384,9 +384,6 @@ int write_table_data(THD*, Backup_info &
> >  
> >    DBUG_PRINT("backup/data",("initializing scheduler"));
> >  
> > -  TABLE_LIST *table_list= 0;
> > -  TABLE_LIST *table_list_last= 0;
> > -
> >    // add unknown "at end" drivers to scheduler, rest to 
> inactive list
> >  
> >    for (uint no=0; no < info.img_count; ++no) @@ -418,12 
> +415,6 @@ int 
> > write_table_data(THD*, Backup_info &
> >  
> >        inactive.push_back(p);
> >      }
> > -    if (!def_or_snap_used)
> > -      def_or_snap_used=  ((i->type() == 
> Image_info::DEFAULT_IMAGE) ||
> > -                          (i->type() == 
> Image_info::SNAPSHOT_IMAGE));
> > -    if (def_or_snap_used)
> > -      get_default_snapshot_tables(&p->drv(), NULL, 
> > -                                  &table_list, &table_list_last);
> >    }
> >  
> >    /*
> > @@ -508,22 +499,6 @@ int write_table_data(THD*, Backup_info &
> >      if (sch.prepare())
> >        goto error;
> >  
> > -    /*
> > -      Open tables for default and snapshot drivers.
> > -    */
> > -    if (table_list)
> > -    {
> > -      if (open_and_lock_tables(::current_thd, table_list))
> > -      {
> > -        DBUG_PRINT("backup", 
> > -          ( "error on open tables for default and snapshot 
> drivers!" ));
> > -        info.report_error(ER_BACKUP_OPEN_TABLES, "backup");
> > -        DBUG_RETURN(ERROR);
> > -      }
> > -      if (table_list_last)
> > -        table_list_last->next_global= NULL; // break lists
> > -    }
> > -
> >      while (sch.prepare_count > 0)
> >      if (sch.step())
> >        goto error;
> > @@ -548,12 +523,6 @@ int write_table_data(THD*, Backup_info &
> >  
> >      DBUG_PRINT("backup/data",("-- DONE --"));
> >    }
> > -
> > -  /*
> > -    If the default or snapshot drivers are used, close the tables.
> > -  */
> > -  if (def_or_snap_used)
> > -    close_thread_tables(::current_thd);
> >  
> >    info.data_size= s.bytes - start_bytes;
> >  
> > diff -Nrup a/sql/item_func.cc b/sql/item_func.cc
> > --- a/sql/item_func.cc	2007-10-17 09:57:32 -04:00
> > +++ b/sql/item_func.cc	2007-10-19 18:00:11 -04:00
> > @@ -3404,7 +3404,9 @@ void debug_sync_point(const char* lock_n
> >      Structure is now initialized.  Try to get the lock.
> >      Set up control struct to allow others to abort locks
> >    */
> > -  THD_SET_PROC_INFO(thd, "User lock");
> > +  char proc_info[128];
> > +  my_sprintf(proc_info,(proc_info, "debug_sync_point: %s", 
> > + lock_name));  THD_SET_PROC_INFO(thd, proc_info);
> >    thd->mysys_var->current_mutex= &LOCK_user_locks;
> >    thd->mysys_var->current_cond=  &ull->cond;
> >  
> > diff -Nrup a/sql/share/errmsg.txt b/sql/share/errmsg.txt
> > --- a/sql/share/errmsg.txt	2007-10-17 09:57:35 -04:00
> > +++ b/sql/share/errmsg.txt	2007-10-19 18:00:18 -04:00
> > @@ -6195,6 +6195,9 @@ ER_BACKUP_SEND_DATA_RETRY  
> ER_BACKUP_OPEN_TABLES
> >          eng "Open and lock tables failed in %-.64s"
> >  
> > +ER_BACKUP_THREAD_INIT
> > +        eng "Cannot initialize backup threads."
> > +
> >  ER_VIEW_NO_CREATION_CTX
> >    eng "View `%-.64s`.`%-.64s` has no creation context"
> >  ER_VIEW_INVALID_CREATION_CTX
> > diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
> > --- a/sql/sql_class.h	2007-09-13 04:56:30 -04:00
> > +++ b/sql/sql_class.h	2007-10-19 18:00:12 -04:00
> > @@ -936,7 +936,8 @@ enum enum_thread_type
> >    SYSTEM_THREAD_SLAVE_SQL= 4,
> >    SYSTEM_THREAD_NDBCLUSTER_BINLOG= 8,
> >    SYSTEM_THREAD_EVENT_SCHEDULER= 16,
> > -  SYSTEM_THREAD_EVENT_WORKER= 32
> > +  SYSTEM_THREAD_EVENT_WORKER= 32,
> > +  SYSTEM_THREAD_BACKUP= 64
> >  };
> >  
> >  
> >
> >
> > --
> > MySQL Code Commits Mailing List
> > For list archives: http://lists.mysql.com/commits
> > To unsubscribe:    
> http://lists.mysql.com/commits?unsub=1
> >
> 
> regards,
> 
> -- 
> Andrei Elkin, Software Developer, PhD
> MySQL Finland Oy, Helsinki, Finland, www.mysql.com
> 
> Are you MySQL certified?  www.mysql.com/certification
> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    
> http://lists.mysql.com/commits?unsub=1
>