MDEV-22227 Assertion `state_ == s_exec' failed in wsrep::client_state::start_transaction

Removed redundant code for BF abort transaction in `thr_lock.cc`.

TOI operations will ignore provided lock_wait_timeout and use `LONG_TIMEOUT`
until operation is finished.

Reviewed-by: Jan Lindström <jan.lindstrom@mariadb.com>

mysql-test/suite/galera/r/MDEV-22227.result

@@ -0,0 +1,16 @@
+connection node_2;
+connection node_1;
+CREATE TABLE t1 (a INT) ENGINE=MyISAM;
+LOCK TABLE t1 WRITE CONCURRENT;
+connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+connection node_1a;
+SET lock_wait_timeout= 1;
+CREATE VIEW v1 AS SELECT * FROM t1;
+connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+connection node_1b;
+SET SESSION wsrep_sync_wait = 0;
+connection node_1;
+UNLOCK TABLES;
+connection node_1a;
+DROP VIEW v1;
+DROP TABLE t1;

mysql-test/suite/galera/t/MDEV-22227.test

@@ -0,0 +1,26 @@
+--source include/galera_cluster.inc
+--source include/have_log_bin.inc
+
+CREATE TABLE t1 (a INT) ENGINE=MyISAM;
+LOCK TABLE t1 WRITE CONCURRENT;
+
+--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
+--connection node_1a
+# TOI operations will ignore lock_wait_timeout
+SET lock_wait_timeout= 1;
+--send CREATE VIEW v1 AS SELECT * FROM t1
+
+--connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1
+--connection node_1b
+SET SESSION wsrep_sync_wait = 0;
+--let $wait_condition = SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE STATE = 'Waiting for table level lock'
+--source include/wait_condition.inc
+
+--connection node_1
+UNLOCK TABLES;
+
+--connection node_1a
+--reap
+
+DROP VIEW v1;
+DROP TABLE t1;

mysys/thr_lock.c

@@ -95,24 +95,6 @@ my_bool thr_lock_inited=0;
 ulong locks_immediate = 0L, locks_waited = 0L;
 enum thr_lock_type thr_upgraded_concurrent_insert_lock = TL_WRITE;
 
-#ifdef WITH_WSREP
-static wsrep_thd_is_brute_force_fun wsrep_thd_is_brute_force= NULL;
-static wsrep_abort_thd_fun wsrep_abort_thd= NULL;
-static my_bool wsrep_debug;
-static my_bool wsrep_convert_LOCK_to_trx;
-static wsrep_on_fun wsrep_on = NULL;
-
-void wsrep_thr_lock_init(
-    wsrep_thd_is_brute_force_fun bf_fun, wsrep_abort_thd_fun abort_fun,
-    my_bool debug, my_bool convert_LOCK_to_trx, wsrep_on_fun on_fun
-) {
-  wsrep_thd_is_brute_force = bf_fun;
-  wsrep_abort_thd          = abort_fun;
-  wsrep_debug              = debug;
-  wsrep_convert_LOCK_to_trx= convert_LOCK_to_trx;
-  wsrep_on                 = on_fun;
-}
-#endif
 /* The following constants are only for debug output */
 #define MAX_THREADS 1000
 #define MAX_LOCKS   1000
@@ -652,93 +634,6 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
   DBUG_RETURN(result);
 }
 
-#ifdef WITH_WSREP
-/*
- * If brute force applier would need to wait for a thr lock,
- * it needs to make sure that it will get the lock without (too much) 
- * delay. 
- * We identify here the owners of blocking locks and ask them to
- * abort. We then put our lock request in the first place in the
- * wait queue. When lock holders abort (one by one) the lock release
- * algorithm should grant the lock to us. We rely on this and proceed
- * to wait_for_locks().
- * wsrep_break_locks() should be called in all the cases, where lock
- * wait would happen.
- *
- * TODO: current implementation might not cover all possible lock wait
- *       situations. This needs an review still.
- * TODO: lock release, might favor some other lock (instead our bf).
- *       This needs an condition to check for bf locks first.
- * TODO: we still have a debug fprintf, this should be removed
- */
-static my_bool 
-wsrep_break_lock(
-    THR_LOCK_DATA *data, struct st_lock_list *lock_queue1, 
-    struct st_lock_list *wait_queue)
-{
-  if (wsrep_on && wsrep_on(data->owner->mysql_thd) &&
-      wsrep_thd_is_brute_force          &&
-      wsrep_thd_is_brute_force(data->owner->mysql_thd, TRUE))
-  {
-    THR_LOCK_DATA *holder;
-
-    /* if locking session conversion to transaction has been enabled,
-       we know that this conflicting lock must be read lock and furthermore,
-       lock holder is read-only. It is safe to wait for him.
-    */
-#ifdef TODO_WHEN_LOCK_TABLES_IS_A_TRANSACTION
-    if (wsrep_convert_LOCK_to_trx && 
-	(THD*)(data->owner->mysql_thd)->in_lock_tables)
-    {
-      if (wsrep_debug) 
-        fprintf(stderr,"WSREP wsrep_break_lock read lock untouched\n");
-      return FALSE;
-    }
-#endif
-    if (wsrep_debug) 
-      fprintf(stderr,"WSREP wsrep_break_lock aborting locks\n");
-
-    /* aborting lock holder(s) here */
-    for (holder=(lock_queue1) ? lock_queue1->data : NULL; 
-	 holder; 
-	 holder=holder->next) 
-    {
-      if (!wsrep_thd_is_brute_force(holder->owner->mysql_thd, TRUE))
-      {
-        wsrep_abort_thd(data->owner->mysql_thd, 
-                        holder->owner->mysql_thd, FALSE);
-      }
-      else
-      {
-        if (wsrep_debug) 
-          fprintf(stderr,"WSREP wsrep_break_lock skipping BF lock conflict\n");
-         return FALSE;
-      }
-    }
-        
-    /* Add our lock to the head of the wait queue */
-    if (*(wait_queue->last)==wait_queue->data)
-    {
-      wait_queue->last=&data->next;
-      assert(wait_queue->data==0);
-    }
-    else
-    {
-      assert(wait_queue->data!=0);
-      wait_queue->data->prev=&data->next;
-    }
-    data->next=wait_queue->data;
-    data->prev=&wait_queue->data;
-    wait_queue->data=data;
-    data->cond=get_cond();
-
-    statistic_increment(locks_immediate,&THR_LOCK_lock);
-    return TRUE;
-  }
-  return FALSE;
-}
-#endif
-
 static enum enum_thr_lock_result
 thr_lock(THR_LOCK_DATA *data, THR_LOCK_INFO *owner, ulong lock_wait_timeout)
 {
@@ -746,9 +641,6 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_INFO *owner, ulong lock_wait_timeout)
   enum enum_thr_lock_result result= THR_LOCK_SUCCESS;
   struct st_lock_list *wait_queue;
   enum thr_lock_type lock_type= data->type;
-#ifdef WITH_WSREP
-  my_bool wsrep_lock_inserted= FALSE;
-#endif
   MYSQL_TABLE_WAIT_VARIABLES(locker, state) /* no ';' */
   DBUG_ENTER("thr_lock");
 
@@ -845,13 +737,6 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_INFO *owner, ulong lock_wait_timeout)
       lock but a high priority write waiting in the write_wait queue.
       In the latter case we should yield the lock to the writer.
     */
-#ifdef WITH_WSREP
-    if (wsrep_break_lock(data, &lock->write, &lock->read_wait))
-    {
-      wsrep_lock_inserted= TRUE;
-    }
-#endif
-
     wait_queue= &lock->read_wait;
   }
   else						/* Request for WRITE lock */
@@ -997,20 +882,8 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_INFO *owner, ulong lock_wait_timeout)
 			 (ulong) lock->read.data->owner->thread_id,
                          data->type));
     }
-#ifdef WITH_WSREP
-    if (wsrep_break_lock(data, &lock->write, &lock->write_wait))
-    {
-      wsrep_lock_inserted= TRUE;
-    }
-#endif
-
     wait_queue= &lock->write_wait;
   }
-  /* Can't get lock yet;  Wait for it */
-#ifdef WITH_WSREP
-  if (wsrep_lock_inserted && wsrep_on(data->owner->mysql_thd))
-    DBUG_RETURN(wait_for_lock(wait_queue, data, 1, lock_wait_timeout));
-#endif
   result= wait_for_lock(wait_queue, data, 0, lock_wait_timeout);
   MYSQL_END_TABLE_LOCK_WAIT(locker);
   DBUG_RETURN(result);

sql/sql_class.h

@@ -615,6 +615,7 @@ typedef struct system_variables
     are based on the cluster size):
   */
   ulong saved_auto_increment_increment, saved_auto_increment_offset;
+  ulong saved_lock_wait_timeout;
 #endif /* WITH_WSREP */
   uint eq_range_index_dive_limit;
   ulong column_compression_zlib_strategy;

sql/wsrep_mysqld.cc

@@ -872,9 +872,6 @@ void wsrep_init_startup (bool sst_first)
 {
   if (wsrep_init()) unireg_abort(1);
 
-  wsrep_thr_lock_init(wsrep_thd_is_BF, wsrep_thd_bf_abort,
-                      wsrep_debug, wsrep_convert_LOCK_to_trx, wsrep_on);
-
   /*
     Pre-initialize global_system_variables.table_plugin with a dummy engine
     (placeholder) required during the initialization of wsrep threads (THDs).
@@ -2194,6 +2191,13 @@ int wsrep_to_isolation_begin(THD *thd, const char *db_, const char *table_,
     thd->variables.auto_increment_increment= 1;
   }
 
+  /*
+    TOI operations will ignore provided lock_wait_timeout and restore it
+    after operation is done.
+   */
+  thd->variables.saved_lock_wait_timeout= thd->variables.lock_wait_timeout;
+  thd->variables.lock_wait_timeout= LONG_TIMEOUT;
+
   if (thd->variables.wsrep_on && wsrep_thd_is_local(thd))
   {
     switch (thd->variables.wsrep_OSU_method) {
@@ -2228,6 +2232,9 @@ void wsrep_to_isolation_end(THD *thd)
 {
   DBUG_ASSERT(wsrep_thd_is_local_toi(thd) ||
               wsrep_thd_is_in_rsu(thd));
+
+  thd->variables.lock_wait_timeout= thd->variables.saved_lock_wait_timeout;
+
   if (wsrep_thd_is_local_toi(thd))
   {
     DBUG_ASSERT(thd->variables.wsrep_OSU_method == WSREP_OSU_TOI);