Hi Sergei, On Thu, Dec 15, 2016 at 04:38:04PM +0100, Sergei Golubchik wrote:
Hi, Sergey!
On Dec 15, Sergey Vojtovich wrote:
+ /* An upgradable shared metadata lock which blocks all attempts to update table data, allowing reads. A connection holding this kind of lock can read table metadata and read diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 1ca7e9c..d3f4517 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1496,6 +1496,7 @@ bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx) to general/slow log would be disabled in read only transactions. */ if (table_list->mdl_request.type >= MDL_SHARED_WRITE && + table_list->mdl_request.type != MDL_SHARED_READ_ONLY &&
if you're going to keep read and write locks mixed, then I'd suggest to move this condition into a small inline helper, like table_list->mdl_request.is_write_lock(). it's used few times already in this very file and there will be only one place to change if we'll need to add, say, MDL_SHARED_WRITE_LOW_PRIO I can merge this from MySQL, it is called is_write_lock_request() there. In fact MySQL didn't have to change this condition for MDL_SHARED_WRITE_LOW_PRIO.
I think it'd be good. There were three or four places in sql_base.cc where it should replace the >= check. Ok.
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 6dc9192..0d740bb 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc ... + thd->pop_internal_handler(); + + if (deadlock_handler.need_reopen()) + { + /* + Deadlock occurred during upgrade of metadata lock. + Let us restart acquring and opening tables for LOCK TABLES.
will you use MDL_SHARED_READ_ONLY right away when retrying? or it'll do the upgrade thing again? Upgrade thing again, which is farily stupid. That's how it was in original code and I didn't change it in a hurry.
Can you do it? It's probably just updating the lock in the corresponding table_list entry.
Still, better do it as a separate change, after you push this MDEV-11227 commit, so that 10.2.3 wouldn't have to wait for it.
Ok. Should I consider this patch is approved? It is possible to push it now, but taking into account complexity and bb state: should we postpone it until bb is greener? Regards, Sergey