db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-09-16 09:06:18 +02:00
Code Issues Releases Activity

Unnest HTTP/TLS/socket timeouts.

Browse Source 
These timeouts were nested, i.e. an error in the socket connection would also retry in the TLS and HTTP layers. This led to a multiplying effect such that it took nine minutes to fully timeout with the default io-timeout of 60 seconds.

A fix for this was attempted in 5314dbff but it reduced retries by too much and had to be reverted in fa5b2d44.

Instead fix by moving connection attempts to the lower layer (e.g. TLS -> socket) out of the exception block but leave it within the retry loop. So, for example, if the socket connection fails after retries then the error will not be retried by TLS. But if the TLS session fails the socket will be reconnected.
This commit is contained in:
David Steele
2025-09-05 11:42:00 -04:00
committed by GitHub
parent adc5e5b23b
commit 9c7f064864
7 changed files with 79 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
doc/xml/release/2025/2.57.0.xml
View File

@@ -1,6 +1,18 @@
<release date="XXXX-XX-XX" version="2.57.0dev" title="Under Development">
<release-core-list>
<release-bug-list>
<release-item>
<github-issue id="2654"/>
<github-pull-request id="2662"/>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="david.christensen"/>
</release-item-contributor-list>
<p>Unnest HTTP/TLS/socket timeouts.</p>
</release-item>
<release-item>
<github-pull-request id="2675"/>

29
src/common/io/http/request.c
View File

@@ -136,6 +136,21 @@ httpRequestProcess(HttpRequest *const this, const bool waitForResponse, const bo
do
{
// Create a session to process the request. This is done outside exception handling to avoid multiplying retries.
HttpSession *session = NULL;
bool send = true;
// If a session is saved then the request was already successfully sent
if (this->session != NULL)
{
session = httpSessionMove(this->session, memContextCurrent());
this->session = NULL;
send = false;
}
// Else the request has not been sent yet or this is a retry
else
session = httpClientOpen(this->client);
// Assume there will be no retry
retry = false;
@@ -143,19 +158,9 @@ httpRequestProcess(HttpRequest *const this, const bool waitForResponse, const bo
{
MEM_CONTEXT_TEMP_BEGIN()
{
HttpSession *session = NULL;
// If a session is saved then the request was already successfully sent
if (this->session != NULL)
// Send the request
if (send)
{
session = httpSessionMove(this->session, memContextCurrent());
this->session = NULL;
}
// Else the request has not been sent yet or this is a retry
else
{
session = httpClientOpen(this->client);
// Write the request as a buffer so secrets do not show up in logs
ioWrite(
httpSessionIoWrite(session),

30
src/common/io/tls/client.c
View File

@@ -327,24 +327,24 @@ tlsClientOpen(THIS_VOID)
cryptoError(SSL_set_tlsext_host_name(tlsSession, strZ(this->host)) != 1, "unable to set TLS host name");
#pragma GCC diagnostic pop
// Open the underlying session first since this is mostly likely to fail. This is done outside exception handling to
// avoid multiplying retries.
IoSession *ioSession = NULL;
TRY_BEGIN()
{
ioSession = ioClientOpen(this->ioClient);
}
CATCH_ANY()
{
SSL_free(tlsSession);
RETHROW();
}
TRY_END();
// Open TLS session
TRY_BEGIN()
{
// Open the underlying session first since this is mostly likely to fail
IoSession *ioSession = NULL;
TRY_BEGIN()
{
ioSession = ioClientOpen(this->ioClient);
}
CATCH_ANY()
{
SSL_free(tlsSession);
RETHROW();
}
TRY_END();
// Open session
result = tlsSessionNew(tlsSession, ioSession, this->timeoutSession);
}
CATCH_ANY()

12
test/src/common/harnessServer.c
View File

@@ -300,7 +300,19 @@ hrnServerRun(IoRead *const read, const HrnServerProtocol protocol, const unsigne
// Start TLS if requested
if (protocol == hrnServerProtocolTls)
{
// Generate as many TLS errors as requested
if (param.tlsErrorTotal > 0)
{
ioSessionClose(serverSession);
ioSessionFree(serverSession);
param.tlsErrorTotal--;
serverSession = ioServerAccept(socketServer, NULL);
}
serverSession = ioServerAccept(tlsServer, serverSession);
}
break;
}

1
test/src/common/harnessServer.h
View File

@@ -58,6 +58,7 @@ typedef struct HrnServerRunParam
const String *certificate; // TLS certificate when protocol = hrnServerProtocolTls
const String *key; // TLS key when protocol = hrnServerProtocolTls
const String *address; // Use address other than 127.0.0.1
unsigned int tlsErrorTotal; // Total TLS errors to cause before success
} HrnServerRunParam;
#define hrnServerRunP(read, protocol, port, ...) \

1
test/src/module/common/ioHttpTest.c
View File

@@ -333,7 +333,6 @@ testRun(void)
TEST_ERROR_FMT(
httpRequestResponse(httpRequestNewP(client, STRDEF("GET"), STRDEF("/")), false), HostConnectError,
"unable to connect to 'localhost:34342 (127.0.0.1)': [111] Connection refused\n"
"[RETRY DETAIL OMITTED]\n"
"[RETRY DETAIL OMITTED]");
HRN_FORK_BEGIN()

24
test/src/module/common/ioTlsTest.c
View File

@@ -1113,7 +1113,8 @@ testRun(void)
HRN_FORK_CHILD_BEGIN(.prefix = "test server", .timeout = 5000)
{
TEST_RESULT_VOID(hrnServerRunP(HRN_FORK_CHILD_READ(), hrnServerProtocolTls, testPort), "tls server");
TEST_RESULT_VOID(
hrnServerRunP(HRN_FORK_CHILD_READ(), hrnServerProtocolTls, testPort, .tlsErrorTotal = 2), "tls server");
}
HRN_FORK_CHILD_END();
@@ -1122,6 +1123,9 @@ testRun(void)
IoWrite *tls = hrnServerScriptBegin(HRN_FORK_PARENT_WRITE(0));
ioBufferSizeSet(12);
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("connect failure");
TEST_ASSIGN(
client,
tlsClientNewP(
@@ -1130,6 +1134,22 @@ testRun(void)
hrnServerScriptAccept(tls);
TEST_ERROR_MULTI(
ioClientOpen(client), ServiceError,
// TLS >= 3
"TLS error [1:167772454] unexpected eof while reading",
// TLS < 3 and Alpine
"TLS error [5:0] no details available");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("connect success (with retry from prior accept)");
TEST_ASSIGN(
client,
tlsClientNewP(
sckClientNew(hrnServerHost(), testPort, 5000, 5000), hrnServerHost(), 5000, 0, TEST_IN_CONTAINER),
"new client");
TEST_ASSIGN(session, ioClientOpen(client), "open client");
TlsSession *tlsSession = (TlsSession *)session->pub.driver;
@@ -1145,7 +1165,7 @@ testRun(void)
buffer,
zNewFmt(
"{type: tls, driver: {ioClient: {type: socket, driver: {host: %s, port: %u, timeoutConnect: 5000"
", timeoutSession: 5000}}, timeoutConnect: 0, timeoutSession: 0, verifyPeer: %s}}",
", timeoutSession: 5000}}, timeoutConnect: 5000, timeoutSession: 0, verifyPeer: %s}}",
strZ(hrnServerHost()), testPort, cvtBoolToConstZ(TEST_IN_CONTAINER)),
"check log");