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

[He Zhenxing]

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
 - rli->flush_info() should be removed when simulating crash
 - Comments for Rpl_info_table_access::create_thd() need remove the
system_thread stuff
 - str_schema str_table, I'd suggest remove the str_ prefix from there
names.

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