Bug#99917: Limit connections on the administrative interface - #727
Bug#99917: Limit connections on the administrative interface #727vidyadharchelluru wants to merge 1 commit into
Conversation
…ions The administrative connection interface (admin_address/admin_port, WL#12138) accepts an unlimited number of connections. Any account holding SERVICE_CONNECTION_ADMIN can therefore exhaust server threads through the administrative interface, and max_connections offers no protection there. Bug#99917 requests an option to cap it. This change adds: * admin_max_connections: GLOBAL, dynamic ulong, range 0..100000, default 0. Caps the number of concurrent connections accepted on the administrative interface. 0 preserves the previous unlimited behaviour, so the default is fully backward compatible. max_connections and admin_max_connections are enforced independently: the former continues to govern only ordinary connections, the latter only administrative ones. * Admin_connections status variable: number of currently open administrative connections. Ordinary connections never touch it. * Admin_connection_errors_max_connections status variable: number of administrative connections refused because admin_max_connections was reached. These rejections deliberately do not increment Connection_errors_max_connections, which keeps counting only ordinary connections refused by max_connections. Threads_connected keeps counting all connections including administrative ones (unchanged). Implementation: both counters are static members of Connection_handler_manager protected by the existing LOCK_connection_count mutex, so no new synchronization primitive or PSI key is needed. check_and_incr_conn_count() rejects an administrative connection with ER_CON_COUNT_ERROR when the cap is reached; lowering the cap below the number of open administrative connections affects new connections only. dec_connection_count() gains an optional is_admin_connection argument (default false) so the counter stays balanced on every disconnect path of the per-thread and one-thread handlers. Internal sessions (srv_session_service passes ignore_max_connection_limit as the first argument) are excluded on both the check and increment side via the internal_session argument. The exported dec_connection_count() hook in thread_pool_priv.h keeps the default; a thread pool implementation serving the administrative interface would need to pass the flag through. Tests: main.admin_max_connections (functional coverage incl. status variable separation), sys_vars.admin_max_connections_basic, plus bookkeeping updates to all_persisted_variables (450 -> 451) and mysqld--help-notwin. This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
|
I confirm the code being submitted is offered under the terms of the OCA signed by Amazon, and that I am authorized to contribute it. |
|
Thank you for signing the OCA. |
|
The admin interface was introduced to provide a path to recover/maintain an otherwise unreachable server. That's precisely why it is guarded by privilege check too and that's why there are no max-connections limits. Applications should not treat this interface as regular server-client interface. |
|
Thanks for the review, and I agree with the intent behind the admin interface that it exists so an operator can reach an otherwise unreachable server. My concern is that unlimited connections don't actually guarantee that property; in some cases they undermine it. A few points I'd ask you to consider:
In summary, Keeping the default with the current behavior and providing an option for administrators to avoid accidental issues because of automations really help avoid unforeseen issues. |
|
One way to prevent multiple administrative threads, is to use the thread plugin. AFAIU, the thread plugin will ensure that all administrative connections are mapped to a single thread. |
|
Agreed! In addition there are ways like user level connection limits like max_user_connections can also help if its implemented. These are true even in case of regular max_connections as well. The intent of this request is to close a gap where flaws in automations can lead to downtime because of lack of control on most useful feature (admin interface). Retaining the default of unlimited connections (admin_max_connections = 0), so that behavior is unchanged for anyone who prefers it today and the recovery guarantee remains intact. Provide experienced operators the option to configure a ceiling on the admin interface, so that those running multiple administrative automations can protect the server against their own automation defects. |
|
I understand the problem... Even so, it's difficult to think of a solution which doesn't simply move the same problem to the next level. (Turtles all the way down...) Just thinking out loud: Maybe it would be possible to limit the number of admin port connections such that: When the limit is reached:
This allows new connections to enter. The existing connections are treated as an error condition, since this port is intended to be used only by 1 or a few admin commands, the fact that there are admin_max_connections of them can be considered an error, and closing them aggressively is kind of a fix - in a weird way. |
|
I think one key question is - whether it is expected that an application would create boundless admin connections. The core idea of the feature is that admin connections would be only a handful and mainly made to support maintenance/recovery that's otherwise not feasible. If an application is creating a huge amount of admin connection, potentially with resource heavy operations - the recovery would hampered as it is. IMHO such a model should be avoided. Further, SERVICE_CONNECTION_ADMIN should be granted with care as it is meant for administrative actions. Lenient distribution of this privilege would create issues. See: https://dev.mysql.com/doc/refman/9.7/en/privileges-provided.html#priv_service-connection-admin Reg. resource constraints, I believe MySQL already has tools to enable it as discussed above (Thread pool, user specific resource restriction etc) and that's actually better way to solve this congestion problem. Judicial usage of SERVICE_CONNECTION_ADMIN with these policies will help contain the blast radius. Also, I understand the backward compatibility aspect but it is more of a required property, not a reason in itself. |
|
The point we need to clarify is why this environment has, or expects to have, more than a small number of concurrent admin-port connections in the first place. The interface is intended for exceptional maintenance and recovery work, not as a shared connection endpoint for monitoring, orchestration, backup, or other routine automation. If the observed problem is caused by such components holding or repeatedly creating admin-port connections, then the underlying issue appears to be how the administrative account/interface is being used. Please describe that deployment model and why those clients cannot be separated, bounded, or redirected to the normal interface. Without that context, it is difficult to conclude that a new server-wide connection limit is the appropriate solution. |
What does this change do?
The administrative connection interface (admin_address/admin_port) accepts an unlimited number of connections. Any account holding SERVICE_CONNECTION_ADMIN can therefore exhaust server threads through the administrative interface, and max_connections offers no protection there. Bug#99917 requests an option to cap number of connections on admin interface.
This change adds:
admin_max_connections: GLOBAL, dynamic ulong, range 0..100000, default 0. Caps the number of concurrent connections accepted on the administrative interface. 0 preserves the previous unlimited behaviour, so the default is fully backward compatible. max_connections and admin_max_connections are enforced independently: the former continues to govern only ordinary connections, the latter only administrative ones.
Admin_connections status variable: number of currently open administrative connections. Ordinary connections never touch it.
Admin_connection_errors_max_connections status variable: number of administrative connections refused because admin_max_connections was reached. These rejections deliberately do not increment Connection_errors_max_connections, which keeps counting only ordinary connections refused by max_connections. Threads_connected keeps counting all connections including administrative ones (unchanged).
Implementation: both counters are static members of Connection_handler_manager protected by the existing LOCK_connection_count mutex, so no new synchronization primitive or PSI key is needed. check_and_incr_conn_count() rejects an administrative connection with ER_CON_COUNT_ERROR when the cap is reached; lowering the cap below the number of open administrative connections affects new
connections only. dec_connection_count() gains an optional is_admin_connection argument (default false) so the counter stays
balanced on every disconnect path of the per-thread and one-thread handlers. Internal sessions (srv_session_service passes
ignore_max_connection_limit as the first argument) are excluded on both the check and increment side via the internal_session argument. The exported dec_connection_count() hook in thread_pool_priv.h keeps the default; a thread pool implementation serving the administrative interface would need to pass the flag through.
Why is it needed?
The new administrative interface enabled using admin_address and admin_port variables helpful in many scenarios for example "Handling too many connections" issue.
However, as of now there is no limit on number of connections that can be established using administrative interface.
"There is no limit on the number of administrative connections."
This change implements the max connections limit
How was it tested?
mysql-test/scripts/ci/mtr.shpasses locallyTests: main.admin_max_connections (functional coverage incl. status variable separation), sys_vars.admin_max_connections_basic, plus
bookkeeping updates to all_persisted_variables (450 -> 451) and mysqld--help-notwin.
Contributor checklist
scripts/ci/format.sh)AI assistance
If AI assistance was used, describe the tool(s) and extent of use:
AI Assistance:
We have used Claude Optus 4.8 for implementation of status variables related code.
Validation of complete patch to be in mysql required format
Added comments automatically in required places using AI
For automated code review and test case generation (tested far many cases than exist in MTR with real workload to ensure it has no regression)
Human Validation:
Complete Idea has been developed manually
The complete code related to addition of parameter implemented manually.
The code has been reviewed manually before submission.
Areas touched
Admin interface connection handler
mysqld.h/cc and sys_var.cc modified to include new parameter.