Repository: sysown/proxysql (language: cpp)

Issue:
Prevent unnecessary `VACUUM` executions for SQLite3 `stats` database
## Description

While profiling several queries commonly received by Admin targeting `stats` database, like queries from other `ProxySQL` instances regarding `Cluster`, it was found that one of the biggest contributions to CPU utilization for serving those queries is a `VACUUM` that is performed after the query execution.

Many times this `VACUUM` is performed after queries that are idempotent on tables that won't benefit at all of this subsequent operation. This is an extract of a `perf` report:

- Query: `SELECT Variable_Name, Variable_Value FROM stats_mysql_global`
- Report extract:
  ```
  -   88.56%     0.01%  Admin            proxysql              [.] void admin_session_handler<MySQL_Session>(MySQL_Session*, void*, _PtrSize_t*)
     - 88.55% void admin_session_handler<MySQL_Session>(MySQL_Session*, void*, _PtrSize_t*)
        + 45.64% ProxySQL_Admin::vacuum_stats(bool)
        + 32.01% SQLite3DB::execute_statement(char const*, char**, int*, int*, SQLite3_result**)
        + 9.94% ProxySQL_Admin::GenericRefreshStatistics(char const*, unsigned int, bool)
        + 0.87% SQLite3_result::~SQLite3_result()
  ```

These `VACUUM` operations should be more selective, or performed when the database is going to benefit of the operation.


Discussion / hints:

Fix patch:
diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp
index 41c21783f..38b2c954c 100644
--- a/lib/MySQL_Session.cpp
+++ b/lib/MySQL_Session.cpp
@@ -4048,7 +4048,7 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) {
 									const uint32_t recv_query_sz { pkt.size - 5 };
 
 									proxy_debug(PROXY_DEBUG_MYSQL_COM, 5, "Processing received query"
-										"   session=%p session_type=%d schemaname=%s query=%.*s\n",
+										"   session=%p session_type=%d schemaname=\"%s\" query=\"%.*s\"\n",
 										this, session_type, schemaname ? schemaname : "", recv_query_sz, recv_query
 									);
 								}
diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp
index af6b7bb87..88890c456 100644
--- a/lib/ProxySQL_Admin.cpp
+++ b/lib/ProxySQL_Admin.cpp
@@ -1682,20 +1682,24 @@ bool ProxySQL_Admin::GenericRefreshStatistics(const char *query_no_space, unsign
 		}
 		//pthread_mutex_unlock(&admin_mutex);
 	}
-	if (
-		stats_mysql_processlist || stats_mysql_connection_pool || stats_mysql_connection_pool_reset ||
-		stats_mysql_query_digest || stats_mysql_query_digest_reset || stats_mysql_errors ||
-		stats_mysql_errors_reset || stats_mysql_global || stats_memory_metrics || 
-		stats_mysql_commands_counters || stats_mysql_query_rules || stats_mysql_users ||
-		stats_mysql_gtid_executed || stats_mysql_free_connections || 
-		stats_pgsql_global || stats_pgsql_connection_pool || stats_pgsql_connection_pool_reset ||
-		stats_pgsql_free_connections || stats_pgsql_users || stats_pgsql_processlist ||
-		stats_pgsql_errors || stats_pgsql_errors_reset || stats_pgsql_query_rules || stats_pgsql_commands_counters ||
-		stats_pgsql_query_digest || stats_pgsql_query_digest_reset
-	) {
-		ret = true;
+
+	int freelist_count = statsdb->return_one_int("PRAGMA freelist_count");
+	if (freelist_count < 1000) {
+		ret = false;
+	} else {
+		int page_count  = statsdb->return_one_int("PRAGMA page_count");
+		ret = (freelist_count * 100 / page_count) > 20;
+
+#ifdef DEBUG
+		if (ret) {
+			proxy_debug(PROXY_DEBUG_ADMIN, 4,
+				"VACUUM required for 'stats_db'   page_count=%d freelist_count=%d\n",
+				page_count, freelist_count
+			);
+		}
+#endif
 	}
-	
+
 	return ret;
 }
 
diff --git a/lib/ProxySQL_Admin_Stats.cpp b/lib/ProxySQL_Admin_Stats.cpp
index 6de28f52f..c6b477f93 100644
--- a/lib/ProxySQL_Admin_Stats.cpp
+++ b/lib/ProxySQL_Admin_Stats.cpp
@@ -428,163 +428,170 @@ void ProxySQL_Admin::p_update_stmt_metrics() {
 	}
 }
 
