From ac1033b0f299a604f7e15c8b2e1b0bea17194a1a Mon Sep 17 00:00:00 2001 From: mkaruza Date: Thu, 30 Jan 2020 15:21:25 +0100 Subject: Incorrect behaviour of WSREP_SYNC_WAIT_UPTO_GTID 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. --- .../suite/galera/r/galera_sync_wait_upto.result | 3 +- .../suite/galera/t/galera_sync_wait_upto.test | 7 +-- sql/item_strfunc.cc | 9 +++- sql/wsrep_mysqld.h | 51 ++++++++++++---------- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/mysql-test/suite/galera/r/galera_sync_wait_upto.result b/mysql-test/suite/galera/r/galera_sync_wait_upto.result index 86c1f80f081..e18cd3b5432 100644 --- a/mysql-test/suite/galera/r/galera_sync_wait_upto.result +++ b/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; diff --git a/mysql-test/suite/galera/t/galera_sync_wait_upto.test b/mysql-test/suite/galera/t/galera_sync_wait_upto.test index 460abf061b2..05353ac7a3a 100644 --- a/mysql-test/suite/galera/t/galera_sync_wait_upto.test +++ b/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); diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index 82259cd6924..bee56a607f7 100644 --- a/sql/item_strfunc.cc +++ b/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 diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h index 222b265248f..077e5602025 100644 --- a/sql/wsrep_mysqld.h +++ b/sql/wsrep_mysqld.h @@ -431,7 +431,7 @@ public: } 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; @@ -439,37 +439,44 @@ public: set_timespec(wait_time, timeout); mysql_mutex_lock(&LOCK_wsrep_gtid_wait_upto); std::multimap::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::iterator it_end; std::multimap::iterator it_begin; @@ -481,16 +488,16 @@ public: { 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}; -- cgit v1.2.1