MDEV-12837: WSREP: BF lock wait long

This is 10.1 version where no merge error exists. wsrep_on_check New check function. Galera can't be enabled if innodb-lock-schedule-algorithm=VATS. innobase_kill_query In Galera async kill we could own lock mutex. innobase_init If Variance-Aware-Transaction-Sheduling Algorithm (VATS) is used on Galera we refuse to start InnoDB. Changed innodb-lock-schedule-algorithm as read-only parameter as it was designed to be. lock_rec_other_has_expl_req, lock_rec_other_has_conflicting, lock_rec_lock_slow lock_table_other_has_incompatible lock_rec_insert_check_and_lock Change pointer to conflicting lock to normal pointer as this pointer contents could be changed later.

Author: janlindstrom

mysql-test/suite/sys_vars/r/sysvars_innodb.result

@@ -1347,7 +1347,7 @@ NUMERIC_MIN_VALUE	NULL
 NUMERIC_MAX_VALUE	NULL
 NUMERIC_BLOCK_SIZE	NULL
 ENUM_VALUE_LIST	fcfs,vats
-READ_ONLYNO
+READ_ONLYYES
 COMMAND_LINE_ARGUMENT	REQUIRED
 VARIABLE_NAME	INNODB_LOCK_WAIT_TIMEOUT
 SESSION_VALUE	50

mysql-test/suite/sys_vars/t/wsrep_on_basic.opt

@@ -0,0 +1 @@
+--innodb-lock-schedule-algorithm=FCFS

sql/sys_vars.cc

@@ -4895,7 +4895,8 @@ static Sys_var_mybool Sys_wsrep_on (
  "wsrep_on", "To enable wsrep replication ",
  SESSION_VAR(wsrep_on), 
  CMD_LINE(OPT_ARG), DEFAULT(FALSE),
- NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
+ NO_MUTEX_GUARD, NOT_IN_BINLOG,
+ ON_CHECK(wsrep_on_check),
  ON_UPDATE(wsrep_on_update));
 
 static Sys_var_charptr Sys_wsrep_start_position (

sql/wsrep_var.cc

@@ -52,12 +52,28 @@ int wsrep_init_vars()
  return 0;
 }
 
+extern ulong innodb_lock_schedule_algorithm;
+
 bool wsrep_on_update (sys_var *self, THD* thd, enum_var_type var_type)
 {
  if (var_type == OPT_GLOBAL) {
  // FIXME: this variable probably should be changed only per session
  thd->variables.wsrep_on = global_system_variables.wsrep_on;
  }
+
+ return false;
+}
+
+bool wsrep_on_check(sys_var *self, THD* thd, set_var* var)
+{
+ bool new_wsrep_on= (bool)var->save_result.ulonglong_value;
+
+ if (new_wsrep_on && innodb_lock_schedule_algorithm != 0) {
+ my_message(ER_WRONG_ARGUMENTS, " WSREP (galera) can't be enabled "
+ "if innodb_lock_schedule_algorithm=VATS. Please configure"
+ " innodb_lock_schedule_algorithm=FCFS and restart.", MYF(0));
+ return true;
+ }
  return false;
 }
 

sql/wsrep_var.h

@@ -41,7 +41,8 @@ int wsrep_init_vars();
 #define DEFAULT_ARGS (THD* thd, enum_var_type var_type)
 #define INIT_ARGS (const char* opt)
 
-extern bool wsrep_causal_reads_update UPDATE_ARGS;
+extern bool wsrep_causal_reads_update UPDATE_ARGS;
+extern bool wsrep_on_check CHECK_ARGS;
 extern bool wsrep_on_update UPDATE_ARGS;
 extern bool wsrep_sync_wait_update UPDATE_ARGS;
 extern bool wsrep_start_position_check CHECK_ARGS;

storage/innobase/handler/ha_innodb.cc

@@ -3478,6 +3478,17 @@ innobase_init(
 goto error;
 }
 
