Re: [Maria-developers] [Commits] 922f970: MDEV-12882 - Assertion failed in MDL_context::upgrade_shared_lock
Hi, Sergey! On Jun 22, Sergey Vojtovich wrote:
revision-id: 922f9700a7fb084e3f7d6cff56e27e24fc603ed7 (mariadb-10.2.6-58-g922f970) parent(s): 62021f391a42c5577190aa43cb8ad91e56235b46 committer: Sergey Vojtovich timestamp: 2017-06-22 17:35:36 +0400 message:
MDEV-12882 - Assertion failed in MDL_context::upgrade_shared_lock
Relaxed assertion (in MySQL it was removed). For "LOCK TABLES t1 WRITE CONCURRENT, t1 READ" upgrade lock to weakest existing suitable lock, which is MDL_SHARED_NO_READ_WRITE.
diff --git a/mysql-test/t/mdl.test b/mysql-test/t/mdl.test new file mode 100644 index 0000000..5a74ae5 --- /dev/null +++ b/mysql-test/t/mdl.test @@ -0,0 +1,15 @@
Hm. I'd totally expect main.mdl test to exist for years :)
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 412cd1d..7c910e8 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2787,6 +2787,7 @@ static bool lock_tables_open_and_lock_tables(THD *thd, TABLE_LIST *tables) ! table->prelocking_placeholder && table->table->file->lock_count() == 0) { + enum enum_mdl_type lock_type; /* In case when LOCK TABLE ... READ LOCAL was issued for table with storage engine which doesn't support READ LOCAL option and doesn't @@ -2799,9 +2800,12 @@ static bool lock_tables_open_and_lock_tables(THD *thd, TABLE_LIST *tables) deadlock_handler.init(); thd->push_internal_handler(&deadlock_handler);
+ lock_type= table->table->mdl_ticket->get_type() == MDL_SHARED_WRITE ? + MDL_SHARED_NO_READ_WRITE : MDL_SHARED_READ_ONLY; + bool result= thd->mdl_context.upgrade_shared_lock( table->table->mdl_ticket, - MDL_SHARED_READ_ONLY, + lock_type, thd->variables.lock_wait_timeout);
This only works if you lock WRITE, then READ. If you do it the other way around, you'll get an error. Isn't it inconsistent? Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Sergei, On Thu, Jun 22, 2017 at 07:57:21PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On Jun 22, Sergey Vojtovich wrote:
revision-id: 922f9700a7fb084e3f7d6cff56e27e24fc603ed7 (mariadb-10.2.6-58-g922f970) parent(s): 62021f391a42c5577190aa43cb8ad91e56235b46 committer: Sergey Vojtovich timestamp: 2017-06-22 17:35:36 +0400 message:
MDEV-12882 - Assertion failed in MDL_context::upgrade_shared_lock
Relaxed assertion (in MySQL it was removed). For "LOCK TABLES t1 WRITE CONCURRENT, t1 READ" upgrade lock to weakest existing suitable lock, which is MDL_SHARED_NO_READ_WRITE.
diff --git a/mysql-test/t/mdl.test b/mysql-test/t/mdl.test new file mode 100644 index 0000000..5a74ae5 --- /dev/null +++ b/mysql-test/t/mdl.test @@ -0,0 +1,15 @@
Hm. I'd totally expect main.mdl test to exist for years :) Me too. I guess in most cases tests for MDL need debug_sync, thus most tests went into mdl_sync.test.
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 412cd1d..7c910e8 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2787,6 +2787,7 @@ static bool lock_tables_open_and_lock_tables(THD *thd, TABLE_LIST *tables) ! table->prelocking_placeholder && table->table->file->lock_count() == 0) { + enum enum_mdl_type lock_type; /* In case when LOCK TABLE ... READ LOCAL was issued for table with storage engine which doesn't support READ LOCAL option and doesn't @@ -2799,9 +2800,12 @@ static bool lock_tables_open_and_lock_tables(THD *thd, TABLE_LIST *tables) deadlock_handler.init(); thd->push_internal_handler(&deadlock_handler);
+ lock_type= table->table->mdl_ticket->get_type() == MDL_SHARED_WRITE ? + MDL_SHARED_NO_READ_WRITE : MDL_SHARED_READ_ONLY; + bool result= thd->mdl_context.upgrade_shared_lock( table->table->mdl_ticket, - MDL_SHARED_READ_ONLY, + lock_type, thd->variables.lock_wait_timeout);
This only works if you lock WRITE, then READ. If you do it the other way around, you'll get an error. Isn't it inconsistent?
Well, it works the other way around too. But in this case connection holds two locks: READ_ONLY and WRITE. Which is even better than NO_READ_WRITE, but I thought connection never acquires more than one lock for a table. Apparently number of locks acquired may depends on the oreder they were requested. I added "reverse" test to my patch. Thanks, Sergey
participants (2)
-
Sergei Golubchik -
Sergey Vojtovich