Re: bzr commit into mysql-next-mr-rpl-merge branch (alfranio.correia:3202) WL#2775

[Alfranio Correia]

Hi Jasonh,

Thank you for the review.

On 10/27/2010 11:27 AM, He Zhenxing wrote:
> Hi Alfranio,
>
> Patch looks fine, here are some minor comments:
>
>   - Please make sure DBUG_ABORT() need to be changed to DBUG_SUICIDE when
> merging to next-mr-bugfixing

This macro is not available yet at next-mr. I will use it as soon as it is
available.

>   - rli->flush_info() should be removed when simulating crash

The flush is only called when non-transactional changes are executed. We need
this to guarantee that the Slave will be ok.

>   - Comments for Rpl_info_table_access::create_thd() need remove the
> system_thread stuff

Done.

>   - str_schema str_table, I'd suggest remove the str_ prefix from there
> names.

If you don't mind I will keep this. The idea is to differentiate the string
from the TABLE *table.

So please, approve the WL#2775.

Cheers.

>
> Alfranio Correia wrote:
>> Hi all,
>>
>> Please, find a new patch at http://lists.mysql.com/commits/121790.
>> I already pushed the patch to the mysql-next-mr.crash-safe.
>>
>> If there is anything left I will commit diffs.
>>
>> Cheers.
>>
>>
>> On 10/20/2010 11:05 AM, He Zhenxing wrote:
>>> Luís Soares wrote
>>>> On 10/20/2010 10:12 AM, Alfranio Correia wrote:
>>>>> Hi Jasonh, Luis,
>>>>>
>>>>> This is the only thing left to be fixed.
>>>>> As you know I will be off-line today but can you (Luis and Jasonh)
>>>>> discuss and tell
>>>>> me what to do? If not possible, let's have a call tomorrow.
>>>>
>>>> Lets have a quick call tomorrow then.
>>
>> I could not come up with a solution for this.
>>
>> If we try to execute an alter table when the server is starting up, we get a
> segmentation fault.
>> This happens because several structures/objects are not ready at this point.
>> And, the alter table code is too complex to extract the necessary low-level
> functions without
>> the risky of missing anything.
>>
>> So I suggest to do what follows:
>>
>>     PROPOSAL 1:
>>     . Let's keep the code as it and file a bug report that I will be addressed as
> a post-push fix.
>>     . In this bug report we can try three different solutions:
>>       1. Call low-level functions to change the type of the engine thus avoiding
> using the
>>         parser.
>>       2. Figure out if the necessary objects can be created and use the current
> idea.
>>       3. Move the call to another point in the code where the problem does not
> happen.
>>
>>     PROPOSAL 2:
>>     . Try to fix the problem by using either (1), (2) or (3).
>>
>> I cannot promise an ETA and will need help in both proposals.
>>
>> Cheers.
>>
>>>>
>>>
>>> OK
>>>
>>>>>
>>>>> Cheers.
>>>>>
>>>>>
>>>>> On 10/19/2010 06:14 AM, He Zhenxing wrote:
>>>>>> Alfranio Correia wrote:
>>>>>>>> R6. Do we allow changing repo table engine dynamically?
> If not
>>>>>>>>         then I think
> Rpl_info_table::do_is_transactional() no need
>>>>>>>>         to check the table every time, I think this can
> be checked
>>>>>>>>         and saved the result when initializing the info
> table.
>>>>>>>
>>>>>>> Yes. It is possible to change it dynamically. A super user
> can
>>>>>>> always execute "alter table".
>>>>>>>
>>>>>>
>>>>>> I tent to think this is not a good idea to allow repo table
> engine to be
>>>>>> changed at runtime, it might cause unexpected behavior,  so I'd
> strongly
>>>>>> suggest to disable this if you do not have any good use cases for
> this
>>>>>> feature.
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>
>