Incorrect behaviour of WSREP_SYNC_WAIT_UPTO_GTID (#1442)

Function `signal_waiters` assigned `m_committed_seqno` variable outside of
mutex lock which caused incorrect behavior of WSREP_SYNC_WAIT_UPTO_GTID.
Fixed by moving assignment inside lock. Added handling of OOM and now
error is reported.
Remove hard-coded seqno value and read seqno directly from current node state.

Author: mkaruza

mysql-test/suite/galera/r/galera_sync_wait_upto.result

@@ -16,11 +16,10 @@ WSREP_SYNC_WAIT_UPTO
 1
 ERROR HY000: Lock wait timeout exceeded; try restarting transaction
 connection node_2;
-SELECT WSREP_SYNC_WAIT_UPTO_GTID('100-1-3');
 connection node_1;
 INSERT INTO t1 VALUES (2);
 connection node_2;
-WSREP_SYNC_WAIT_UPTO_GTID('100-1-3')
+WSREP_SYNC_WAIT_UPTO
 1
 connection node_1;
 DROP TABLE t1;

mysql-test/suite/galera/t/galera_sync_wait_upto.test

@@ -24,7 +24,9 @@ SELECT WSREP_SYNC_WAIT_UPTO_GTID('1-1-1,1-1-2');
 
 # Expected starting seqno
 
---let $start_seqno = 2
+--let $last_seen_gtid = `SELECT WSREP_LAST_SEEN_GTID()`
+--let $s1 = `SELECT SUBSTR('$last_seen_gtid', LOCATE('-', '$last_seen_gtid') + LENGTH('-'))`
+--let $start_seqno = `SELECT SUBSTR('$s1', LOCATE('-', '$s1') + LENGTH('-'))`
 
 # If set to low value, expect no waiting
 
@@ -57,10 +59,9 @@ SELECT WSREP_SYNC_WAIT_UPTO_GTID('1-1-1,1-1-2');
 --disable_query_log
 --let $wait_seqno = $start_seqno
 --inc $wait_seqno
+--send_eval SELECT WSREP_SYNC_WAIT_UPTO_GTID('100-1-$wait_seqno') AS WSREP_SYNC_WAIT_UPTO
 --enable_query_log
 
---send_eval SELECT WSREP_SYNC_WAIT_UPTO_GTID('100-1-$wait_seqno')
-
 --connection node_1
 INSERT INTO t1 VALUES (2);
 

sql/item_strfunc.cc

@@ -5297,6 +5297,7 @@ longlong Item_func_wsrep_sync_wait_upto::val_int()
   uint timeout;
   rpl_gtid *gtid_list;
   uint32 count;
+  int wait_gtid_ret= 0;
   int ret= 1;
 
   if (args[0]->null_value)
@@ -5323,11 +5324,17 @@ longlong Item_func_wsrep_sync_wait_upto::val_int()
     if (wsrep_check_gtid_seqno(gtid_list[0].domain_id, gtid_list[0].server_id,
                                gtid_list[0].seq_no))
     {
-      if (wsrep_gtid_server.wait_gtid_upto(gtid_list[0].seq_no, timeout))
+      wait_gtid_ret= wsrep_gtid_server.wait_gtid_upto(gtid_list[0].seq_no, timeout);
+      if ((wait_gtid_ret == ETIMEDOUT) || (wait_gtid_ret == ETIME))
       {
         my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0), func_name());
         ret= 0;
       }
+      else if (wait_gtid_ret == ENOMEM)
+      {
+        my_error(ER_OUTOFMEMORY, MYF(0), func_name());
+        ret= 0;
+      }
     }
   }
   else

sql/wsrep_mysqld.h

@@ -431,45 +431,52 @@ class Wsrep_gtid_server
   }
   int wait_gtid_upto(const uint64_t seqno, uint timeout)
   {
-    int wait_result;
+    int wait_result= 0;
     struct timespec wait_time;
     int ret= 0;
     mysql_cond_t wait_cond;
     mysql_cond_init(key_COND_wsrep_gtid_wait_upto, &wait_cond, NULL);
     set_timespec(wait_time, timeout);
     mysql_mutex_lock(&LOCK_wsrep_gtid_wait_upto);
     std::multimap<uint64, mysql_cond_t*>::iterator it;
-    try
+    if (seqno > m_seqno)
     {
-      it= m_wait_map.insert(std::make_pair(seqno, &wait_cond));
-    } 
-    catch (std::bad_alloc& e)
-    {
-      return 0;
-    }
-    while ((m_committed_seqno < seqno) && !m_force_signal)
-    {
-      wait_result= mysql_cond_timedwait(&wait_cond,
-                                        &LOCK_wsrep_gtid_wait_upto,
-                                        &wait_time);
-      if (wait_result == ETIMEDOUT || wait_result == ETIME)
+      try
       {
-        ret= 1;
-        break;
+        it= m_wait_map.insert(std::make_pair(seqno, &wait_cond));
+      } 
+      catch (std::bad_alloc& e)
+      {
+         ret= ENOMEM;
+      }
+      while (!ret && (m_committed_seqno < seqno) && !m_force_signal)
+      {
+        wait_result= mysql_cond_timedwait(&wait_cond,
+                                          &LOCK_wsrep_gtid_wait_upto,
+                                          &wait_time);
+        if (wait_result == ETIMEDOUT || wait_result == ETIME)
+        {
+          ret= wait_result;
+          break;
+        }
+      }
+      if (ret != ENOMEM)
+      {
+        m_wait_map.erase(it);
       }
     }
-    m_wait_map.erase(it);
     mysql_mutex_unlock(&LOCK_wsrep_gtid_wait_upto);
     mysql_cond_destroy(&wait_cond);
     return ret;
   }
   void signal_waiters(uint64 seqno, bool signal_all)
   {
+    mysql_mutex_lock(&LOCK_wsrep_gtid_wait_upto);
     if (!signal_all && (m_committed_seqno >= seqno))
     {
+      mysql_mutex_unlock(&LOCK_wsrep_gtid_wait_upto);
       return;
     }
-    mysql_mutex_lock(&LOCK_wsrep_gtid_wait_upto);
     m_force_signal= true;
     std::multimap<uint64, mysql_cond_t*>::iterator it_end;
     std::multimap<uint64, mysql_cond_t*>::iterator it_begin;
@@ -481,16 +488,16 @@ class Wsrep_gtid_server
     {
       it_end= m_wait_map.upper_bound(seqno);
     }
+    if (m_committed_seqno < seqno)
+    {
+      m_committed_seqno= seqno;
+    }
     for (it_begin = m_wait_map.begin(); it_begin != it_end; ++it_begin)
     {
       mysql_cond_signal(it_begin->second);
     }
     m_force_signal= false;
     mysql_mutex_unlock(&LOCK_wsrep_gtid_wait_upto);
-    if (m_committed_seqno < seqno)
-    {
-      m_committed_seqno= seqno;
-    }
   }
 private:
   const wsrep_server_gtid_t m_undefined= {0,0,0};