maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

assuming we're heading towards "the InnoDB solution" this patch is not that
relevant anymore. Answers inline.

On Thu, May 07, 2015 at 09:09:30PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On May 07, Sergey Vojtovich wrote:
> > revision-id: 334e7c9bccbc6892efbb35f5930f80ccf1d770a8
> > parent(s): cbc5157feb9801310e458f7ed10983ad478c881e
> > committer: Sergey Vojtovich
> > branch nick: mariadb
> > timestamp: 2015-04-01 14:07:02 +0400
> > message:
> > 
> > MDEV-7895 - HANDLER READ doesn't upgrade metadata lock from S to SR
> > 
> > Change code for HANDLER READ statements to upgrade S metadata lock to
> > SR metadata lock for the duration of read. This allows us properly
> > isolate HANDLER READ from LOCK TABLES WRITE and makes metadata locking
> > for these statements consistent with locking for other DML.
> > 
> > HANDLER-related tests had to be adjusted to take into account that
> > HANDLER READ will wait for and acquire SR lock.
> 
> code-wise it's ok, but I'm not sure I like the resulting behavior. Here,
> the very first test:
> 
>   con1: handler t1 open;
>   con2: drop table t1;    -- waits
>   con1: handler t1 read;  -- deadlock
> 
> as a user I'd expect that after handler open, I can use the handler. I
> don't care about locks, but supposedly handler open should take them all
> or handler read should solve it somehow, why I, a user, should bother?
> 
> So, indeed, one option would be to acquire SR lock on handler open.
In MySQL "drop table" waits too (uhm, why?), but "handler read" fails with
"table doesn't exist" and drop completes afterwards.

In MariaDB "drop" for some reason didn't abort handler lock. I tend to remember
I debugged this mechanism at that time and found it somewhat broken. I might
have fixed this in another pre-requisite patch.

> 
> Also, note the comment for MDL_SHARED:
> 
>   /*
>     A shared metadata lock.
>     To be used in cases when we are interested in object metadata only
>     and there is no intention to access object data (e.g. for stored
>     routines or during preparing prepared statements).
>     We also mis-use this type of lock for open HANDLERs, since lock
>     acquired by this statement has to be compatible with lock acquired
>     by LOCK TABLES ... WRITE statement, i.e. SNRW (We can't get by by
>     acquiring S lock at HANDLER ... OPEN time and upgrading it to SR
>     lock for HANDLER ... READ as it doesn't solve problem with need
>     to abort DML statements which wait on table level lock while having
>     open HANDLER in the same connection).
>     To avoid deadlock which may occur when SNRW lock is being upgraded to
>     X lock for table on which there is an active S lock which is owned by
>     thread which waits in its turn for table-level lock owned by thread
>     performing upgrade we have to use thr_abort_locks_for_thread()
>     facility in such situation.
>     This problem does not arise for locks on stored routines as we don't
>     use SNRW locks for them. It also does not arise when S locks are used
>     during PREPARE calls as table-level locks are not acquired in this
>     case.
>   */
>   MDL_SHARED,
> 
> Which begs more questions:
> 
>  - why open handler lock has to be compatible with SNRW?
Because handler open as such doesn't intend to access table data?

> 
>  - why using S and upgrading to SR "didn't solve the problem" and now it
>    suddenly does?
Frankly speaking I failed to parse this statement.

Thanks,
Sergey