sysown / proxysql

High-performance MySQL proxy with a GPL license.
http://www.proxysql.com
GNU General Public License v3.0
6.05k stars 983 forks source link

Disabling AWS RDS topology auto-discovery by default #4578

Closed nielsvogell closed 4 months ago

nielsvogell commented 4 months ago

This pull request provides an option to disable the AWS RDS MySQL Multi-AZ DB Cluster auto-discovery feature, added in v2.6.2. It also disables the feature by default and adds an additional check.

Background Currently, ProxySql regularly (by default every 1000th time checking the read_only status of a server) queries the mysql.rds_topology table on any AWS RDS DB instance or cluster in its server list to find information about database endpoints. It is designed to work with AWS RDS Multi-AZ DB Clusters, which consist of three individual RDS instances whose endpoints are stored in the mysql.rds_topology table and whose names end in -instance-1, -instance-2, and -instance-3, respectively.

Issue The mysql.rds_topology table may however be used for other purposes in the future, so the endpoints in the table cannot by default be assumed to be instances in a Multi-AZ DB Cluster. If the table contains an endpoint, ProxySQL automatically adds this endpoint to the runtime_mysql_servers table and starts routing traffic to it. Depending on what kind of endpoint is in the mysql.rds_topology, this may result in undesired behavior.

Solution To prevent accidentally reading from the mysql.rds_topology table, this PR disables the auto-discovery by default and adds an additional check to only process the information in the table if the topology matches a Multi-AZ DB Cluster deployment. To (re-)enable the auto-discovery for Multi-AZ DB Clusters, in the ProxySql Admin DB, set the global variable mysql-monitor_aws_rds_topology_discovery_interval to a value greater than zero (it used to be 1000) and load the variables to runtime. (As per the the code, the variable determines "How frequently a topology discovery is performed, e.g. a value of 500 means one topology discovery every 500 read-only checks".)

mirostauder commented 4 months ago

Automated message: PR pending admin approval for build testing

renecannao commented 4 months ago

add to whitelist

mirostauder commented 4 months ago

retest this please

nielsvogell commented 4 months ago

Sorry, when making the commit, I accidentally changed the wrong default value. This may have affected some of the tests. Others seem to fail because of missing access to the secrets for pull-requests from forks.

Will run another local test to make sure I didn't miss anything else.

nielsvogell commented 4 months ago

Done, functionality works as expected. Please let me know if there is anything else in my code that may cause tests to fail. (I don't see any logs so am not sure what causes the failures.)

nielsvogell commented 4 months ago

Thanks for reviewing. I have addressed the two comments. Let me know if there are any more issues.

JavierJF commented 4 months ago

Hi @nielsvogell, yw! Thanks for the contribution and for addressing the minor feedback. I have no further comments, the PR is ready to be merged.

mirostauder commented 4 months ago

retest this please