Re: [Maria-developers] 334e7c9: MDEV-7895 - HANDLER READ doesn't upgrade metadata lock from S to SR
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. 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? - why using S and upgrading to SR "didn't solve the problem" and now it suddenly does? Regards, Sergei
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
participants (2)
-
Sergei Golubchik -
Sergey Vojtovich