From 39bb8a0d3a96eef63219dff262a9306a4f59dae9 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 12 Sep 2023 18:09:58 -0400 Subject: [PATCH] Support for dual stack connections. The prior code would only connect to the first address provided by getaddrinfo(). Instead try each address in the list. If all connections fail then wait and try them all again until timeout. Currently a round robin approach is used where each connection attempt must fail before the next connection is attempted. This works fine, for example, when an ipv6 address has no route to the host, but will work less well when a host answers but doesn't respond in a timely fashion. We may consider a Happy Eyeballs approach in the future, but since pgBackRest is primarily a background process, it is not clear that slightly improved response time (in the case of failed connections) is worth the extra complexity. --- doc/xml/release/2023/2.48.xml | 19 +++++++++++++ src/common/io/socket/client.c | 43 +++++++++++++++++++++-------- test/src/module/common/ioHttpTest.c | 3 +- test/src/module/common/ioTlsTest.c | 8 ++++-- 4 files changed, 58 insertions(+), 15 deletions(-) diff --git a/doc/xml/release/2023/2.48.xml b/doc/xml/release/2023/2.48.xml index 468bcf217..e8f7b6179 100644 --- a/doc/xml/release/2023/2.48.xml +++ b/doc/xml/release/2023/2.48.xml @@ -1,6 +1,25 @@ + + + + + + + + + + + + + + + + +

Support for dual stack connections.

+ + diff --git a/src/common/io/socket/client.c b/src/common/io/socket/client.c index 3b8c25f8a..8f75e2ed3 100644 --- a/src/common/io/socket/client.c +++ b/src/common/io/socket/client.c @@ -9,6 +9,7 @@ Socket Client #include #include "common/debug.h" +#include "common/error/retry.h" #include "common/io/client.h" #include "common/io/socket/address.h" #include "common/io/socket/client.h" @@ -72,8 +73,13 @@ sckClientOpen(THIS_VOID) MEM_CONTEXT_TEMP_BEGIN() { + Wait *const wait = waitNew(this->timeoutConnect); + ErrorRetry *const errRetry = errRetryNew(); bool retry; - Wait *wait = waitNew(this->timeoutConnect); + + // Get an address list for the host + const AddressInfo *const addrInfo = addrInfoNew(this->host, this->port); + unsigned int addrInfoIdx = 0; do { @@ -83,8 +89,8 @@ sckClientOpen(THIS_VOID) TRY_BEGIN() { - // Get an address for the host. We are only going to try the first address returned. - const struct addrinfo *const addressFound = addrInfoGet(addrInfoNew(this->host, this->port), 0); + // Get next address for the host + const struct addrinfo *const addressFound = addrInfoGet(addrInfo, addrInfoIdx); // Connect to the host fd = socket(addressFound->ai_family, addressFound->ai_socktype, addressFound->ai_protocol); @@ -106,19 +112,32 @@ sckClientOpen(THIS_VOID) } CATCH_ANY() { - if (fd != -1) - close(fd); + // Close socket + close(fd); - // Retry if wait time has not expired - if (waitMore(wait)) + // Add the error retry info + errRetryAdd(errRetry); + + // Increment address info index and reset if the end has been reached. When the end has been reached sleep for a bit + // to hopefully have better chance at succeeding, otherwise continue right to the next address as long as there is + // some time left. + addrInfoIdx++; + + if (addrInfoIdx >= addrInfoSize(addrInfo)) { - LOG_DEBUG_FMT("retry %s: %s", errorTypeName(errorType()), errorMessage()); - retry = true; - - statInc(SOCKET_STAT_RETRY_STR); + addrInfoIdx = 0; + retry = waitMore(wait); } else - RETHROW(); + retry = waitRemaining(wait) > 0; + + // Error when no retries remain + if (!retry) + THROWP(errRetryType(errRetry), strZ(errRetryMessage(errRetry))); + + // Log retry + LOG_DEBUG_FMT("retry %s: %s", errorTypeName(errorType()), errorMessage()); + statInc(SOCKET_STAT_RETRY_STR); } TRY_END(); } diff --git a/test/src/module/common/ioHttpTest.c b/test/src/module/common/ioHttpTest.c index bc87b5783..a10ba5a0d 100644 --- a/test/src/module/common/ioHttpTest.c +++ b/test/src/module/common/ioHttpTest.c @@ -306,7 +306,8 @@ testRun(void) TEST_ERROR_FMT( httpRequestResponse(httpRequestNewP(client, STRDEF("GET"), STRDEF("/")), false), HostConnectError, - "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused", + "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused\n" + "[RETRY DETAIL OMITTED]", hrnServerPort(0)); HRN_FORK_BEGIN() diff --git a/test/src/module/common/ioTlsTest.c b/test/src/module/common/ioTlsTest.c index 424e00e13..bedd5f054 100644 --- a/test/src/module/common/ioTlsTest.c +++ b/test/src/module/common/ioTlsTest.c @@ -368,7 +368,9 @@ testRun(void) TEST_ASSIGN(client, sckClientNew(STRDEF("localhost"), hrnServerPort(0), 100, 100), "new client"); TEST_ERROR_FMT( - ioClientOpen(client), HostConnectError, "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused", + ioClientOpen(client), HostConnectError, + "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused\n" + "[RETRY DETAIL OMITTED]", hrnServerPort(0)); // This address should not be in use in a test environment -- if it is the test will fail @@ -451,7 +453,9 @@ testRun(void) client, tlsClientNewP(sckClientNew(STRDEF("localhost"), hrnServerPort(0), 100, 100), STRDEF("X"), 100, 100, true), "new client"); TEST_ERROR_FMT( - ioClientOpen(client), HostConnectError, "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused", + ioClientOpen(client), HostConnectError, + "unable to connect to 'localhost:%u (127.0.0.1)': [111] Connection refused\n" + "[RETRY DETAIL OMITTED]", hrnServerPort(0)); // -------------------------------------------------------------------------------------------------------------------------