From 386a8929db429fffea984e395bebb1d5ac85a700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Wed, 1 Jul 2020 17:23:51 +0200 Subject: [PATCH 1/6] Fixes #2916: 'admindb' is now replaced if current one doesn't match the one in the static object --- include/proxysql_config.h | 2 +- include/proxysql_restapi.h | 2 +- lib/ProxySQL_Admin.cpp | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/proxysql_config.h b/include/proxysql_config.h index bc095b76c..1a98cce35 100644 --- a/include/proxysql_config.h +++ b/include/proxysql_config.h @@ -7,8 +7,8 @@ class SQLite3DB; extern const char* config_header; class ProxySQL_Config { - SQLite3DB* admindb; public: + SQLite3DB* admindb; ProxySQL_Config(SQLite3DB* db); virtual ~ProxySQL_Config(); diff --git a/include/proxysql_restapi.h b/include/proxysql_restapi.h index 9660a1264..b1de2bd82 100644 --- a/include/proxysql_restapi.h +++ b/include/proxysql_restapi.h @@ -22,8 +22,8 @@ public: }; class ProxySQL_Restapi { - SQLite3DB* admindb; public: + SQLite3DB* admindb; ProxySQL_Restapi(SQLite3DB* db); virtual ~ProxySQL_Restapi(); diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index 84c2e86e4..e672a38ed 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -778,11 +778,17 @@ int ProxySQL_Test___GetDigestTable(bool reset, bool use_swap) { ProxySQL_Config& ProxySQL_Admin::proxysql_config() { static ProxySQL_Config instance = ProxySQL_Config(admindb); + if (instance.admindb != admindb) { + instance.admindb = admindb; + } return instance; } ProxySQL_Restapi& ProxySQL_Admin::proxysql_restapi() { static ProxySQL_Restapi instance = ProxySQL_Restapi(admindb); + if (instance.admindb != admindb) { + instance.admindb = admindb; + } return instance; } From c415cb449f7eb07b14cf1828ca9d06aaff8de41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Wed, 1 Jul 2020 18:02:23 +0200 Subject: [PATCH 2/6] Fixes #2918: Worker needs to be nullified after being deleted, otherwise freed memory will be accessed later --- src/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.cpp b/src/main.cpp index 0e1427878..3cf2057b8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -766,6 +766,7 @@ void * mysql_worker_thread_func(void *arg) { worker->run(); //delete worker; delete worker; + mysql_thread->worker=NULL; // l_mem_destroy(__thr_sfp); __sync_fetch_and_sub(&GloVars.statuses.stack_memory_mysql_threads,tmp_stack_size); return NULL; From 72ee8586ee12277f9293aeddb68ccf6f34c6a68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Wed, 1 Jul 2020 18:33:45 +0200 Subject: [PATCH 3/6] Fixes #2919: Access to GloMTH from 'refresh_variables' and it's destruction is now mutex protected --- include/proxysql_glovars.hpp | 1 + lib/MySQL_Thread.cpp | 2 ++ lib/ProxySQL_GloVars.cpp | 1 + src/main.cpp | 2 ++ 4 files changed, 6 insertions(+) diff --git a/include/proxysql_glovars.hpp b/include/proxysql_glovars.hpp index 097708c0b..6b8b299f4 100644 --- a/include/proxysql_glovars.hpp +++ b/include/proxysql_glovars.hpp @@ -92,6 +92,7 @@ class ProxySQL_GlobalVariables { #ifdef PROXYSQLCLICKHOUSE bool clickhouse_server; #endif /* PROXYSQLCLICKHOUSE */ + pthread_mutex_t ext_glomth_mutex; } global; struct mysql { char *server_version; diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index db10dd809..9db1fba7e 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -4731,6 +4731,7 @@ void MySQL_Thread::process_all_sessions() { } void MySQL_Thread::refresh_variables() { + pthread_mutex_lock(&GloVars.global.ext_glomth_mutex); if (GloMTH==NULL) { return; } @@ -4910,6 +4911,7 @@ void MySQL_Thread::refresh_variables() { mysql_thread___session_debug=(bool)GloMTH->get_variable_int((char *)"session_debug"); #endif /* DEBUG */ GloMTH->wrunlock(); + pthread_mutex_unlock(&GloVars.global.ext_glomth_mutex); } MySQL_Thread::MySQL_Thread() { diff --git a/lib/ProxySQL_GloVars.cpp b/lib/ProxySQL_GloVars.cpp index 44570d7ad..323e7949b 100644 --- a/lib/ProxySQL_GloVars.cpp +++ b/lib/ProxySQL_GloVars.cpp @@ -100,6 +100,7 @@ ProxySQL_GlobalVariables::ProxySQL_GlobalVariables() : // global.use_proxysql_mem=false; pthread_mutex_init(&global.start_mutex,NULL); pthread_mutex_init(&checksum_mutex,NULL); + pthread_mutex_init(&global.ext_glomth_mutex,NULL); epoch_version = 0; checksums_values.updates_cnt = 0; checksums_values.dumped_at = 0; diff --git a/src/main.cpp b/src/main.cpp index 3cf2057b8..59c139831 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1173,8 +1173,10 @@ void ProxySQL_Main_shutdown_all_modules() { } if (GloMTH) { cpu_timer t; + pthread_mutex_lock(&GloVars.global.ext_glomth_mutex); delete GloMTH; GloMTH=NULL; + pthread_mutex_unlock(&GloVars.global.ext_glomth_mutex); #ifdef DEBUG std::cerr << "GloMTH shutdown in "; #endif From 543f2ce8b601b51cc2b625f633bb9795c7a62aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Wed, 1 Jul 2020 22:08:57 +0200 Subject: [PATCH 4/6] Fixes #2921: Prevent 'child_mysql' to access 'GloMTH' if it's already de-initialized --- lib/ProxySQL_Admin.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index e672a38ed..983093dab 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -4603,6 +4603,7 @@ void ProxySQL_Admin::vacuum_stats(bool is_admin) { void *child_mysql(void *arg) { + if (GloMTH == nullptr) { return NULL; } pthread_attr_t thread_attr; size_t tmp_stack_size=0; From a6b95a3344498e70207b4a03fb4fb0a3ee3a3fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Wed, 1 Jul 2020 22:10:35 +0200 Subject: [PATCH 5/6] Fixes #2920: Improved logic for 'PROXYSQL STOP' and 'PROXYSQL START' - Added new logic for making both commands actually wait for the completion of the operations they started. - Now "PROXYSQL STOP" command sends and OK packet to the client on completion. --- lib/ProxySQL_Admin.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index 983093dab..2a4a6f1c1 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -1206,7 +1206,11 @@ bool admin_handler_command_proxysql(char *query_no_space, unsigned int query_no_ rc=__sync_bool_compare_and_swap(&GloVars.global.nostart,1,0); } if (rc) { + __sync_bool_compare_and_swap(&GloMTH->status_variables.threads_initialized, 1, 0); proxy_debug(PROXY_DEBUG_ADMIN, 4, "Starting ProxySQL following PROXYSQL START command\n"); + while(__sync_fetch_and_add(&GloMTH->status_variables.threads_initialized, 0) == 1) { + usleep(1000); + } SPA->send_MySQL_OK(&sess->client_myds->myprot, NULL); } else { proxy_warning("ProxySQL was already started when received PROXYSQL START command\n"); @@ -1237,6 +1241,11 @@ bool admin_handler_command_proxysql(char *query_no_space, unsigned int query_no_ GloMTH->commit(); glovars.reload=2; __sync_bool_compare_and_swap(&glovars.shutdown,0,1); + GloMTH->signal_all_threads(0); + while (__sync_fetch_and_add(&glovars.shutdown,0)==1) { + usleep(1000); + } + SPA->send_MySQL_OK(&sess->client_myds->myprot, NULL); return false; } From 00e9696dd3fd0e022be2084a340369a355c9321c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 2 Jul 2020 10:44:54 +0200 Subject: [PATCH 6/6] Added documentation for the new extra logic introduced for 'PROXYSQL STOP' and 'PROXYSQL START' --- lib/ProxySQL_Admin.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index 2a4a6f1c1..5efaa847f 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -1206,6 +1206,8 @@ bool admin_handler_command_proxysql(char *query_no_space, unsigned int query_no_ rc=__sync_bool_compare_and_swap(&GloVars.global.nostart,1,0); } if (rc) { + // Set the status variable 'threads_initialized' to 0 because it's initialized back + // in main 'init_phase3'. After GloMTH have been initialized again. __sync_bool_compare_and_swap(&GloMTH->status_variables.threads_initialized, 1, 0); proxy_debug(PROXY_DEBUG_ADMIN, 4, "Starting ProxySQL following PROXYSQL START command\n"); while(__sync_fetch_and_add(&GloMTH->status_variables.threads_initialized, 0) == 1) { @@ -1241,10 +1243,16 @@ bool admin_handler_command_proxysql(char *query_no_space, unsigned int query_no_ GloMTH->commit(); glovars.reload=2; __sync_bool_compare_and_swap(&glovars.shutdown,0,1); + // After setting the shutdown flag, we should wake all threads and wait for + // the shutdown phase to complete. GloMTH->signal_all_threads(0); while (__sync_fetch_and_add(&glovars.shutdown,0)==1) { usleep(1000); } + // After shutdown phase is completed, we must to send a 'OK' to the + // mysql client, otherwise, since this session might not be drop due + // to the waiting condition, the client wont disconnect and will + // keep forever waiting for acknowledgement. SPA->send_MySQL_OK(&sess->client_myds->myprot, NULL); return false; }