Cluster: Admin deadlocks with servers sync (SHOW PROMETHEUS METRICS / LOAD ADMIN VARIABLES vs ProxySQL_Cluster_Nodes::mutex) #6493

Summary

A Cluster servers pull and two Admin statements take ProxySQL_Cluster_Nodes::mutex and ProxySQL_Admin::sql_query_global_mutex in opposite orders. When they overlap, the Admin interface (admin and stats ports) and Cluster sync on every peer hang for good. Only killing the process recovers.

• The Cluster peer thread holds the nodes mutex across ProxySQL_Node_Entry::set_checksums(). The servers pulls started from there take the global SQL mutex after fetching from the peer.
• SHOW PROMETHEUS METRICS holds the global SQL mutex (as every Admin statement does) and then takes the nodes mutex while refreshing the Cluster node metrics.
• LOAD ADMIN VARIABLES TO RUNTIME does the same when an admin-cluster_*_diffs_before_sync goes from 0 to a non-zero value.

Affected versions / tiers

• All tiers (Stable v3.0.x, v3.1.x, v4.0.x).
• Regression vs v3.0.11, in practice. The same inversion already existed in v3.0.11, but there only the mysql/pgsql query rules pulls took the global SQL mutex, so it could only be hit while query rules were syncing. Commit d759171 ("fix(cluster): serialize servers pulls with Admin commands") added the global SQL mutex to all four servers pulls. Those pulls run on every mysql/pgsql servers checksum change, including runtime-only topology changes, and the nodes mutex is held during the whole network fetch. That makes the window much wider and much more frequent.

Root cause

Line numbers are at d7ce280.

Cluster order: nodes mutex, then global SQL mutex.

// lib/ProxySQL_Cluster.cpp:4183
pthread_mutex_lock(&mutex);
...
node->set_checksums(_r);            // :4187, runs the pulls with 'mutex' held

Each pull connects to the peer and fetches first, then:

proxysql_admin_global_sql_lock(GloAdmin->sql_query_global_mutex);

The lock sites are pull_runtime_mysql_servers_from_peer :2079, pull_mysql_servers_v2_from_peer :2367, pull_runtime_pgsql_servers_from_peer :3634 and pull_pgsql_servers_v2_from_peer :3885. The query rules pulls lock at :1235 and :3474.

Admin order: global SQL mutex, then nodes mutex.

• lib/Admin_Handler.cpp:3211 takes sql_query_global_mutex for every statement.
• SHOW PROMETHEUS METRICS (lib/Admin_Handler.cpp:5215) calls pa->serial_exposer({}). Its update callback update_modules_metrics() calls GloProxyCluster->p_update_metrics() (lib/ProxySQL_Admin.cpp:3001), which reaches ProxySQL_Cluster_Nodes::update_prometheus_nodes_metrics():

  // lib/ProxySQL_Cluster.cpp:4831
  pthread_mutex_lock(&mutex);

• LOAD ADMIN VARIABLES TO RUNTIME goes through set_variable(..., lock=true). When a cluster_*_diffs_before_sync is re-enabled from 0, it calls GloProxyCluster->Reset_Global_Checksums(lock) (lib/ProxySQL_Admin.cpp:4786 and 11 siblings), and that function locks the nodes mutex at lib/ProxySQL_Cluster.cpp:4222. At this point the session also holds the Admin rwlock (wrlock() in flush_admin_variables___database_to_runtime).

The code already handles this exact inversion elsewhere with release windows. See stats___proxysql_servers_checksums() / stats___proxysql_servers_metrics() in lib/ProxySQL_Admin_Stats.cpp:1614-1680 and LOAD/SAVE PROXYSQL SERVERS in lib/Admin_Handler.cpp:2579/2608. These two paths have no such window.

How to reproduce

You need two or more cluster nodes, with node A syncing mysql servers from B (default admin-cluster_mysql_servers_diffs_before_sync).

1. On B, loop:

   REPLACE INTO mysql_servers (hostgroup_id,hostname,port,status,weight) VALUES (4242,'127.0.0.2',3306,'OFFLINE_HARD',<n>);
   LOAD MYSQL SERVERS TO RUNTIME;

2. On A, in parallel, loop SHOW PROMETHEUS METRICS; on the admin port, the way a monitoring agent polls it. A second option is to loop:

   SET admin-cluster_mysql_query_rules_diffs_before_sync=0; LOAD ADMIN VARIABLES TO RUNTIME;
   SET admin-cluster_mysql_query_rules_diffs_before_sync=3; LOAD ADMIN VARIABLES TO RUNTIME;

3. Sooner or later a statement on A never returns. From then on every Admin statement on A blocks, and A's Cluster threads stop syncing.

The race:

1. A's peer thread takes the nodes mutex and starts pull_mysql_servers_v2_from_peer, which fetches from B.
2. On A, an Admin session takes the global SQL mutex and blocks on the nodes mutex.
3. The pull finishes its fetch and blocks on the global SQL mutex.

Impact

• The Admin and stats interfaces on the node hang permanently. All Cluster sync threads on that node hang as well, since they share the nodes mutex.
• Any monitoring stack that scrapes SHOW PROMETHEUS METRICS through the Admin SQL interface on a ProxySQL Cluster can hit it during normal topology changes. The REST /metrics endpoint is not affected, because it does not hold the global SQL mutex.

Proposed fix

Keep a single order: ProxySQL_Cluster_Nodes::mutex before sql_query_global_mutex. A thread holding the global SQL mutex must never wait for the nodes mutex.

• SHOW PROMETHEUS METRICS: call serial_exposer() inside a proxysql_admin_global_sql_release_scope. This is the same window the stats snapshots already use. The collection then runs exactly as it does for REST /metrics, and the session's lock levels are restored before the result is sent.
• Reset_Global_Checksums(true) (the Admin caller): a release window would not be safe here. The session also holds the Admin rwlock, and other Admin sessions take that rwlock after the global SQL mutex, so releasing only the global mutex would create a new cycle. Instead, the reset request is recorded in an atomic flag, and the next Update_Global_Checksum() applies it under the nodes mutex before it compares. global_checksum is only read there, so the effect is unchanged: the next check of every peer fetches the full checksums. Reset_Global_Checksums(false) (the Cluster's own admin variables pull, already under the nodes mutex) still resets immediately.

After the fix, these are the only places that take both locks:

REST /metrics, the TSDB sampler (v3.1+) and the Cluster's own proxysql_servers / admin variables pulls take the nodes mutex without the global SQL mutex.

Is this really a bug?

Yes. Both lock orders exist in the current code with no release window or try-lock, and the result is a permanent hang of the Admin interface. The codebase already treats the same inversion as a bug for the stats tables (#4415 and the comments in ProxySQL_Admin_Stats.cpp). The only question is how likely it is to be hit. Since d759171 it is reachable from any servers checksum change combined with an Admin-side Prometheus scrape, which is a common production setup.