From bd23b15ad7281d9e59aaba4afa5120f9c55bb1a9 Mon Sep 17 00:00:00 2001 From: Moti Cohen Date: Sun, 23 Oct 2022 16:00:47 +0300 Subject: [PATCH] Fix sentinel function that compares hostnames (if failed resolve) (#11419) Funcion sentinelAddrEqualsHostname() of sentinel makes DNS resolve and based on it determines if two IP addresses are equal. Now, If the DNS resolve command fails, the function simply returns 0, even if the hostnames are identical. This might become an issue in case of failover such that sentinel might receives from Redis instance, response to regular INFO query it sent, and wrongly decide that the instance is pointing to is different leader than the one recorded because of this function, yet hostnames are identical. In turn sentinel disconnects the connection between sentinel and valid slave which leads to -failover-abort-no-good-slave. See issue #11241. I managed to reproduce only part of the flow in which the function return wrong result and trigger +fix-slave-config. The fix is even if the function failed to resolve then compare based on hostnames. That is our best effort as long as the server is unavailable for some reason. It is fine since Redis instance cannot have multiple hostnames for a given setup --- src/sentinel.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sentinel.c b/src/sentinel.c index 30d939aee..48369df99 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -616,10 +616,16 @@ int sentinelAddrOrHostnameEqual(sentinelAddr *a, sentinelAddr *b) { int sentinelAddrEqualsHostname(sentinelAddr *a, char *hostname) { char ip[NET_IP_STR_LEN]; - /* We always resolve the hostname and compare it to the address */ + /* Try resolve the hostname and compare it to the address */ if (anetResolve(NULL, hostname, ip, sizeof(ip), - sentinel.resolve_hostnames ? ANET_NONE : ANET_IP_ONLY) == ANET_ERR) - return 0; + sentinel.resolve_hostnames ? ANET_NONE : ANET_IP_ONLY) == ANET_ERR) { + + /* If failed resolve then compare based on hostnames. That is our best effort as + * long as the server is unavailable for some reason. It is fine since Redis + * instance cannot have multiple hostnames for a given setup */ + return !strcasecmp(sentinel.resolve_hostnames ? a->hostname : a->ip, hostname); + } + /* Compare based on address */ return !strcasecmp(a->ip, ip); }