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
>>>>
>>>
>
