Re: bk commit - 4.1 tree (istruewing:1.2630) BUG#26379

Hi Dmitri, Konstantin,

Dmitri Lenev wrote:
> Hello Ingo!
>
> * ingo@mysql.com <ingo@mysql.com> [07/05/21 19:55]:

>> ChangeSet@1.2630, 2007-05-21 17:48:22+02:00, istruewing@chilla.local +23 -0
>>   Bug#26379 - Combination of FLUSH TABLE and REPAIR TABLE corrupts
>>               a MERGE table

I committed an intermediate study for the new open approach:

bk commit into 5.1 tree (istruewing:1.2530) BUG#26379 date: 2007-07-13 17:58:57+02:00

This is able to open a merge table successfully. No attempt is made yet to close it, or treat it otherwise.

I have a couple of questions. Please see below.

> 1) myrg_open():
>
> Change this function in such way that it will only read list of
> child tables from the .MRG and associate this list with handler
> or even TABLE object (this still can be done with the help of
> callback function). The rest of the code from this function,
> which assumes that child tables are already open, should be
> moved to separate function which should be called once SQL-layer
> will open them.

Done.

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

> 2) ha_myisammrg::extra():
>
> Introduce a new value of parameter for handler::extra() which will
> inform handler that SQL-layer opened child tables so handler can
> use them. Upon this call handler also may perform those checks
> that we have moved to separate function during the first step.

Done.

> Also we might need a separate parameter value that will tell
> handler that it couldn't use child tables as they are about to be
> closed.

I fail to see why this could be necessary. The closing thread won't use the children during close. Other threads have their own instances open.

> It is probably a good idea to add asserts which will
> ensure that most of ha_myisammrg methods are not called outside
> of these bracketing calls of handler::extra().

The way I understood your suggestions, and the way I implemented them, this seems not necessary. The initial open does not allocate any MERGE object. So there is a NULL MYRG_INFO pointer in ha_myisammrg. Attempts to use it will fail anyway. But again I don't see how this could happen at all. Same reason as with close.

> 3) open_tables()/close_thread_tables() (tentatively):
>
> Introduce code at SQL-layer that for each merge table add its
> child tables to statement's global table list for further opening,
> much in the same way it is done now in mysql_make_view() for views.
> But unlike for views we also have to remove child tables from the
> statement table list to make the process prepared-statement safe.
>
> Two obvious (but not necessarily the best) places for this are
> open_tables() and close_thread_tables(). In the first function we
> can add all child tables for each merge table being opened to the
> statement's table list, when doing this we might want to keep
> pointers to the first and last of these tables associated with
> merge table to simplify their removal afterwards. When we open
> last child table for a merge table we should call handler::extra()
> for this merge table to inform handler that all underlying tables
> are open. Alternatively we can do this for all merge tables at
> the end of open_tables() call.

As you can see from my patch I understood the above so that I should add the children to the table list (next_global) when a merge table is opened, and remove them after the last child has been opened.

Do you need more help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

> During the close_thread_tables()
> we should pass through all merge tables and use pointers stored
> earlier to exclude child tables from the statement's table list.

If we want to finally close the MERGE table, we could take a similar approach as with open (add children to open_tables and mark them "old").

But when we want to release the table to the cache for later reuse by the same or another thread, then we must not do the same with the children. The MERGE storage engine holds references to the MyISAM table objects. So we must assure that the same TABLE objects with their MyISAM objects become children of the MERGE table when it is reused.

The alternative would be to re-reference the MyISAM tables from the MERGE table every time the MERGE table is reused. We must also repeat the compatibility checks, which are costly.

Please advise.

...
> 4) ha_myisammrg::lock_count()/store_lock() or lock_tables():
>
> Change ha_myisammrg::lock_count() and store_lock() to do nothing
> and rely on SQL-layer locking child tables. Another possible
> alternative is to change lock_tables() to ignore child tables.

IMHO we do not need to change anything here. We took the children off the table list. So lock_tables() won't handle them directly.

...
> 6) reopen_table(), reopen_tables():
>
> Remove those usages of reopen_table()/reopen_tables() which are not
> associated with reopening table under LOCK TABLES (i.e. get rid of
> its usage within wait_for_tables()). This will simplify changing of
> reopen_table() in such way that it will safely handle merge tables.
> In particular I think we should disallow situations when we will
> reopen merge tables in place of non-merge tables or merge tables
> with different child tables (this can lead to deadlock).

Can we help you?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

This may be the appropriate place to raise the question: How to handle MERGE tables when a child is refreshed? In this case we must at least re-reference this child and check its compatibility. If we don't want to over-stress extra(), then we should do this by re-opening the whole MERGE table.

Anyway we need to know about the parent-child relationship in the table cache. Unless we re-open the MERGE table on every reuse as I mentioned above.

...

Regards
Ingo

-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140

-- 
MySQL Code Commits Mailing List
For list archives: 
http://lists.mysql.com/commits
To unsubscribe:    
http://lists.mysql.com/commits?unsub=lists@pantek.com