Re: bk commit into 5.1 tree (mats:1.2559) BUG#36197

[Mats Kindahl]

Konstantin Osipov wrote:
> * Mats Kindahl <mats@stripped> [08/05/09 17:53]:
>
> Good: open_tables() is removed from
> Table_map_event::do_apply_event(), and now all tables are opened
> and locked in Rows_log_event::do_apply_event(), by means of
> simple_open_and_lock_tables(). simple_open_and_lock_tables() has
> an assert that there are no other open/locked tables in the
> thread, thus this code should be change-safe in case someone in
> future breaks the open table cache invariant like the old code
> did.
>
> Now Table_map_event only collects the used tables in the
> table list, and they all are opened at once.
> It's nice that it also simplifies the implementation quite a bit.
>   

Yes. The old code was a result from me originally misunderstanding of 
how the table manipulation was to be used coupled with evolutionary 
changes to how this was handled. As you say, the code is significantly 
simpler.

> The patch is OK to push.
>
>   
>> ChangeSet@stripped, 2008-05-09 15:30:54+02:00, mats@mats-laptop.(none) +8 -0
>>   BUG#36197: flush tables (or little table cache) can cause crash on slave
>>   
>>   When flushing tables, there were a slight chance that the flush was occuring
>>   between processing of two table map events. Since the tables are opened
>>   one by one, it might result in that the tables were not valid and that sub-
>>   sequent locking of tables would cause the slave to crash.
>>   
>>   The problem is solved by opening and locking all tables at once using
>>   simple_open_n_lock_tables(). Also, the patch contain a change to open_tables()
>>   so that pre-locking only takes place when the trg_event_map is zero, which
>>     
>
> is not zero
>   

Good catch. Will fix it.

Thanks!

/Matz

>   
>>   was not the case before (this caused the lock to be placed in
> thd->locked_tables
>>   instead of thd->lock since the assumption was that triggers would be called
>>   later and therefore the tables should be pre-locked).
>>     
>
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com