Merge pull request #4662 from sysown/v2.7_dns_cache_fixes

renecannao · web-flow · commit 2726c277102c · 2024-09-20T17:51:19.000+02:00
DNS record will be removed if MySQL connection fails or times out for a specific hostname [v2.7]
diff --git a/include/MySQL_Monitor.hpp b/include/MySQL_Monitor.hpp
@@ -398,7 +398,9 @@ struct DNS_Cache_Record {
 class DNS_Cache {
 
 public:
-	DNS_Cache() : enabled(true) {
+	// By default, the DNS cache is disabled.
+	// This handles the case when ProxySQL is executed with the -M/--no-monitor option.
+	DNS_Cache() : enabled(false) {
 		int rc = pthread_rwlock_init(&rwlock_, NULL);
 		assert(rc == 0);
 	}
@@ -445,6 +447,7 @@ class MySQL_Monitor {
 	static std::string dns_lookup(const std::string& hostname, bool return_hostname_if_lookup_fails = true, size_t* ip_count = NULL);
 	static std::string dns_lookup(const char* hostname, bool return_hostname_if_lookup_fails = true, size_t* ip_count = NULL);
 	static bool update_dns_cache_from_mysql_conn(const MYSQL* mysql);
+	static void remove_dns_record_from_dns_cache(const std::string& hostname);
 	static void trigger_dns_cache_update();
 
 	void process_discovered_topology(const std::string& originating_server_hostname, const vector<MYSQL_ROW>& discovered_servers, int reader_hostgroup);
@@ -457,6 +460,7 @@ class MySQL_Monitor {
 	void drop_tables_defs(std::vector<table_def_t *> *tables_defs);
 	void check_and_build_standard_tables(SQLite3DB *db, std::vector<table_def_t *> *tables_defs);
 	static bool _dns_cache_update(const std::string& hostname, std::vector<std::string>&& ip_address);
+	static void _remove_dns_record_from_dns_cache(const std::string& hostname);
 
 	public:
 	pthread_mutex_t group_replication_mutex; // for simplicity, a mutex instead of a rwlock
diff --git a/include/MySQL_Session.h b/include/MySQL_Session.h
@@ -473,6 +473,8 @@ class KillArgs {
 	KillArgs(char *u, char *p, char *h, unsigned int P, unsigned int _hid, unsigned long i, int kt, int _use_ssl, MySQL_Thread* _mt, char *ip);
 	~KillArgs();
 	const char* get_host_address() const;
+	void resolve_hostname();
+	void remove_dns_record();
 
 private:
 	char* ip_addr;
diff --git a/include/ProxySQL_Cluster.hpp b/include/ProxySQL_Cluster.hpp
@@ -104,6 +104,7 @@ class ProxySQL_Node_Address {
 	~ProxySQL_Node_Address();
 	const char* get_host_address() const;
 	void resolve_hostname();
+	void remove_dns_record();
 private:
 	char* ip_addr;
 };
diff --git a/lib/MySQL_Monitor.cpp b/lib/MySQL_Monitor.cpp
@@ -1564,6 +1564,10 @@ bool MySQL_Monitor_State_Data::create_new_connection() {
 			myrc=mysql_real_connect(mysql, "localhost", mysql_thread___monitor_username, mysql_thread___monitor_password, NULL, 0, hostname, 0);
 		}
 		if (myrc==NULL) {
+			// port == 0 means we are connecting to a unix socket
+			if (port) {
+				MySQL_Monitor::remove_dns_record_from_dns_cache(hostname);
+			}
 			mysql_error_msg=strdup(mysql_error(mysql));
 			int myerrno=mysql_errno(mysql);
 			MyHGM->p_update_mysql_error_counter(p_mysql_error_type::proxysql, hostgroup_id, hostname, port, myerrno);
@@ -6670,6 +6674,15 @@ bool MySQL_Monitor::update_dns_cache_from_mysql_conn(const MYSQL* mysql)
 	return result;
 }
 
+void MySQL_Monitor::remove_dns_record_from_dns_cache(const std::string& hostname) {
+
+	// if IP was provided, no need to update dns cache
+	if (hostname.empty() || validate_ip(hostname))
+		return;
+
+	_remove_dns_record_from_dns_cache(hostname);
+}
+
 bool MySQL_Monitor::_dns_cache_update(const std::string &hostname, std::vector<std::string>&& ip_address) {
 	static thread_local std::shared_ptr<DNS_Cache> dns_cache_thread;
 
@@ -6686,6 +6699,18 @@ bool MySQL_Monitor::_dns_cache_update(const std::string &hostname, std::vector<s
 	return false;
 }
 
+void MySQL_Monitor::_remove_dns_record_from_dns_cache(const std::string& hostname) {
+	static thread_local std::shared_ptr<DNS_Cache> dns_cache_thread;
+
+	if (!dns_cache_thread && GloMyMon)
+		dns_cache_thread = GloMyMon->dns_cache;
+
+	if (dns_cache_thread) {
+		dns_cache_thread->remove(trim(hostname));
+		proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Direct DNS cache record removed. (Hostname:[%s])\n", hostname.c_str());
+	}
+}
+
 void MySQL_Monitor::trigger_dns_cache_update() {
 	if (GloMyMon) {
 		GloMyMon->force_dns_cache_update = true;
@@ -6714,19 +6739,20 @@ bool DNS_Cache::add(const std::string& hostname, std::vector<std::string>&& ips)
 
 bool DNS_Cache::add_if_not_exist(const std::string& hostname, std::vector<std::string>&& ips) {
 	if (!enabled) return false;
-
+	bool item_added = false;
 	int rc = pthread_rwlock_wrlock(&rwlock_);
 	assert(rc == 0);
 	if (records.find(hostname) == records.end()) {
 		proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Updating DNS cache. (Hostname:[%s] IP:[%s])\n", hostname.c_str(), debug_iplisttostring(ips).c_str());
 		auto& ip_addr = records[hostname];
 		ip_addr.ips = std::move(ips);
 		__sync_fetch_and_and(&ip_addr.counter, 0);
+		item_added = true;
 	}
 	rc = pthread_rwlock_unlock(&rwlock_);
 	assert(rc == 0);
 
-	if (GloMyMon)
+	if (item_added && GloMyMon)
 		__sync_fetch_and_add(&GloMyMon->dns_cache_record_updated, 1);
 
 	return true;
diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp
@@ -209,13 +209,7 @@ bool Session_Regex::match(char *m) {
 KillArgs::KillArgs(char* u, char* p, char* h, unsigned int P, unsigned int _hid, unsigned long i, int kt, int _use_ssl, MySQL_Thread* _mt) :
 	KillArgs(u, p, h, P, _hid, i, kt, _use_ssl, _mt, NULL) {
 	// resolving DNS if available in Cache
-	if (h && P) {
-		const std::string& ip = MySQL_Monitor::dns_lookup(h, false);
-
-		if (ip.empty() == false) {
-			ip_addr = strdup(ip.c_str());
-		}
-	}
+	resolve_hostname();
 }
 
 KillArgs::KillArgs(char* u, char* p, char* h, unsigned int P, unsigned int _hid, unsigned long i, int kt, int _use_ssl, MySQL_Thread *_mt, char *ip) {
@@ -250,6 +244,25 @@ const char* KillArgs::get_host_address() const {
 	return host_address;
 }
 
+void KillArgs::resolve_hostname() {
+	if (ip_addr) {
+		free(ip_addr);
+		ip_addr = NULL;
+	}
+	if (hostname && port) {
+		const std::string& ip = MySQL_Monitor::dns_lookup(hostname, false);
+
+		if (ip.empty() == false) {
+			ip_addr = strdup(ip.c_str());
+		}
+	}
+}
+
+void KillArgs::remove_dns_record() {
+	if (hostname && port) {
+		MySQL_Monitor::remove_dns_record_from_dns_cache(hostname);
+	}
+}
 
 /**
  * @brief Thread function to kill a query or connection on a MySQL server.
@@ -325,6 +338,8 @@ void* kill_query_thread(void *arg) {
 		ret=mysql_real_connect(mysql,"localhost",ka->username,ka->password,NULL,0,ka->hostname,0);
 	}
 	if (!ret) {
+		ka->remove_dns_record();
+		//ka->resolve_hostname();
 		int myerr = mysql_errno(mysql);
 		if (ssl_params != NULL && myerr == 2026) {
 			proxy_error("Failed to connect to server %s:%d to run KILL %s %lu.  SSL Params: %s , %s , %s , %s , %s , %s , %s , %s\n",
diff --git a/lib/ProxySQL_Cluster.cpp b/lib/ProxySQL_Cluster.cpp
@@ -242,6 +242,7 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) {
 				}
 			} else {
 				proxy_warning("Cluster: unable to connect to peer %s:%d . Error: %s\n", node->hostname, node->port, mysql_error(conn));
+				node->remove_dns_record();
 				node->resolve_hostname();
 				mysql_close(conn);
 				conn = mysql_init(NULL);
@@ -4482,3 +4483,11 @@ void ProxySQL_Node_Address::resolve_hostname() {
 		}
 	}
 }
+
+void ProxySQL_Node_Address::remove_dns_record() {
+	// make sure hostname is not NULL and port is not 0 (UNIX socket)
+	if (hostname && port) {
+		MySQL_Monitor::remove_dns_record_from_dns_cache(hostname);
+	}
+}
+
diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp
@@ -1278,11 +1278,19 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) {
 			//}
 			MySQL_Monitor::update_dns_cache_from_mysql_conn(mysql);
 			break;
-		case ASYNC_CONNECT_FAILED:
+		case ASYNC_CONNECT_FAILED: 
+			// port == 0 means we are connecting to a unix socket
+			if (parent->port) {
+				MySQL_Monitor::remove_dns_record_from_dns_cache(parent->address);
+			}
 			MyHGM->p_update_mysql_error_counter(p_mysql_error_type::mysql, parent->myhgc->hid, parent->address, parent->port, mysql_errno(mysql));
 			parent->connect_error(mysql_errno(mysql));
 			break;
 		case ASYNC_CONNECT_TIMEOUT:
+			// port == 0 means we are connecting to a unix socket
+			if (parent->port) {
+				MySQL_Monitor::remove_dns_record_from_dns_cache(parent->address);
+			}
 			//proxy_error("Connect timeout on %s:%d : %llu - %llu = %llu\n",  parent->address, parent->port, myds->sess->thread->curtime , myds->wait_until, myds->sess->thread->curtime - myds->wait_until);
 			proxy_error("Connect timeout on %s:%d : exceeded by %lluus\n", parent->address, parent->port, myds->sess->thread->curtime - myds->wait_until);
 			MyHGM->p_update_mysql_error_counter(p_mysql_error_type::mysql, parent->myhgc->hid, parent->address, parent->port, mysql_errno(mysql));

Original file line number	Diff line number	Diff line change
`@@ -242,6 +242,7 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) {`
`242`	`242`	`}`
`243`	`243`	`} else {`
`244`	`244`	`proxy_warning("Cluster: unable to connect to peer %s:%d . Error: %s\n", node->hostname, node->port, mysql_error(conn));`
	`245`	`+ node->remove_dns_record();`
`245`	`246`	`node->resolve_hostname();`
`246`	`247`	`mysql_close(conn);`
`247`	`248`	`conn = mysql_init(NULL);`
`@@ -4482,3 +4483,11 @@ void ProxySQL_Node_Address::resolve_hostname() {`
`4482`	`4483`	`}`
`4483`	`4484`	`}`
`4484`	`4485`	`}`
	`4486`	`+`
	`4487`	`+void ProxySQL_Node_Address::remove_dns_record() {`
	`4488`	`+ // make sure hostname is not NULL and port is not 0 (UNIX socket)`
	`4489`	`+ if (hostname && port) {`
	`4490`	`+ MySQL_Monitor::remove_dns_record_from_dns_cache(hostname);`
	`4491`	`+ }`
	`4492`	`+}`
	`4493`	`+`