Hello Ingo!
* Ingo Strüwing <ingo@stripped> [07/08/29 22:41]:
> Dmitri Lenev, 20.06.2007 21:57:
> > 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.
>
> wait_for_tables() is used in mysql_lock_tables() only. It is called when
> not all locks could be taken and we need to wait for the tables to be
> refreshed before we can try again.
>
> I do not understand what we could replace for reopen_tables(). All
> tables are still open and listed in thd->open_tables, including the
> MERGE children. Tables that need refresh have closed their handlers.
> They need a low-level re-open only. A possible alternative, I could
> think of would be to leave mysql_lock_tables with an error code after
> waiting for refresh, and start a new cycle in open_and_lock_tables().
Actually, open_and_lock_tables() and similar functions already call
mysql_lock_tables() with MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN flag, which
indicates that in cases when wait for lock is aborted, mysql_lock_tables()
should immediately need_reopen parameter to TRUE and return,
instead of calling wait_for_tables().
The main reason for this is that reopen_tables() is unable to properly
reopen tables of a DML statement, since, in a general case, this process
involves rebuilding of pre-locking list for the statement (for example,
after reopen a table might have different triggers).
So the branch of mysql_lock_tables() that involves wait_for_tables()
is already used only in a limited number of cases, particularly, by
ALTER TABLE.
> But this can only be done when mysql_lock_tables() had been called from
> there. It is not an option in other cases. So what do you suggest to
> replace for reopen_tables()?
My suggestion was to get rid of this particular usage of reopen_tables()
(and wait_for_tables() as well). I don't see why always returning a special
error in cases when lock request is aborted is not an option...
AFAIU all places where mysql_lock_tables() is used can be classified as:
*) Places, like open_ltable(), where it is trivial to restart open and
lock tables loop.
*) Places where an attempt to lock a table should never be aborted...
If such an abort happens in such a place, then it is very likely that we
already have a bug in our logic and now this bug is simply hidden by
implicit reopen_tables(). For example, when in CREATE TABLE SELECT
implementation we call mysql_lock_tables() to lock table that we just
have created we assume that this call will immediately succeed (as
we have exclusive name-lock on it). If this attempt to obtain table-level
lock can be aborted then implied call to wait_for_tables() would release
exclusive name-lock on this table making this statement non-atomic.
> Another thing is that I don't see yet, why it would help us. I don't see
> a problem in making reopen_tables() MERGE safe (yet).
>
> Can you please help, before I try the impossible? ;-)
Indeed, it is not impossible. I just think that implementing full support
of merged tables for both uses of reopen_tables() (in wait_for_tables()
and when reopening tables after FLUSH/ALTER under LOCK TABLES) can be more
complex than simply getting rid of one of this scenarios. Note that these
scenarios will probably require different handling of merge tables by
reopen_tables() call. In wait_for_tables()'s case we should successfully
reopen merge table even if number of its underlying tables has increased
whereas in the case when reopen_tables() is called to reopen tables under
LOCK TABLES such such situation may lead to deadlock and therefore error
should be emitted.
> ...
>
> Possible problem: in reopen_tables() we report an error for each failed
> reopen_table(), but we do still take locks on the successfully opened
> tables. Is this required/desirable? Don't we need to abort after
> reopen_tables() anyway?
Hmm... May be the original idea was that in cases when reopen_tables() is
used to reopen tables during the FLUSH/ALTER TABLE under LOCK TABLES it
is better to leave tables for which reopen succeeded open and locked ?
Please don't hesitate to ping me on IRC/drop me an e-mail if some of
above points are too vague and need elaboration!
--
Dmitri Lenev, Software Developer
MySQL AB, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
