MDEV-9155 Enabling Defragmenting in 10.1.8 still causes OPTIMIZE TABLE to take metadatalocks

take MDL_SHARED_WRITE instead of MDL_SHARED_NO_READ_WRITE
for OPTIMIZE TABLE. For engines that need a stronger
lock (like MyISAM), reopen the table with
MDL_SHARED_NO_READ_WRITE.

mysql-test/suite/handler/innodb.result

@@ -543,7 +543,6 @@ optimize table t1;
 proceed with the normal connection
 handler t1 read next;
 c1
-1
 handler t1 close;
 read the result from the other connection
 Table	Op	Msg_type	Msg_text

mysql-test/suite/innodb/r/defrag_mdl-9155.result

@@ -0,0 +1,15 @@
+set global innodb_defragment=1;
+create table t1 (a int not null primary key auto_increment, b varchar(256), key second(a, b)) engine=innodb;
+insert t1 select null, repeat('a', 256) from seq_1_to_100;
+select count(*) from t1;
+count(*)
+100
+start transaction;
+select count(*) from t1;
+count(*)
+100
+optimize table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	optimize	status	OK
+drop table t1;
+set global innodb_defragment=default;

mysql-test/suite/innodb/t/defrag_mdl-9155.test

@@ -0,0 +1,22 @@
+#
+# MDEV-9155 Enabling Defragmenting in 10.1.8 still causes OPTIMIZE TABLE to take metadatalocks
+#
+source include/have_innodb.inc;
+source include/have_sequence.inc;
+
+set global innodb_defragment=1;
+create table t1 (a int not null primary key auto_increment, b varchar(256), key second(a, b)) engine=innodb;
+insert t1 select null, repeat('a', 256) from seq_1_to_100;
+select count(*) from t1;
+
+connect (con1,localhost,root);
+start transaction;
+select count(*) from t1;
+
+connection default;
+optimize table t1;
+
+connection con1;
+drop table t1;
+
+set global innodb_defragment=default;

sql/handler.h

@@ -248,6 +248,15 @@ enum enum_alter_inplace_result {
  */
 #define HA_CAN_EXPORT                 (1LL << 45)
 
+/*
+  Storage engine does not require an exclusive metadata lock
+  on the table during optimize. (TODO and repair?).
+  It can allow other connections to open the table.
+  (it does not necessarily mean that other connections can
+  read or modify the table - this is defined by THR locks and the
+  ::store_lock() method).
+*/
+#define HA_CONCURRENT_OPTIMIZE          (1LL << 46)
 
 /*
   Set of all binlog flags. Currently only contain the capabilities

sql/sql_admin.cc

@@ -381,9 +381,13 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
       To allow concurrent execution of read-only operations we acquire
       weak metadata lock for them.
     */
-    table->mdl_request.set_type((lock_type >= TL_WRITE_ALLOW_WRITE) ?
-                                MDL_SHARED_NO_READ_WRITE : MDL_SHARED_READ);
+    table->mdl_request.set_type(lex->sql_command == SQLCOM_REPAIR
+                                ? MDL_SHARED_NO_READ_WRITE
+                                : lock_type >= TL_WRITE_ALLOW_WRITE
+                                ? MDL_SHARED_WRITE : MDL_SHARED_READ);
+
     /* open only one table from local list of command */
+    while (1)
     {
       TABLE_LIST *save_next_global, *save_next_local;
       save_next_global= table->next_global;
@@ -483,6 +487,20 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
         result_code= HA_ADMIN_FAILED;
         goto send_result;
       }
+
+      if (!table->table || table->mdl_request.type != MDL_SHARED_WRITE ||
+          table->table->file->ha_table_flags() & HA_CONCURRENT_OPTIMIZE)
+        break;
+
+      trans_rollback_stmt(thd);
+      trans_rollback(thd);
+      close_thread_tables(thd);
+      table->table= NULL;
+      thd->mdl_context.release_transactional_locks();
+      table->mdl_request.init(MDL_key::TABLE, table->db, table->table_name,
+                              MDL_SHARED_NO_READ_WRITE, MDL_TRANSACTION);
+    }
+
 #ifdef WITH_PARTITION_STORAGE_ENGINE
       if (table->table)
       {
@@ -521,7 +539,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
         }
       }
 #endif
-    }
     DBUG_PRINT("admin", ("table: 0x%lx", (long) table->table));
 
     if (prepare_func)
@@ -627,11 +644,13 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
       It only closes instances in other connections, but if this
       connection has LOCK TABLE t1 a READ, t1 b WRITE,
       both t1 instances will be kept open.
-      There is no need to execute this branch for InnoDB, which does
-      repair by recreate.
-      Hence, this code should be executed only for MyISAM engine.
+
+      Note that this code is only executed for engines that request
+      MDL_SHARED_NO_READ_WRITE lock (MDL_SHARED_WRITE cannot be upgraded)
+      by *not* having HA_CONCURRENT_OPTIMIZE table_flag.
     */
-    if (lock_type == TL_WRITE && !table->table->s->tmp_table)
+    if (lock_type == TL_WRITE && !table->table->s->tmp_table &&
+                        table->mdl_request.type > MDL_SHARED_WRITE)
     {
       if (wait_while_table_is_used(thd, table->table,
                                    HA_EXTRA_PREPARE_FOR_RENAME))

storage/innobase/handler/ha_innodb.cc

@@ -2736,7 +2736,7 @@ ha_innobase::ha_innobase(
 	:handler(hton, table_arg),
 	int_table_flags(HA_REC_NOT_IN_SEQ |
 		  HA_NULL_IN_KEY | HA_CAN_VIRTUAL_COLUMNS |
-		  HA_CAN_INDEX_BLOBS |
+		  HA_CAN_INDEX_BLOBS | HA_CONCURRENT_OPTIMIZE |
 		  HA_CAN_SQL_HANDLER |
 		  HA_PRIMARY_KEY_REQUIRED_FOR_POSITION |
 		  HA_PRIMARY_KEY_IN_READ_INDEX |

storage/xtradb/handler/ha_innodb.cc

@@ -3136,7 +3136,7 @@ ha_innobase::ha_innobase(
 	:handler(hton, table_arg),
 	int_table_flags(HA_REC_NOT_IN_SEQ |
 		  HA_NULL_IN_KEY | HA_CAN_VIRTUAL_COLUMNS |
-		  HA_CAN_INDEX_BLOBS |
+		  HA_CAN_INDEX_BLOBS | HA_CONCURRENT_OPTIMIZE |
 		  HA_CAN_SQL_HANDLER |
 		  HA_PRIMARY_KEY_REQUIRED_FOR_POSITION |
 		  HA_PRIMARY_KEY_IN_READ_INDEX |