+using row_bind_t = void (*)(int offset, SQLite3DB* db, sqlite3_stmt* stmt, SQLite3_row* row);
+
+void sqlite3_bulk_step(
+	SQLite3DB* db,
+	sqlite3_stmt* row_stmt,
+	sqlite3_stmt* bulk_stmt,
+	SQLite3_result* resultset,
+	row_bind_t row_bind
+) {
+	int max_bulk_row_idx = resultset->rows_count / 32;
+	max_bulk_row_idx = max_bulk_row_idx * 32;
+
+	int rc = 0;
+	int row_idx = 0;
+
+	for (SQLite3_row* row : resultset->rows) {
+		int e_idx = row_idx % 32;
+
+		if (row_idx < max_bulk_row_idx) {
+			row_bind(e_idx, db, bulk_stmt, row);
+
+			if (e_idx == 31) {
+				SAFE_SQLITE3_STEP2(bulk_stmt);
+				rc = (*proxy_sqlite3_clear_bindings)(bulk_stmt); ASSERT_SQLITE_OK(rc, db);
+				rc = (*proxy_sqlite3_reset)(bulk_stmt); ASSERT_SQLITE_OK(rc, db);
+			}
+		} else {
+			row_bind(0, db, row_stmt, row);
+
+			SAFE_SQLITE3_STEP2(row_stmt);
+			rc = (*proxy_sqlite3_clear_bindings)(row_stmt); ASSERT_SQLITE_OK(rc, db);
+			rc = (*proxy_sqlite3_reset)(row_stmt); ASSERT_SQLITE_OK(rc, db);
+		}
+
+		row_idx += 1;
+	}
+}
+
+void stats_mysql_global___bind_row(
+	int offset, SQLite3DB* db, sqlite3_stmt* stmt, SQLite3_row* row
+) {
+	int rc = (*proxy_sqlite3_bind_text)(stmt, (offset * 2) + 1, row->fields[0], -1, SQLITE_TRANSIENT);
+	ASSERT_SQLITE_OK(rc, db);
+	rc = (*proxy_sqlite3_bind_text)(stmt, (offset * 2) + 2, row->fields[1], -1, SQLITE_TRANSIENT);
+	ASSERT_SQLITE_OK(rc, db);
+}
+
+template <class...> constexpr std::false_type always_false {};
+
+template <typename T>
+const void sqlite3_global_stats_row_step(
+	SQLite3DB* db, sqlite3_stmt* stmt, const char* name, T val
+) {
+	char buf[32] = { 0 };
+
+	if constexpr (std::is_same_v<T, int32_t>)  {
+		sprintf(buf, "%d", val);
+	} else if constexpr (std::is_same_v<T, uint64_t>) {
+		sprintf(buf, "%lu", val);
+	} else if constexpr (std::is_same_v<T, unsigned long long>) {
+		sprintf(buf, "%llu", val);
+	} else if constexpr (std::is_same_v<T, bool>) {
+		sprintf(buf, "%s", val ? "true" : "false");
+	} else {
+		static_assert(always_false<T>, "Non-exhaustive switch");
+	}
+
+	int rc = (*proxy_sqlite3_bind_text)(stmt, 1, name, -1, SQLITE_TRANSIENT);
+	ASSERT_SQLITE_OK(rc, db);
+	rc = (*proxy_sqlite3_bind_text)(stmt, 2, buf, -1, SQLITE_TRANSIENT);
+	ASSERT_SQLITE_OK(rc, db);
+
+	SAFE_SQLITE3_STEP2(stmt);
+	rc = (*proxy_sqlite3_clear_bindings)(stmt); ASSERT_SQLITE_OK(rc, db);
+	rc = (*proxy_sqlite3_reset)(stmt); ASSERT_SQLITE_OK(rc, db);
+};
+
 void ProxySQL_Admin::stats___mysql_global() {
 	if (!GloMTH) return;
 	SQLite3_result * resultset=GloMTH->SQL3_GlobalStatus(true);
 	if (resultset==NULL) return;
+
 	statsdb->execute("BEGIN");
 	statsdb->execute("DELETE FROM stats_mysql_global");
-	char *a=(char *)"INSERT INTO stats_mysql_global VALUES (\"%s\",\"%s\")";
-	for (std::vector<SQLite3_row *>::iterator it = resultset->rows.begin() ; it != resultset->rows.end(); ++it) {
-		SQLite3_row *r=*it;
-		int arg_len=0;
-		for (int i=0; i<2; i++) {
-			arg_len+=strlen(r->fields[i]);
-		}
-		char *query=(char *)malloc(strlen(a)+arg_len+32);
-		sprintf(query,a,r->fields[0],r->fields[1]);
-		statsdb->execute(query);
-		free(query);
-	}
+
+	const string q_row_insert { "INSERT INTO stats_mysql_global VALUES (?1, ?2)" };
+	const string q_bulk_insert {
+		"INSERT INTO stats_mysql_global VALUES " + generate_multi_rows_query(32, 2)
+	};
+
+	sqlite3_stmt* row_stmt = nullptr;
+	int rc = statsdb->prepare_v2(q_row_insert.c_str(), &row_stmt);
+	ASSERT_SQLITE_OK(rc, statsdb);
+	sqlite3_stmt* bulk_stmt = nullptr;
+	rc = statsdb->prepare_v2(q_bulk_insert.c_str(), &bulk_stmt);
+	ASSERT_SQLITE_OK(rc, statsdb);
+
+	sqlite3_bulk_step(statsdb, row_stmt, bulk_stmt, resultset, stats_mysql_global___bind_row);
+
 	delete resultset;
 	resultset=NULL;
 
 	resultset=MyHGM->SQL3_Get_ConnPool_Stats();
+
 	if (resultset) {
-		for (std::vector<SQLite3_row *>::iterator it = resultset->rows.begin() ; it != resultset->rows.end(); ++it) {
-			SQLite3_row *r=*it;
-			int arg_len=0;
-			for (int i=0; i<2; i++) {
-				arg_len+=strlen(r->fields[i]);
-			}
-			char *query=(char *)malloc(strlen(a)+arg_len+32);
-			sprintf(query,a,r->fields[0],r->fields[1]);
-			statsdb->execute(query);
-			free(query);
-		}
+		sqlite3_bulk_step(statsdb, row_stmt, bulk_stmt, resultset, stats_mysql_global___bind_row);
 		delete resultset;
 		resultset=NULL;
 	}
 
-	int highwater;
-	int current;
-	(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
-	char bu[32];
-	char *vn=NULL;
-	char *query=NULL;
-	vn=(char *)"SQLite3_memory_bytes";
-	sprintf(bu,"%d",current);
-	query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-	sprintf(query,a,vn,bu);
-	statsdb->execute(query);
-	free(query);
+	{
+		int highwater, current = 0;
+		(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "SQLite3_memory_bytes", current);
+	}
 
-	unsigned long long connpool_mem=MyHGM->Get_Memory_Stats();
-	vn=(char *)"ConnPool_memory_bytes";
-	sprintf(bu,"%llu",connpool_mem);
-	query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-	sprintf(query,a,vn,bu);
-	statsdb->execute(query);
-	free(query);
+	{
+		unsigned long long connpool_mem=MyHGM->Get_Memory_Stats();
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "ConnPool_memory_bytes", connpool_mem);
+	}
 
 	if (GloMyStmt) {
-		uint64_t stmt_client_active_unique = 0;
-		uint64_t stmt_client_active_total = 0;
-		uint64_t stmt_max_stmt_id = 0;
-		uint64_t stmt_cached = 0;
-		uint64_t stmt_server_active_unique = 0;
-		uint64_t stmt_server_active_total = 0;
-		GloMyStmt->get_metrics(&stmt_client_active_unique,&stmt_client_active_total,&stmt_max_stmt_id,&stmt_cached,&stmt_server_active_unique,&stmt_server_active_total);
-		vn=(char *)"Stmt_Client_Active_Total";
-		sprintf(bu,"%lu",stmt_client_active_total);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-		vn=(char *)"Stmt_Client_Active_Unique";
-		sprintf(bu,"%lu",stmt_client_active_unique);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-		vn=(char *)"Stmt_Server_Active_Total";
-		sprintf(bu,"%lu",stmt_server_active_total);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-		vn=(char *)"Stmt_Server_Active_Unique";
-		sprintf(bu,"%lu",stmt_server_active_unique);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-		vn=(char *)"Stmt_Max_Stmt_id";
-		sprintf(bu,"%lu",stmt_max_stmt_id);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-		vn=(char *)"Stmt_Cached";
-		sprintf(bu,"%lu",stmt_cached);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
+		uint64_t client_active_unique = 0;
+		uint64_t client_active_total = 0;
+		uint64_t max_stmt_id = 0;
+		uint64_t cached = 0;
+		uint64_t server_active_unique = 0;
+		uint64_t server_active_total = 0;
+
+		GloMyStmt->get_metrics(
+			&client_active_unique,
+			&client_active_total,
+			&max_stmt_id,
+			&cached,
+			&server_active_unique,
+			&server_active_total
+		);
+
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Client_Active_Total", client_active_total);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Client_Active_Unique", client_active_unique);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Server_Active_Total", server_active_total);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Server_Active_Unique", server_active_unique);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Max_Stmt_id", max_stmt_id);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "Stmt_Cached", cached);
 	}
 
 	if (GloMyQC && (resultset= GloMyQC->SQL3_getStats())) {
-		for (std::vector<SQLite3_row *>::iterator it = resultset->rows.begin() ; it != resultset->rows.end(); ++it) {
-			SQLite3_row *r=*it;
-			int arg_len=0;
-			for (int i=0; i<2; i++) {
-				arg_len+=strlen(r->fields[i]);
-			}
-			char *query=(char *)malloc(strlen(a)+arg_len+32);
-			sprintf(query,a,r->fields[0],r->fields[1]);
-			statsdb->execute(query);
-			free(query);
-		}
+		sqlite3_bulk_step(statsdb, row_stmt, bulk_stmt, resultset, stats_mysql_global___bind_row);
 		delete resultset;
 		resultset=NULL;
 	}
 
 	if (GloMyLdapAuth) {
 		resultset=GloMyLdapAuth->SQL3_getStats();
-		if (resultset) {
-			for (std::vector<SQLite3_row *>::iterator it = resultset->rows.begin() ; it != resultset->rows.end(); ++it) {
-				SQLite3_row *r=*it;
-				int arg_len=0;
-				for (int i=0; i<2; i++) {
-					arg_len+=strlen(r->fields[i]);
-				}
-				char *query=(char *)malloc(strlen(a)+arg_len+32);
-				sprintf(query,a,r->fields[0],r->fields[1]);
-				statsdb->execute(query);
-				free(query);
-			}
-			delete resultset;
-			resultset=NULL;
-		}
+		sqlite3_bulk_step(statsdb, row_stmt, bulk_stmt, resultset, stats_mysql_global___bind_row);
 	}
 
 	if (GloMyQPro) {
 		unsigned long long mu = GloMyQPro->get_new_req_conns_count();
-		vn=(char *)"new_req_conns_count";
-		sprintf(bu,"%llu",mu);
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
-	}
-	{
-		vn=(char *)"mysql_listener_paused";
-		sprintf(bu, "%s", ( admin_proxysql_mysql_paused==true ? "true" : "false") );
-		query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
-		sprintf(query,a,vn,bu);
-		statsdb->execute(query);
-		free(query);
+		sqlite3_global_stats_row_step(statsdb, row_stmt, "new_req_conns_count", mu);
 	}
