MDEV-28930 ALTER TABLE Deadlocks with parallel TL_WRITE

ALTER ONLINE TABLE acquires table with TL_READ. Myisam normally acquires
TL_WRITE for DML, which makes it hang until table is freed.

We deadlock once ALTER upgrades its MDL lock.

Solution:
Unlock table earlier. We don't need to hold TL_READ once we finished
copying. Relay log replication requires no data locks on `from` table.

5 files changed, 65 insertions, 9 deletions

diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result
index 019e70eec10..79daab9f689 100644
--- a/mysql-test/main/alter_table_online_debug.result
+++ b/mysql-test/main/alter_table_online_debug.result
@@ -579,6 +579,27 @@ drop table t1;
 drop table t2;
 drop table t3;
 #
+# MDEV-28930 ALTER TABLE Deadlocks with parallel TL_WRITE
+#
+create table t1(a int) engine=myisam select 1;
+set debug_sync='alter_table_online_before_lock SIGNAL ready WAIT_FOR go_for_locking';
+alter table t1 force;
+connection con2;
+set debug_sync='now WAIT_FOR ready';
+set debug_sync='thr_multi_lock_before_thr_lock SIGNAL go_for_locking';
+update t1 set a=2;
+connection default;
+set debug_sync='alter_table_online_before_lock SIGNAL ready WAIT_FOR go_for_locking';
+alter table mysql.global_priv force;
+connection con2;
+set debug_sync='now WAIT_FOR ready';
+set debug_sync='thr_multi_lock_before_thr_lock SIGNAL go_for_locking';
+create user user1@localhost;
+connection default;
+set debug_sync=reset;
+drop user user1@localhost;
+drop table t1;
+#
 # MDEV-28959 Online alter ignores strict table mode
 #
 create table t1 (a int);
diff --git a/mysql-test/main/alter_table_online_debug.test b/mysql-test/main/alter_table_online_debug.test
index d3a9a581ab3..285de17600a 100644
--- a/mysql-test/main/alter_table_online_debug.test
+++ b/mysql-test/main/alter_table_online_debug.test
@@ -718,6 +718,39 @@ drop table t2;
 drop table t3;
 
 --echo #
+--echo # MDEV-28930 ALTER TABLE Deadlocks with parallel TL_WRITE
+--echo #
+create table t1(a int) engine=myisam select 1;
+
+set debug_sync='alter_table_online_before_lock SIGNAL ready WAIT_FOR go_for_locking';
+--send
+alter table t1 force;
+
+--connection con2
+set debug_sync='now WAIT_FOR ready';
+set debug_sync='thr_multi_lock_before_thr_lock SIGNAL go_for_locking';
+update t1 set a=2;
+
+--connection default
+--reap
+
+set debug_sync='alter_table_online_before_lock SIGNAL ready WAIT_FOR go_for_locking';
+--send
+alter table mysql.global_priv force;
+
+--connection con2
+set debug_sync='now WAIT_FOR ready';
+set debug_sync='thr_multi_lock_before_thr_lock SIGNAL go_for_locking';
+create user user1@localhost;
+
+--connection default
+--reap
+
+set debug_sync=reset;
+drop user user1@localhost;
+drop table t1;
+
+--echo #
 --echo # MDEV-28959 Online alter ignores strict table mode
 --echo #
 create table t1 (a int);
diff --git a/mysql-test/suite/perfschema/r/alter_table_progress.result b/mysql-test/suite/perfschema/r/alter_table_progress.result
index 010fcd9c24f..13245f60f29 100644
--- a/mysql-test/suite/perfschema/r/alter_table_progress.result
+++ b/mysql-test/suite/perfschema/r/alter_table_progress.result
@@ -77,17 +77,15 @@ stage/sql/table lock	NULL	NULL
 stage/sql/After create	NULL	NULL
 stage/sql/copy to tmp table	5	5
 stage/sql/Enabling keys	NULL	NULL
+stage/sql/Unlocking tables	NULL	NULL
+stage/sql/Enabling keys	NULL	NULL
 stage/sql/Apply log event	NULL	NULL
 stage/sql/After apply log event	NULL	NULL
 stage/sql/Rename result table	NULL	NULL
-stage/sql/Unlocking tables	NULL	NULL
-stage/sql/Rename result table	NULL	NULL
 stage/sql/End of update loop	NULL	NULL
 stage/sql/Query end	NULL	NULL
 stage/sql/Commit	NULL	NULL
 stage/sql/closing tables	NULL	NULL
-stage/sql/Unlocking tables	NULL	NULL
-stage/sql/closing tables	NULL	NULL
 stage/sql/Commit implicit	NULL	NULL
 stage/sql/Starting cleanup	NULL	NULL
 stage/sql/Freeing items	NULL	NULL
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
index 52f82dde263..1ed50f71d1f 100644
--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -5743,12 +5743,11 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
   thd->set_query_timer();
 
   /*
-    If there is no locks taken, this is the first binrow event seen
-    after the table map events.  We should then lock all the tables
-    used in the transaction and proceed with execution of the actual
-    event.
+    If there are no tables open, this must be the first row event seen
+    after the table map events. We should then open and lock all tables
+    used in the transaction and proceed with execution of the actual event.
   */
-  if (!thd->lock)
+  if (!thd->open_tables)
   {
     /*
       Lock_tables() reads the contents of thd->lex, so they must be
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index f852af4a7cc..4e98ab36221 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -12391,6 +12391,11 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     // We restore bitmaps, because update event is going to mess up with them.
     to->default_column_bitmaps();
 
+    end_read_record(&info);
+    init_read_record_done= false;
+    mysql_unlock_tables(thd, thd->lock);
+    thd->lock= NULL;
+
     error= online_alter_read_from_binlog(thd, &rgi, binlog);
 
     DEBUG_SYNC(thd, "alter_table_online_before_lock");