Merge pull request #4758 from sysown/v3.0-fix_hang_on_resume

Fix halt on 'PROXYSQL RESUME' command - Port of #4757
sysown · Nov 26, 2024 · ca1d207 · ca1d207
2 parents be5ec4e + 4a75f74
commit ca1d207
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 35 deletions.
diff --git a/include/ProxySQL_Poll.h b/include/ProxySQL_Poll.h
@@ -35,7 +35,6 @@ class ProxySQL_Poll {
 	T **myds;
 	unsigned long long *last_recv;
 	unsigned long long *last_sent;
-	std::atomic<bool> bootstrapping_listeners;
 	volatile int pending_listener_add;
 	volatile int pending_listener_del;
 	unsigned int poll_timeout;

diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp
@@ -1204,7 +1204,11 @@ int MySQL_Threads_Handler::listener_del(const char *iface) {
 		}
 		for (i=0;i<num_threads;i++) {
 			MySQL_Thread *thr=(MySQL_Thread *)mysql_threads[i].worker;
-			while(__sync_fetch_and_add(&thr->mypolls.pending_listener_del,0));
+			while(__sync_fetch_and_add(&thr->mypolls.pending_listener_del,0)) {
+				// Since 'listeners_stop' is performed in 'maintenance_loops' by the
+				// workers this active-wait is likely to take some time.
+				usleep(std::min(std::max(mysql_thread___poll_timeout/20, 10000), 40000));
+			}
 		}
 		MLM->del(idx);
 #ifdef SO_REUSEPORT
@@ -3127,15 +3131,11 @@ void MySQL_Thread::run_BootstrapListener() {
 		if (n) {
 			poll_listener_add(n);
 			assert(__sync_bool_compare_and_swap(&mypolls.pending_listener_add,n,0));
-		} else {
-			if (GloMTH->bootstrapping_listeners == false) {
-				// we stop looping
-				mypolls.bootstrapping_listeners = false;
-			}
 		}
-#ifdef DEBUG
-		usleep(5+rand()%10);
-#endif
+		// The delay for the active-wait is a fraction of 'poll_timeout'. Since other
+		// threads may be waiting on poll for further operations, checks are meaningless
+		// until that timeout expires (other workers make progress).
+		usleep(std::min(std::max(mysql_thread___poll_timeout/20, 10000), 40000) + (rand() % 2000));
 	}
 }
 
@@ -3238,9 +3238,7 @@ void MySQL_Thread::run() {
 #endif // IDLE_THREADS
 
 		pthread_mutex_unlock(&thread_mutex);
-		if (unlikely(mypolls.bootstrapping_listeners == true)) {
-			run_BootstrapListener();
-		}
+		run_BootstrapListener();
 
 		// flush mysql log file
 		GloMyLogger->flush();

diff --git a/lib/PgSQL_Thread.cpp b/lib/PgSQL_Thread.cpp
@@ -1158,7 +1158,11 @@ int PgSQL_Threads_Handler::listener_del(const char* iface) {
 		}
 		for (i = 0; i < num_threads; i++) {
 			PgSQL_Thread* thr = (PgSQL_Thread*)pgsql_threads[i].worker;
-			while (__sync_fetch_and_add(&thr->mypolls.pending_listener_del, 0));
+			while (__sync_fetch_and_add(&thr->mypolls.pending_listener_del, 0)) {
+				// Since 'listeners_stop' is performed in 'maintenance_loops' by the
+				// workers this active-wait is likely to take some time.
+				usleep(std::min(std::max(pgsql_thread___poll_timeout/20, 10000), 40000));
+			}
 		}
 		MLM->del(idx);
 #ifdef SO_REUSEPORT
@@ -2998,26 +3002,19 @@ void PgSQL_Thread::run() {
 #endif // IDLE_THREADS
 
 		pthread_mutex_unlock(&thread_mutex);
-		if (unlikely(mypolls.bootstrapping_listeners == true)) {
-			while ( // spin here if ...
-				(n = __sync_add_and_fetch(&mypolls.pending_listener_add, 0)) // there is a new listener to add
-				||
-				(GloPTH->bootstrapping_listeners == true) // PgSQL_Thread_Handlers has more listeners to configure
-				) {
-				if (n) {
-					poll_listener_add(n);
-					assert(__sync_bool_compare_and_swap(&mypolls.pending_listener_add, n, 0));
-				}
-				else {
-					if (GloPTH->bootstrapping_listeners == false) {
-						// we stop looping
-						mypolls.bootstrapping_listeners = false;
-					}
-				}
-#ifdef DEBUG
-				usleep(5 + rand() % 10);
-#endif
+		while ( // spin here if ...
+			(n = __sync_add_and_fetch(&mypolls.pending_listener_add, 0)) // there is a new listener to add
+			||
+			(GloPTH->bootstrapping_listeners == true) // PgSQL_Thread_Handlers has more listeners to configure
+		) {
+			if (n) {
+				poll_listener_add(n);
+				assert(__sync_bool_compare_and_swap(&mypolls.pending_listener_add, n, 0));
 			}
+			// The delay for the active-wait is a fraction of 'poll_timeout'. Since other
+			// threads may be waiting on poll for further operations, checks are meaningless
+			// until that timeout expires (other workers make progress).
+			usleep(std::min(std::max(pgsql_thread___poll_timeout/20, 10000), 40000) + (rand() % 2000));
 		}
 
 		proxy_debug(PROXY_DEBUG_NET, 7, "poll_timeout=%u\n", mypolls.poll_timeout);

diff --git a/lib/ProxySQL_Poll.cpp b/lib/ProxySQL_Poll.cpp
@@ -68,7 +68,6 @@ ProxySQL_Poll<T>::ProxySQL_Poll() {
 	len=0;
 	pending_listener_add=0;
 	pending_listener_del=0;
-	bootstrapping_listeners = true;
 	size=MIN_POLL_LEN;
 	fds=(struct pollfd *)malloc(size*sizeof(struct pollfd));
 	myds=(T**)malloc(size*sizeof(T *));

diff --git a/test/tap/tests/reg_test_4402-mysql_fields-t.cpp b/test/tap/tests/reg_test_4402-mysql_fields-t.cpp
@@ -106,7 +106,9 @@ int main(int argc, char** argv) {
 
 		// to check table alias issue:
 		{
-			const std::string& query = "SELECT data FROM testdb.dummy_table AS " + generate_random_string(length);
+			// NOTE: The randomly generated string should be escaped \`\`, otherwise could collide
+			// with SQL reserved words, causing an invalid test failure.
+			const std::string& query = "SELECT data FROM testdb.dummy_table AS `" + generate_random_string(length) + "`";
 			MYSQL_QUERY__(proxysql, query.c_str());
 
 			MYSQL_RES* res = mysql_use_result(proxysql);