Rafal,
All changes made with the following exceptions.
See new patch:
http://lists.mysql.com/commits/46693
Chuck
> > + if (hdl)
> > + end_tbl_read();
> > + if (locking_thd)
> > + locking_thd->kill_locking_thread();
> > + DBUG_RETURN(OK);
>
> Why close_thread_tables() is not called here as it is in the
> CS driver?
Because the default driver uses the be_thread class to open and lock tables
in a separate thread. The CS driver does not -- it opens and closes its own
tables. This was necessary because of the unique requirements of using a
transaction -- it must be handled in the same thread as the tables in the
transaction are referenced.
> Why you don't use kill_locking_thread() method as it is done
> in the default
> driver? This method is carefully written to avoid race
> conditions and other
> synchronisation issues and I don't think it can be replaced
> by such simple
> statement.
See above.
>
> Is it correct to kill the table locking thread before the
> operations below are
> performed, for example closing the consistent read transaction?
The locking thread isn't used here.
>
> > + if (m_trans_start)
> > + {
> > + ha_autocommit_or_rollback(locking_thd->m_thd, 0);
> > + end_active_trans(locking_thd->m_thd);
> > + m_trans_start= FALSE;
> > + }
> > + if (tables_open)
> > + {
> > + if (hdl)
> > + default_backup::Backup::end_tbl_read();
> > + close_thread_tables(locking_thd->m_thd);
>
> Why close_thrad_tables() is called here but not in the default driver?
See above.
>
> > + tables_open= FALSE;
> > + }
> > + DBUG_RETURN(OK);
> > +}
>
> Class snapshot_backup::Backup inherits from
> default_backup::Backup. Thus it
> would make sense if snapshot_backup::Backup::cleanup() calls
> default_backup::Backup::cleanup(). This way, whenever we add
> something to
> cleanup procedure of the default backup driver we won't
> forget to do it also in
> the CS driver. Of course, the CS driver must do some
> additional cleanup actions
> like ending the transactions and then call the inherited
> cleanup() method.
The cleanup() methods have to be different for the CS and default driver
since one uses the locking thread and the other has a transaction. See
above.
> > private:
> > my_bool tables_open; ///< Indicates if tables are open
> > + my_bool m_cancel; ///< Cancel backup
>
> Why m_cancel flag if we have CANCEL mode?
Because this cancel is for controlling the CS driver code. The variable mode
is private to the default driver. The CS driver has its own cleanup() method
that is different from the default driver's cancel() method.