+#ifdef WITH_WSREP
+/* Currently, Galera does not support VATS lock schedule algorithm. */
+if (innodb_lock_schedule_algorithm == INNODB_LOCK_SCHEDULE_ALGORITHM_VATS
+ && global_system_variables.wsrep_on) {
+/* Do not allow InnoDB startup with VATS and Galera */
+sql_print_error("In Galera environment Variance-Aware-Transaction-Sheduling Algorithm"
+" is not supported.");
+goto error;
+}
+#endif /* WITH_WSREP */
+
 #ifndef HAVE_LZ4
 if (innodb_compression_algorithm == PAGE_LZ4_ALGORITHM) {
 sql_print_error("InnoDB: innodb_compression_algorithm = %lu unsupported.\n"
@@ -4882,8 +4893,8 @@ innobase_kill_query(
 wsrep_thd_is_BF(current_thd, FALSE),
 lock_get_info(trx->lock.wait_lock).c_str());
 
-if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE) &&
- trx->abort_type == TRX_SERVER_ABORT) {
+if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)
+ && trx->abort_type == TRX_SERVER_ABORT) {
 ut_ad(!lock_mutex_own());
 lock_mutex_enter();
 }
@@ -19234,7 +19245,7 @@ static MYSQL_SYSVAR_ULONG(doublewrite_batch_size, srv_doublewrite_batch_size,
 #endif /* defined UNIV_DEBUG || defined UNIV_PERF_DEBUG */
 
 static MYSQL_SYSVAR_ENUM(lock_schedule_algorithm, innodb_lock_schedule_algorithm,
- PLUGIN_VAR_RQCMDARG,
+ PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
  "The algorithm Innodb uses for deciding which locks to grant next when"
  " a lock is released. Possible values are"
  " FCFS"

storage/innobase/lock/lock0lock.cc

@@ -925,15 +925,21 @@ lock_reset_lock_and_trx_wait(
 
 ib_logf(IB_LOG_LEVEL_INFO,
 "Trx id " TRX_ID_FMT
-" is waiting a lock in statement %s"
+" is waiting a lock "
 " for this trx id " TRX_ID_FMT
-" and statement %s wait_lock %p",
+" wait_lock %p",
 lock->trx->id,
-stmt ? stmt : "NULL",
 trx_id,
-stmt2 ? stmt2 : "NULL",
 lock->trx->lock.wait_lock);
 
+if (stmt) {
+ib_logf(IB_LOG_LEVEL_INFO, " SQL1: %s\n", stmt);
+}
+
+if (stmt2) {
+ib_logf(IB_LOG_LEVEL_INFO, " SQL2: %s\n", stmt2);
+}
+
 ut_ad(lock->trx->lock.wait_lock == lock);
 }
 
@@ -1151,7 +1157,7 @@ lock_rec_has_to_wait(
 type_mode, lock_is_on_supremum);
 fprintf(stderr,
 "conflicts states: my %d locked %d\n",
-wsrep_thd_conflict_state(trx->mysql_thd, FALSE), 
+wsrep_thd_conflict_state(trx->mysql_thd, FALSE),
 wsrep_thd_conflict_state(lock2->trx->mysql_thd, FALSE) );
 lock_rec_print(stderr, lock2);
 if (for_locking) return FALSE;
@@ -1687,7 +1693,7 @@ lock_rec_discard(lock_t*	in_lock);
 Checks if some other transaction has a lock request in the queue.
 @return	lock or NULL */
 static
-const lock_t*
+lock_t*
 lock_rec_other_has_expl_req(
 /*========================*/
 enum lock_mode	mode,/*!< in: LOCK_S or LOCK_X */
@@ -1704,7 +1710,7 @@ lock_rec_other_has_expl_req(
 requests by all transactions
 are taken into account */
 {
-const lock_t*	lock;
+lock_t*	lock;
 
 ut_ad(lock_mutex_own());
 ut_ad(mode == LOCK_X || mode == LOCK_S);
@@ -1713,7 +1719,7 @@ lock_rec_other_has_expl_req(
 
 for (lock = lock_rec_get_first(block, heap_no);
  lock != NULL;
- lock = lock_rec_get_next_const(heap_no, lock)) {
+ lock = lock_rec_get_next(heap_no, lock)) {
 
 if (lock->trx != trx
  && (gap
@@ -1800,7 +1806,7 @@ Checks if some other transaction has a conflicting explicit lock request
 in the queue, so that we have to wait.
 @return	lock or NULL */
 static
-const lock_t*
+lock_t*
 lock_rec_other_has_conflicting(
 /*===========================*/
 enum lock_mode	mode,/*!< in: LOCK_S or LOCK_X,
@@ -1812,7 +1818,7 @@ lock_rec_other_has_conflicting(
 ulint	heap_no,/*!< in: heap number of the record */
 const trx_t*	trx)/*!< in: our transaction */
 {
-const lock_t*	lock;
+lock_t*	lock;
 ibool	is_supremum;
 
 ut_ad(lock_mutex_own());
@@ -1821,13 +1827,16 @@ lock_rec_other_has_conflicting(
 
 for (lock = lock_rec_get_first(block, heap_no);
  lock != NULL;
- lock = lock_rec_get_next_const(heap_no, lock)) {
+ lock = lock_rec_get_next(heap_no, lock)) {
 
 #ifdef WITH_WSREP
 if (lock_rec_has_to_wait(TRUE, trx, mode, lock, is_supremum)) {
 if (wsrep_on_trx(trx)) {
 trx_mutex_enter(lock->trx);
-wsrep_kill_victim(trx, lock);
+/* Below function will roll back either trx
+or lock->trx depending on priority of the
+transaction. */
+wsrep_kill_victim(const_cast<trx_t*>(trx), lock);
 trx_mutex_exit(lock->trx);
 }
 #else
@@ -2023,15 +2032,17 @@ wsrep_print_wait_locks(
 {
 if (wsrep_debug && c_lock->trx->lock.wait_lock != c_lock) {
 fprintf(stderr, "WSREP: c_lock != wait lock\n");
-if (lock_get_type_low(c_lock) & LOCK_TABLE)
+if (lock_get_type_low(c_lock) & LOCK_TABLE) {
 lock_table_print(stderr, c_lock);
-else
+} else {
 lock_rec_print(stderr, c_lock);
+}
 
-if (lock_get_type_low(c_lock->trx->lock.wait_lock) & LOCK_TABLE)
+if (lock_get_type_low(c_lock->trx->lock.wait_lock) & LOCK_TABLE) {
 lock_table_print(stderr, c_lock->trx->lock.wait_lock);
-else
+} else {
 lock_rec_print(stderr, c_lock->trx->lock.wait_lock);
+}
 }
 }
 #endif /* WITH_WSREP */
@@ -2217,8 +2228,8 @@ lock_rec_create(
 if (wsrep_debug) {
 fprintf(
 stderr,
-"WSREP: c_lock canceled %llu\n",
-(ulonglong) c_lock->trx->id);
+"WSREP: c_lock canceled " TRX_ID_FMT "\n",
+c_lock->trx->id);
 }
 
 /* have to bail out here to avoid lock_set_lock... */
@@ -2519,6 +2530,16 @@ lock_rec_enqueue_waiting(
 err = DB_LOCK_WAIT;
 }
 
+#ifdef WITH_WSREP
+if (!lock_get_wait(lock) && wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
+if (wsrep_debug) {
+fprintf(stderr, "WSREP: BF thread got lock granted early, ID " TRX_ID_FMT
+"\n",
+lock->trx->id);
+}
+return(DB_SUCCESS);
+ }
+#endif /* WITH_WSREP */
 // Move it only when it does not cause a deadlock.
 if (err != DB_DEADLOCK
 && innodb_lock_schedule_algorithm
@@ -2814,7 +2835,7 @@ lock_rec_lock_slow(
 /* The trx already has a strong enough lock on rec: do
 nothing */
 #ifdef WITH_WSREP
-} else if ((c_lock = (ib_lock_t*)lock_rec_other_has_conflicting(
+} else if ((c_lock = lock_rec_other_has_conflicting(
  static_cast<enum lock_mode>(mode),
  block, heap_no, trx))) {
 #else
@@ -2946,6 +2967,15 @@ lock_rec_has_to_wait_in_queue(
 #ifdef WITH_WSREP
 if (wsrep_thd_is_BF(wait_lock->trx->mysql_thd, FALSE) &&
  wsrep_thd_is_BF(lock->trx->mysql_thd, TRUE)) {
+if (wsrep_debug) {
+fprintf(stderr,
+"BF-BF lock conflict " TRX_ID_FMT
+" : " TRX_ID_FMT "\n",
+wait_lock->trx->id,
+lock->trx->id);
+lock_rec_print(stderr, wait_lock);
+lock_rec_print(stderr, lock);
+}
 /* don't wait for another BF lock */
 continue;
 }
@@ -3104,7 +3134,7 @@ lock_grant_and_move_on_page(
 && !lock_rec_has_to_wait_in_queue(lock)) {
 
 lock_grant(lock, false);
-
+
 if (previous != NULL) {
 /* Move the lock to the head of the list. */
 HASH_GET_NEXT(hash, previous) = HASH_GET_NEXT(hash, lock);
@@ -4978,8 +5008,8 @@ lock_table_create(
 }
 
 if (wsrep_debug) {
-fprintf(stderr, "WSREP: c_lock canceled %llu\n",
-(ulonglong) c_lock->trx->id);
+fprintf(stderr, "WSREP: c_lock canceled " TRX_ID_FMT "\n",
+c_lock->trx->id);
 }
 }
 if (c_lock) {
@@ -5251,7 +5281,7 @@ Checks if other transactions have an incompatible mode lock request in
 the lock queue.
 @return	lock or NULL */
 UNIV_INLINE
-const lock_t*
+lock_t*
 lock_table_other_has_incompatible(
 /*==============================*/
 const trx_t*	trx,/*!< in: transaction, or NULL if all
@@ -5262,7 +5292,7 @@ lock_table_other_has_incompatible(
 const dict_table_t*	table,/*!< in: table */
 enum lock_mode	mode)/*!< in: lock mode */
 {
-const lock_t*	lock;
+lock_t*	lock;
 
 ut_ad(lock_mutex_own());
 
@@ -5315,7 +5345,7 @@ lock_table(
 #endif
 trx_t*	trx;
 dberr_terr;
-const lock_t*	wait_for;
+lock_t*wait_for;
 
 ut_ad(table != NULL);
 ut_ad(thr != NULL);
@@ -5362,13 +5392,13 @@ lock_table(
 
 if (wait_for != NULL) {
 #ifdef WITH_WSREP
- err = lock_table_enqueue_waiting((ib_lock_t*)wait_for, mode | flags, table, thr);
+err = lock_table_enqueue_waiting(wait_for, mode | flags, table, thr);
 #else
 err = lock_table_enqueue_waiting(mode | flags, table, thr);
 #endif
 } else {
 #ifdef WITH_WSREP
- lock_table_create(c_lock, table, mode | flags, trx);
+lock_table_create(c_lock, table, mode | flags, trx);
 #else
 lock_table_create(table, mode | flags, trx);
 #endif
@@ -7036,10 +7066,10 @@ lock_rec_insert_check_and_lock(
 on the successor, which produced an unnecessary deadlock. */
 
 #ifdef WITH_WSREP
-if ((c_lock = (ib_lock_t*)lock_rec_other_has_conflicting(
- static_cast<enum lock_mode>(
- LOCK_X | LOCK_GAP | LOCK_INSERT_INTENTION),
- block, next_rec_heap_no, trx))) {
+if ((c_lock = lock_rec_other_has_conflicting(
+static_cast<enum lock_mode>(
+LOCK_X | LOCK_GAP | LOCK_INSERT_INTENTION),
+block, next_rec_heap_no, trx))) {
 #else
 if (lock_rec_other_has_conflicting(
  static_cast<enum lock_mode>(
@@ -7052,7 +7082,7 @@ lock_rec_insert_check_and_lock(
 
 #ifdef WITH_WSREP
 err = lock_rec_enqueue_waiting(c_lock,
- LOCK_X | LOCK_GAP | LOCK_INSERT_INTENTION,
+LOCK_X | LOCK_GAP | LOCK_INSERT_INTENTION,
 block, next_rec_heap_no, index, thr);
 #else
 err = lock_rec_enqueue_waiting(