Rafal,
Ok, I understand now. Patch approved.
Chuck
> -----Original Message-----
> From: Rafal Somla [mailto:rsomla@stripped]
> Sent: Monday, March 31, 2008 9:44 AM
> To: Chuck Bell
> Cc: commits@stripped; 'Ingo Strüwing'
> Subject: Re: bk commit into 6.0 tree (rafal:1.2606) BUG#34721
>
> Chuck,
>
> Chuck Bell wrote:
> > Rafal,
> >
> > The patch looks good. The test is exactly what I was
> looking for except...
> >
> > I think we should change the use of MyISAM for this test.
> The MyISAM
> > native driver is a certainty (regardless of timing).
> However, if you
> > used the memory engine it is unlikely that we would write a
> native engine for it.
> > Thus we would not have to change anything once this patch is pushed.
> >
>
> I've chosen MyISAM exactly because it will have a native
> driver soon. The reason is that only for a storage engine
> with native backup, there will be a difference in the choice
> of backup driver between the normal operation and when the
> test code is injected. That is,
>
> 1. In normal operation (without injection) a native backup
> driver will be used.
>
> 2. With injected code, MyISAM will not create the native
> backup engine and therefore the default backup driver will be used.
>
> Note that if a storage engine has no native backup, then in
> both cases (with and without code injection) the default (or
> CS) backup driver will be used. This is the case now.
>
> Thus the test case is more a clear-cut if the engine used
> *has* native backup.
>
> Rafal
>
> > Chuck
> >
> >> -----Original Message-----
> >> From: Rafal Somla [mailto:rsomla@stripped]
> >> Sent: Monday, March 31, 2008 7:32 AM
> >> To: Chuck Bell
> >> Cc: commits@stripped; Ingo Strüwing
> >> Subject: Re: bk commit into 6.0 tree (rafal:1.2606) BUG#34721
> >>
> >> Hi Chuck,
> >>
> >> I added a test case for testing the backup engine selection logic.
> >> The new patch is <http://lists.mysql.com/commits/44675>
> >>
> >> Rafal
> >>
> >> Chuck Bell wrote:
> >>> Rafal,
> >>>
> >>> I think we need to add a test for this using
> DBUG_EXECUTE_IF(). It
> >>> will help us detect problems when new native drivers are
> >> added and/or
> >>> some collateral errors occur based changes to the server or
> >> backup code.
> >>> Chuck
> >>>
> >>>> -----Original Message-----
> >>>> From: rsomla@stripped [mailto:rsomla@stripped]
> >>>> Sent: Tuesday, March 25, 2008 10:22 AM
> >>>> To: commits@stripped
> >>>> Subject: bk commit into 6.0 tree (rafal:1.2606) BUG#34721
> >>>>
> >>>> Below is the list of changes that have just been
> committed into a
> >>>> local 6.0 repository of rafal. When rafal 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-03-25 15:22:11+01:00,
> >> rafal@quant.(none) +2 -0
> >>>> BUG#34721 (Backup driver can't refuse to provide driver):
> >>>>
> >>>> This patch modifies backup kernel so that it falls back
> >> to built in
> >>>> backup
> >>>> engines in case a native backup engine can not be created.
> >>>>
> >>>> This is for the special case when storage engine defines a
> >>>> get_backup_engine()
> >>>> factory function (inside the handlerton), but that
> >> function fails
> >>>> to create a
> >>>> backup engine instance. Previous code haven't dealt
> >> correctly with
> >>>> that case.
> >>>> Now it puts a warning and tries the built-in engines.
> >>>>
> >>>> sql/backup/backup_info.cc@stripped, 2008-03-25 15:22:04+01:00,
> >>>> rafal@quant.(none) +10 -9
> >>>> In case a native backup engine has been selected for a given
> >>>> table, check if
> >>>> created Native_snapshot object is valid (in particular, has
> >>>> successfully created
> >>>> the engine). Only then use it for that table,
> >> otherwise try other
> >>>> backup engines
> >>>> including the built-in ones.
> >>>>
> >>>> sql/backup/be_native.h@stripped, 2008-03-25 15:22:05+01:00,
> >>>> rafal@quant.(none) +1 -1
> >>>> If the get_backup_engine() factory function defined by
> >> a storage
> >>>> engine failes
> >>>> to create a native backup engine don't treat it as a
> >>>> (fatal) error. In that case
> >>>> we will try to use default, built-in backup engines.
> Thus only
> >>>> warning is printed.
> >>>>
> >>>> diff -Nrup a/sql/backup/backup_info.cc
> b/sql/backup/backup_info.cc
> >>>> --- a/sql/backup/backup_info.cc 2008-03-21 09:43:43 +01:00
> >>>> +++ b/sql/backup/backup_info.cc 2008-03-25 15:22:04 +01:00
> >>>> @@ -83,16 +83,17 @@ Backup_info::find_backup_engine(const ba
> >>>> {
> >>>> Native_snapshot *nsnap= new Native_snapshot(m_ctx, se);
> >>>> DBUG_ASSERT(nsnap);
> >>>> - snapshots.push_front(nsnap);
> >>>> - native_snapshots.insert(se, nsnap);
> >>>>
> >>>> - /*
> >>>> - Question: Can native snapshot for a given storage
> >>>> engine not accept
> >>>> - a table using that engine? If yes, then what to do
> >>>> in that case - error
> >>>> - or try other (default) snapshots?
> >>>> - */
> >>>> - DBUG_ASSERT(nsnap->accept(tbl, se));
> >>>> - snap= nsnap;
> >>>> + if (nsnap->is_valid())
> >>>> + {
> >>>> + snapshots.push_front(nsnap);
> >>>> + native_snapshots.insert(se, nsnap);
> >>>> +
> >>>> + if (nsnap->accept(tbl, se))
> >>>> + snap= nsnap;
> >>>> + }
> >>>> + else
> >>>> + delete nsnap;
> >>>> }
> >>>>
> >>>> /*
> >>>> diff -Nrup a/sql/backup/be_native.h b/sql/backup/be_native.h
> >>>> --- a/sql/backup/be_native.h 2008-03-04 17:06:22 +01:00
> >>>> +++ b/sql/backup/be_native.h 2008-03-25 15:22:05 +01:00
> >>>> @@ -97,7 +97,7 @@ int Native_snapshot::init(Logger &log, c
> >>>> if (m_be)
> >>>> m_be->free();
> >>>> m_be= NULL;
> >>>> - log.report_error(ER_BACKUP_CREATE_BE, m_name);
> >>>> + log.report_error(log_level::WARNING,
> >>>> ER_BACKUP_CREATE_BE, m_name);
> >>>> return 1;
> >>>> }
> >>>>
> >>>>
> >>>> --
> >>>> MySQL Code Commits Mailing List
> >>>> For list archives: http://lists.mysql.com/commits
> >>>> To unsubscribe:
> >>>> http://lists.mysql.com/commits?unsub=1
> >>>>
> >>>
> >
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>