+
+	sqlite3_global_stats_row_step(statsdb, row_stmt, "mysql_listener_paused", admin_proxysql_mysql_paused);
+
 	statsdb->execute("COMMIT");
 }
 


Test patch:
diff --git a/test/tap/tap/utils.cpp b/test/tap/tap/utils.cpp
index 5821da27e..e6f27fc0f 100644
--- a/test/tap/tap/utils.cpp
+++ b/test/tap/tap/utils.cpp
@@ -1114,8 +1114,11 @@ int get_proxysql_cpu_usage(const CommandLine& cl, double& cpu_usage, uint32_t in
 		sleep(1);
 	}
 
-	MYSQL_QUERY(admin, "SELECT * FROM system_cpu ORDER BY timestamp DESC LIMIT 2");
-	MYSQL_RES* admin_res = mysql_store_result(admin);
+	MYSQL_QUERY(admin, "SELECT * FROM system_cpu ORDER BY timestamp DESC LIMIT 5");
+	MYSQL_RES* admin_res { mysql_store_result(admin) };
+	const string sys_cpu_str { dump_as_table(admin_res) };
+	diag("Dumping latest 5 entries from 'system_cpu':\n%s", sys_cpu_str.c_str());
+
 	MYSQL_ROW row = mysql_fetch_row(admin_res);
 
 	double s_clk = (1000.0 / sysconf(_SC_CLK_TCK));