> Just a quick single-issue review, but I agree with Maxim that having
> one PRNG, seeded once, would be simpler
I don't agree that it's simpler. Because now there's a mutex you have
to manage, and honestly cross-platform threading in C is not simple.
However, I attached two additional patches that implement this
approach on top of the previous patchset. Just to make sure that
this patch is not blocked on this.
> with the tangible benefit that it would eliminate weird behavior on
> simultaneous connections when the client isn't using OpenSSL.
This is true, but still I think in practice very few people have a libpq
that's compiled without OpenSSL support.
> I'm guessing a simple lock on a
> global PRNG would be less overhead than the per-connection
> strong_random machinery, too, but I have no data to back that up.
It might very well have less overhead, but neither of them should take
up any significant amount of time during connection establishment.
> The test seed could then be handled globally as well (envvar?) so that you
> don't have to introduce a debug-only option into the connection string.
Why is a debug-only envvar any better than a debug-only connection option?
For now I kept the connection option approach, since to me they seem pretty
much equivalent.
From 561dca3b9510464600b4da8d1397e7762f523568 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <jelte.fennema@microsoft.com>
Date: Fri, 13 Jan 2023 16:52:17 +0100
Subject: [PATCH v7 3/4] Make mutexes easier to use in libpq
For process global mutexes windows requires some different setup than
other OSes. This abstracts away that logic into the newly added pglock
and pgunlock functions. The main reason to do this refactoring is to
prepare for a new global mutex that will be added in a follow up commit.
---
src/interfaces/libpq/fe-connect.c | 58 +++++++++++++++---------
src/interfaces/libpq/fe-secure-openssl.c | 33 +++-----------
src/interfaces/libpq/libpq-int.h | 21 +++++++++
3 files changed, 64 insertions(+), 48 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 18a07d810dc..69ed891703a 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7424,36 +7424,17 @@ pqGetHomeDirectory(char *buf, int bufsize)
static void
default_threadlock(int acquire)
{
-#ifdef ENABLE_THREAD_SAFETY
-#ifndef WIN32
- static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
-#else
- static pthread_mutex_t singlethread_lock = NULL;
- static long mutex_initlock = 0;
-
- if (singlethread_lock == NULL)
- {
- while (InterlockedExchange(&mutex_initlock, 1) == 1)
- /* loop, another thread own the lock */ ;
- if (singlethread_lock == NULL)
- {
- if (pthread_mutex_init(&singlethread_lock, NULL))
- Assert(false);
- }
- InterlockedExchange(&mutex_initlock, 0);
- }
-#endif
+ static pglock_t singlethread_lock = PGLOCK_INITIALIZER;
if (acquire)
{
- if (pthread_mutex_lock(&singlethread_lock))
+ if (!pglock(&singlethread_lock))
Assert(false);
}
else
{
- if (pthread_mutex_unlock(&singlethread_lock))
+ if (!pgunlock(&singlethread_lock))
Assert(false);
}
-#endif
}
pgthreadlock_t
@@ -7468,3 +7449,36 @@ PQregisterThreadLock(pgthreadlock_t newhandler)
return prev;
}
+
+bool
+pglock(pglock_t * lock)
+{
+#ifdef WIN32
+ if (lock->mutex == NULL)
+ {
+ while (InterlockedExchange(&lock->mutex_initlock, 1) == 1)
+ /* loop, another thread own the lock */ ;
+ if (lock->mutex == NULL)
+ {
+ if (pthread_mutex_init(&lock->mutex, NULL))
+ return false;
+ }
+ InterlockedExchange(&lock->mutex_initlock, 0);
+ }
+#endif
+ if (pthread_mutex_lock(&lock->mutex))
+ {
+ return false;
+ }
+ return true;
+}
+
+bool
+pgunlock(pglock_t * lock)
+{
+ if (pthread_mutex_unlock(&lock->mutex))
+ {
+ return false;
+ }
+ return true;
+}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index ab2cbf045b8..c52c2ccf217 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -94,12 +94,7 @@ static bool ssl_lib_initialized = false;
#ifdef ENABLE_THREAD_SAFETY
static long crypto_open_connections = 0;
-#ifndef WIN32
-static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
-#else
-static pthread_mutex_t ssl_config_mutex = NULL;
-static long win32_ssl_create_mutex = 0;
-#endif
+static pglock_t ssl_config_lock = PGLOCK_INITIALIZER;
#endif /* ENABLE_THREAD_SAFETY */
static PQsslKeyPassHook_OpenSSL_type PQsslKeyPassHook = NULL;
@@ -744,21 +739,7 @@ int
pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto)
{
#ifdef ENABLE_THREAD_SAFETY
-#ifdef WIN32
- /* Also see similar code in fe-connect.c, default_threadlock() */
- if (ssl_config_mutex == NULL)
- {
- while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
- /* loop, another thread own the lock */ ;
- if (ssl_config_mutex == NULL)
- {
- if (pthread_mutex_init(&ssl_config_mutex, NULL))
- return -1;
- }
- InterlockedExchange(&win32_ssl_create_mutex, 0);
- }
-#endif
- if (pthread_mutex_lock(&ssl_config_mutex))
+ if (!pglock(&ssl_config_lock))
return -1;
#ifdef HAVE_CRYPTO_LOCK
@@ -775,7 +756,7 @@ pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto)
pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
if (!pq_lockarray)
{
- pthread_mutex_unlock(&ssl_config_mutex);
+ pgunlock(&ssl_config_lock);
return -1;
}
for (i = 0; i < CRYPTO_num_locks(); i++)
@@ -784,7 +765,7 @@ pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto)
{
free(pq_lockarray);
pq_lockarray = NULL;
- pthread_mutex_unlock(&ssl_config_mutex);
+ pgunlock(&ssl_config_lock);
return -1;
}
}
@@ -827,7 +808,7 @@ pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto)
}
#ifdef ENABLE_THREAD_SAFETY
- pthread_mutex_unlock(&ssl_config_mutex);
+ pgunlock(&ssl_config_lock);
#endif
return 0;
}
@@ -849,7 +830,7 @@ destroy_ssl_system(void)
{
#if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_CRYPTO_LOCK)
/* Mutex is created in pgtls_init() */
- if (pthread_mutex_lock(&ssl_config_mutex))
+ if (!pglock(&ssl_config_lock))
return;
if (pq_init_crypto_lib && crypto_open_connections > 0)
@@ -875,7 +856,7 @@ destroy_ssl_system(void)
*/
}
- pthread_mutex_unlock(&ssl_config_mutex);
+ pgunlock(&ssl_config_lock);
#endif
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 86dd1d6d405..01dd4190f33 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -672,7 +672,28 @@ extern pgthreadlock_t pg_g_threadlock;
#define pglock_thread() pg_g_threadlock(true)
#define pgunlock_thread() pg_g_threadlock(false)
+
+typedef struct pglock_t
+{
+ pthread_mutex_t mutex;
+#ifdef WIN32
+ long mutex_initlock;
+#endif
+} pglock_t;
+
+#ifdef WIN32
+#define PGLOCK_INITIALIZER { NULL, 0 }
+#else
+#define PGLOCK_INITIALIZER { PTHREAD_MUTEX_INITIALIZER }
+#endif
+
+extern bool pglock(pglock_t * lock);
+extern bool pgunlock(pglock_t * lock);
#else
+#define pglock_t bool;
+#define PGLOCK_INITIALIZER false;
+#define pglock(lock) ((void) true)
+#define pgunlock(lock) ((void) true)
#define pglock_thread() ((void) 0)
#define pgunlock_thread() ((void) 0)
#endif
--
2.34.1
From c10deacdab931ed30f9c35594e8c99f787989387 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <jelte.fennema@microsoft.com>
Date: Wed, 30 Nov 2022 10:07:19 +0100
Subject: [PATCH v7 1/4] libpq: Run pgindent after a9e9a9f32b3
It seems that pgindent was not run after the error handling refactor in
commit a9e9a9f32b35edf129c88e8b929ef223f8511f59. This fixes that and
also addresses a few other things pgindent wanted to change in libpq.
---
src/interfaces/libpq/fe-auth-scram.c | 2 +-
src/interfaces/libpq/fe-auth.c | 8 +-
src/interfaces/libpq/fe-connect.c | 124 +++++++++++------------
src/interfaces/libpq/fe-exec.c | 16 +--
src/interfaces/libpq/fe-lobj.c | 42 ++++----
src/interfaces/libpq/fe-misc.c | 10 +-
src/interfaces/libpq/fe-protocol3.c | 2 +-
src/interfaces/libpq/fe-secure-common.c | 6 +-
src/interfaces/libpq/fe-secure-gssapi.c | 12 +--
src/interfaces/libpq/fe-secure-openssl.c | 64 ++++++------
src/interfaces/libpq/fe-secure.c | 8 +-
src/interfaces/libpq/libpq-int.h | 4 +-
12 files changed, 149 insertions(+), 149 deletions(-)
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 9c42ea4f819..12c3d0bc333 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -716,7 +716,7 @@ read_server_final_message(fe_scram_state *state, char *input)
return false;
}
libpq_append_conn_error(conn, "error received from server in SCRAM exchange: %s",
- errmsg);
+ errmsg);
return false;
}
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 9afc6f19b9a..ab454e6cd02 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -73,7 +73,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
if (!ginbuf.value)
{
libpq_append_conn_error(conn, "out of memory allocating GSSAPI buffer (%d)",
- payloadlen);
+ payloadlen);
return STATUS_ERROR;
}
if (pqGetnchar(ginbuf.value, payloadlen, conn))
@@ -223,7 +223,7 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
if (!inputbuf)
{
libpq_append_conn_error(conn, "out of memory allocating SSPI buffer (%d)",
- payloadlen);
+ payloadlen);
return STATUS_ERROR;
}
if (pqGetnchar(inputbuf, payloadlen, conn))
@@ -623,7 +623,7 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
if (!challenge)
{
libpq_append_conn_error(conn, "out of memory allocating SASL buffer (%d)",
- payloadlen);
+ payloadlen);
return STATUS_ERROR;
}
@@ -1277,7 +1277,7 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
else
{
libpq_append_conn_error(conn, "unrecognized password encryption algorithm \"%s\"",
- algorithm);
+ algorithm);
return NULL;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 50b5df3490b..773e9e1f3a2 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1079,7 +1079,7 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "could not match %d host names to %d hostaddr values",
- count_comma_separated_elems(conn->pghost), conn->nconnhost);
+ count_comma_separated_elems(conn->pghost), conn->nconnhost);
return false;
}
}
@@ -1159,7 +1159,7 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "could not match %d port numbers to %d hosts",
- count_comma_separated_elems(conn->pgport), conn->nconnhost);
+ count_comma_separated_elems(conn->pgport), conn->nconnhost);
return false;
}
}
@@ -1248,7 +1248,7 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
- "channel_binding", conn->channel_binding);
+ "channel_binding", conn->channel_binding);
return false;
}
}
@@ -1273,7 +1273,7 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
- "sslmode", conn->sslmode);
+ "sslmode", conn->sslmode);
return false;
}
@@ -1293,7 +1293,7 @@ connectOptions2(PGconn *conn)
case 'v': /* "verify-ca" or "verify-full" */
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "sslmode value \"%s\" invalid when SSL support is not compiled in",
- conn->sslmode);
+ conn->sslmode);
return false;
}
#endif
@@ -1313,16 +1313,16 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
- "ssl_min_protocol_version",
- conn->ssl_min_protocol_version);
+ "ssl_min_protocol_version",
+ conn->ssl_min_protocol_version);
return false;
}
if (!sslVerifyProtocolVersion(conn->ssl_max_protocol_version))
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
- "ssl_max_protocol_version",
- conn->ssl_max_protocol_version);
+ "ssl_max_protocol_version",
+ conn->ssl_max_protocol_version);
return false;
}
@@ -1359,7 +1359,7 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "gssencmode value \"%s\" invalid when GSSAPI support is not compiled in",
- conn->gssencmode);
+ conn->gssencmode);
return false;
}
#endif
@@ -1392,8 +1392,8 @@ connectOptions2(PGconn *conn)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
- "target_session_attrs",
- conn->target_session_attrs);
+ "target_session_attrs",
+ conn->target_session_attrs);
return false;
}
}
@@ -1609,7 +1609,7 @@ connectNoDelay(PGconn *conn)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "could not set socket to TCP no delay mode: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
}
#endif
@@ -1787,7 +1787,7 @@ parse_int_param(const char *value, int *result, PGconn *conn,
error:
libpq_append_conn_error(conn, "invalid integer value \"%s\" for connection option \"%s\"",
- value, context);
+ value, context);
return false;
}
@@ -1816,9 +1816,9 @@ setKeepalivesIdle(PGconn *conn)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "%s(%s) failed: %s",
- "setsockopt",
- PG_TCP_KEEPALIVE_IDLE_STR,
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ "setsockopt",
+ PG_TCP_KEEPALIVE_IDLE_STR,
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
}
#endif
@@ -1850,9 +1850,9 @@ setKeepalivesInterval(PGconn *conn)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "%s(%s) failed: %s",
- "setsockopt",
- "TCP_KEEPINTVL",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ "setsockopt",
+ "TCP_KEEPINTVL",
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
}
#endif
@@ -1885,9 +1885,9 @@ setKeepalivesCount(PGconn *conn)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "%s(%s) failed: %s",
- "setsockopt",
- "TCP_KEEPCNT",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ "setsockopt",
+ "TCP_KEEPCNT",
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
}
#endif
@@ -1949,8 +1949,8 @@ prepKeepalivesWin32(PGconn *conn)
if (!setKeepalivesWin32(conn->sock, idle, interval))
{
libpq_append_conn_error(conn, "%s(%s) failed: error code %d",
- "WSAIoctl", "SIO_KEEPALIVE_VALS",
- WSAGetLastError());
+ "WSAIoctl", "SIO_KEEPALIVE_VALS",
+ WSAGetLastError());
return 0;
}
return 1;
@@ -1983,9 +1983,9 @@ setTCPUserTimeout(PGconn *conn)
char sebuf[256];
libpq_append_conn_error(conn, "%s(%s) failed: %s",
- "setsockopt",
- "TCP_USER_TIMEOUT",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ "setsockopt",
+ "TCP_USER_TIMEOUT",
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
}
#endif
@@ -2354,7 +2354,7 @@ keep_going: /* We will come back to here until there is
if (ret || !conn->addrlist)
{
libpq_append_conn_error(conn, "could not translate host name \"%s\" to address: %s",
- ch->host, gai_strerror(ret));
+ ch->host, gai_strerror(ret));
goto keep_going;
}
break;
@@ -2366,7 +2366,7 @@ keep_going: /* We will come back to here until there is
if (ret || !conn->addrlist)
{
libpq_append_conn_error(conn, "could not parse network address \"%s\": %s",
- ch->hostaddr, gai_strerror(ret));
+ ch->hostaddr, gai_strerror(ret));
goto keep_going;
}
break;
@@ -2377,8 +2377,8 @@ keep_going: /* We will come back to here until there is
if (strlen(portstr) >= UNIXSOCK_PATH_BUFLEN)
{
libpq_append_conn_error(conn, "Unix-domain socket path \"%s\" is too long (maximum %d bytes)",
- portstr,
- (int) (UNIXSOCK_PATH_BUFLEN - 1));
+ portstr,
+ (int) (UNIXSOCK_PATH_BUFLEN - 1));
goto keep_going;
}
@@ -2391,7 +2391,7 @@ keep_going: /* We will come back to here until there is
if (ret || !conn->addrlist)
{
libpq_append_conn_error(conn, "could not translate Unix-domain socket path \"%s\" to address: %s",
- portstr, gai_strerror(ret));
+ portstr, gai_strerror(ret));
goto keep_going;
}
break;
@@ -2513,7 +2513,7 @@ keep_going: /* We will come back to here until there is
}
emitHostIdentityInfo(conn, host_addr);
libpq_append_conn_error(conn, "could not create socket: %s",
- SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)));
goto error_return;
}
@@ -2543,7 +2543,7 @@ keep_going: /* We will come back to here until there is
if (!pg_set_noblock(conn->sock))
{
libpq_append_conn_error(conn, "could not set socket to nonblocking mode: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
conn->try_next_addr = true;
goto keep_going;
}
@@ -2552,7 +2552,7 @@ keep_going: /* We will come back to here until there is
if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1)
{
libpq_append_conn_error(conn, "could not set socket to close-on-exec mode: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
conn->try_next_addr = true;
goto keep_going;
}
@@ -2581,9 +2581,9 @@ keep_going: /* We will come back to here until there is
(char *) &on, sizeof(on)) < 0)
{
libpq_append_conn_error(conn, "%s(%s) failed: %s",
- "setsockopt",
- "SO_KEEPALIVE",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ "setsockopt",
+ "SO_KEEPALIVE",
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
err = 1;
}
else if (!setKeepalivesIdle(conn)
@@ -2708,7 +2708,7 @@ keep_going: /* We will come back to here until there is
(char *) &optval, &optlen) == -1)
{
libpq_append_conn_error(conn, "could not get socket error status: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
}
else if (optval != 0)
@@ -2735,7 +2735,7 @@ keep_going: /* We will come back to here until there is
&conn->laddr.salen) < 0)
{
libpq_append_conn_error(conn, "could not get client address from socket: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
}
@@ -2775,7 +2775,7 @@ keep_going: /* We will come back to here until there is
libpq_append_conn_error(conn, "requirepeer parameter is not supported on this platform");
else
libpq_append_conn_error(conn, "could not get peer credentials: %s",
- strerror_r(errno, sebuf, sizeof(sebuf)));
+ strerror_r(errno, sebuf, sizeof(sebuf)));
goto error_return;
}
@@ -2788,7 +2788,7 @@ keep_going: /* We will come back to here until there is
if (strcmp(remote_username, conn->requirepeer) != 0)
{
libpq_append_conn_error(conn, "requirepeer specifies \"%s\", but actual peer user name is \"%s\"",
- conn->requirepeer, remote_username);
+ conn->requirepeer, remote_username);
free(remote_username);
goto error_return;
}
@@ -2829,7 +2829,7 @@ keep_going: /* We will come back to here until there is
if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
{
libpq_append_conn_error(conn, "could not send GSSAPI negotiation packet: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
}
@@ -2840,7 +2840,7 @@ keep_going: /* We will come back to here until there is
else if (!conn->gctx && conn->gssencmode[0] == 'r')
{
libpq_append_conn_error(conn,
- "GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)");
+ "GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)");
goto error_return;
}
#endif
@@ -2882,7 +2882,7 @@ keep_going: /* We will come back to here until there is
if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
{
libpq_append_conn_error(conn, "could not send SSL negotiation packet: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
}
/* Ok, wait for response */
@@ -2911,7 +2911,7 @@ keep_going: /* We will come back to here until there is
if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK)
{
libpq_append_conn_error(conn, "could not send startup packet: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
free(startpacket);
goto error_return;
}
@@ -3012,7 +3012,7 @@ keep_going: /* We will come back to here until there is
else
{
libpq_append_conn_error(conn, "received invalid response to SSL negotiation: %c",
- SSLok);
+ SSLok);
goto error_return;
}
}
@@ -3123,7 +3123,7 @@ keep_going: /* We will come back to here until there is
else if (gss_ok != 'G')
{
libpq_append_conn_error(conn, "received invalid response to GSSAPI negotiation: %c",
- gss_ok);
+ gss_ok);
goto error_return;
}
}
@@ -3201,7 +3201,7 @@ keep_going: /* We will come back to here until there is
if (!(beresp == 'R' || beresp == 'v' || beresp == 'E'))
{
libpq_append_conn_error(conn, "expected authentication request from server, but received %c",
- beresp);
+ beresp);
goto error_return;
}
@@ -3216,17 +3216,17 @@ keep_going: /* We will come back to here until there is
* Try to validate message length before using it.
* Authentication requests can't be very large, although GSS
* auth requests may not be that small. Same for
- * NegotiateProtocolVersion. Errors can be a
- * little larger, but not huge. If we see a large apparent
- * length in an error, it means we're really talking to a
- * pre-3.0-protocol server; cope. (Before version 14, the
- * server also used the old protocol for errors that happened
- * before processing the startup packet.)
+ * NegotiateProtocolVersion. Errors can be a little larger,
+ * but not huge. If we see a large apparent length in an
+ * error, it means we're really talking to a pre-3.0-protocol
+ * server; cope. (Before version 14, the server also used the
+ * old protocol for errors that happened before processing the
+ * startup packet.)
*/
if ((beresp == 'R' || beresp == 'v') && (msgLength < 8 || msgLength > 2000))
{
libpq_append_conn_error(conn, "expected authentication request from server, but received %c",
- beresp);
+ beresp);
goto error_return;
}
@@ -3705,7 +3705,7 @@ keep_going: /* We will come back to here until there is
/* Append error report to conn->errorMessage. */
libpq_append_conn_error(conn, "\"%s\" failed",
- "SHOW transaction_read_only");
+ "SHOW transaction_read_only");
/* Close connection politely. */
conn->status = CONNECTION_OK;
@@ -3755,7 +3755,7 @@ keep_going: /* We will come back to here until there is
/* Append error report to conn->errorMessage. */
libpq_append_conn_error(conn, "\"%s\" failed",
- "SELECT pg_is_in_recovery()");
+ "SELECT pg_is_in_recovery()");
/* Close connection politely. */
conn->status = CONNECTION_OK;
@@ -3768,8 +3768,8 @@ keep_going: /* We will come back to here until there is
default:
libpq_append_conn_error(conn,
- "invalid connection state %d, probably indicative of memory corruption",
- conn->status);
+ "invalid connection state %d, probably indicative of memory corruption",
+ conn->status);
goto error_return;
}
@@ -7148,7 +7148,7 @@ pgpassfileWarning(PGconn *conn)
if (sqlstate && strcmp(sqlstate, ERRCODE_INVALID_PASSWORD) == 0)
libpq_append_conn_error(conn, "password retrieved from file \"%s\"",
- conn->pgpassfile);
+ conn->pgpassfile);
}
}
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index ec62550e385..0c2dae6ed9e 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1444,7 +1444,7 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
libpq_append_conn_error(conn, "%s not allowed in pipeline mode",
- "PQsendQuery");
+ "PQsendQuery");
return 0;
}
@@ -1512,7 +1512,7 @@ PQsendQueryParams(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -1558,7 +1558,7 @@ PQsendPrepare(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -1652,7 +1652,7 @@ PQsendQueryPrepared(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -2099,10 +2099,9 @@ PQgetResult(PGconn *conn)
/*
* We're about to return the NULL that terminates the round of
- * results from the current query; prepare to send the results
- * of the next query, if any, when we're called next. If there's
- * no next element in the command queue, this gets us in IDLE
- * state.
+ * results from the current query; prepare to send the results of
+ * the next query, if any, when we're called next. If there's no
+ * next element in the command queue, this gets us in IDLE state.
*/
pqPipelineProcessQueue(conn);
res = NULL; /* query is complete */
@@ -3047,6 +3046,7 @@ pqPipelineProcessQueue(PGconn *conn)
return;
case PGASYNC_IDLE:
+
/*
* If we're in IDLE mode and there's some command in the queue,
* get us into PIPELINE_IDLE mode and process normally. Otherwise
diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c
index 4cb6a468597..206266fd043 100644
--- a/src/interfaces/libpq/fe-lobj.c
+++ b/src/interfaces/libpq/fe-lobj.c
@@ -142,7 +142,7 @@ lo_truncate(PGconn *conn, int fd, size_t len)
if (conn->lobjfuncs->fn_lo_truncate == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_truncate");
+ "lo_truncate");
return -1;
}
@@ -205,7 +205,7 @@ lo_truncate64(PGconn *conn, int fd, pg_int64 len)
if (conn->lobjfuncs->fn_lo_truncate64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_truncate64");
+ "lo_truncate64");
return -1;
}
@@ -395,7 +395,7 @@ lo_lseek64(PGconn *conn, int fd, pg_int64 offset, int whence)
if (conn->lobjfuncs->fn_lo_lseek64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_lseek64");
+ "lo_lseek64");
return -1;
}
@@ -485,7 +485,7 @@ lo_create(PGconn *conn, Oid lobjId)
if (conn->lobjfuncs->fn_lo_create == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_create");
+ "lo_create");
return InvalidOid;
}
@@ -558,7 +558,7 @@ lo_tell64(PGconn *conn, int fd)
if (conn->lobjfuncs->fn_lo_tell64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_tell64");
+ "lo_tell64");
return -1;
}
@@ -667,7 +667,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
if (fd < 0)
{ /* error */
libpq_append_conn_error(conn, "could not open file \"%s\": %s",
- filename, strerror_r(errno, sebuf, sizeof(sebuf)));
+ filename, strerror_r(errno, sebuf, sizeof(sebuf)));
return InvalidOid;
}
@@ -723,8 +723,8 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not read from file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return InvalidOid;
}
@@ -778,8 +778,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not open file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return -1;
}
@@ -799,8 +799,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not write to file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return -1;
}
}
@@ -822,7 +822,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
if (close(fd) != 0 && result >= 0)
{
libpq_append_conn_error(conn, "could not write to file \"%s\": %s",
- filename, strerror_r(errno, sebuf, sizeof(sebuf)));
+ filename, strerror_r(errno, sebuf, sizeof(sebuf)));
result = -1;
}
@@ -954,56 +954,56 @@ lo_initialize(PGconn *conn)
if (lobjfuncs->fn_lo_open == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_open");
+ "lo_open");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_close == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_close");
+ "lo_close");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_creat == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_creat");
+ "lo_creat");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_unlink == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_unlink");
+ "lo_unlink");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_lseek == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_lseek");
+ "lo_lseek");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_tell == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_tell");
+ "lo_tell");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_read == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "loread");
+ "loread");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_write == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lowrite");
+ "lowrite");
free(lobjfuncs);
return -1;
}
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3653a1a8a62..660cdec93c9 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -749,8 +749,8 @@ retry4:
*/
definitelyEOF:
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
@@ -1067,7 +1067,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "%s() failed: %s", "select",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
}
return result;
@@ -1280,7 +1280,7 @@ libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n)
* newline.
*/
void
-libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...)
+libpq_append_error(PQExpBuffer errorMessage, const char *fmt,...)
{
int save_errno = errno;
bool done;
@@ -1309,7 +1309,7 @@ libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...)
* format should not end with a newline.
*/
void
-libpq_append_conn_error(PGconn *conn, const char *fmt, ...)
+libpq_append_conn_error(PGconn *conn, const char *fmt,...)
{
int save_errno = errno;
bool done;
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8ab6a884165..b79d74f7489 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -466,7 +466,7 @@ static void
handleSyncLoss(PGconn *conn, char id, int msgLength)
{
libpq_append_conn_error(conn, "lost synchronization with server: got message type \"%c\", length %d",
- id, msgLength);
+ id, msgLength);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
conn->asyncStatus = PGASYNC_READY; /* drop out of PQgetResult wait loop */
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index de115b37649..3ecc7bf6159 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -226,7 +226,7 @@ pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
* wrong given the subject matter.
*/
libpq_append_conn_error(conn, "certificate contains IP address with invalid length %zu",
- iplen);
+ iplen);
return -1;
}
@@ -235,7 +235,7 @@ pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
if (!addrstr)
{
libpq_append_conn_error(conn, "could not convert certificate's IP address to string: %s",
- strerror_r(errno, sebuf, sizeof(sebuf)));
+ strerror_r(errno, sebuf, sizeof(sebuf)));
return -1;
}
@@ -292,7 +292,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
else if (names_examined == 1)
{
libpq_append_conn_error(conn, "server certificate for \"%s\" does not match host name \"%s\"",
- first_name, host);
+ first_name, host);
}
else
{
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index 6220e4a1014..bed6e62435b 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -213,8 +213,8 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "client tried to send oversize GSSAPI packet (%zu > %zu)",
- (size_t) output.length,
- PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
+ (size_t) output.length,
+ PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
errno = EIO; /* for lack of a better idea */
goto cleanup;
}
@@ -349,8 +349,8 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len)
if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)",
- (size_t) input.length,
- PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+ (size_t) input.length,
+ PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
errno = EIO; /* for lack of a better idea */
return -1;
}
@@ -588,8 +588,8 @@ pqsecure_open_gss(PGconn *conn)
if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)",
- (size_t) input.length,
- PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+ (size_t) input.length,
+ PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
return PGRES_POLLING_FAILED;
}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 983536de251..ab2cbf045b8 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -213,12 +213,12 @@ rloop:
if (result_errno == EPIPE ||
result_errno == ECONNRESET)
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
else
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
}
else
{
@@ -313,12 +313,12 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE || result_errno == ECONNRESET)
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
else
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
}
else
{
@@ -410,7 +410,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (algo_type == NULL)
{
libpq_append_conn_error(conn, "could not find digest for NID %s",
- OBJ_nid2sn(algo_nid));
+ OBJ_nid2sn(algo_nid));
return NULL;
}
break;
@@ -962,7 +962,7 @@ initialize_SSL(PGconn *conn)
if (ssl_min_ver == -1)
{
libpq_append_conn_error(conn, "invalid value \"%s\" for minimum SSL protocol version",
- conn->ssl_min_protocol_version);
+ conn->ssl_min_protocol_version);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -988,7 +988,7 @@ initialize_SSL(PGconn *conn)
if (ssl_max_ver == -1)
{
libpq_append_conn_error(conn, "invalid value \"%s\" for maximum SSL protocol version",
- conn->ssl_max_protocol_version);
+ conn->ssl_max_protocol_version);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1032,7 +1032,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read root certificate file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
SSL_CTX_free(SSL_context);
return -1;
@@ -1084,10 +1084,10 @@ initialize_SSL(PGconn *conn)
*/
if (fnbuf[0] == '\0')
libpq_append_conn_error(conn, "could not get home directory to locate root certificate file\n"
- "Either provide the file or change sslmode to disable server certificate verification.");
+ "Either provide the file or change sslmode to disable server certificate verification.");
else
libpq_append_conn_error(conn, "root certificate file \"%s\" does not exist\n"
- "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
+ "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1117,7 +1117,7 @@ initialize_SSL(PGconn *conn)
if (errno != ENOENT && errno != ENOTDIR)
{
libpq_append_conn_error(conn, "could not open certificate file \"%s\": %s",
- fnbuf, strerror_r(errno, sebuf, sizeof(sebuf)));
+ fnbuf, strerror_r(errno, sebuf, sizeof(sebuf)));
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1135,7 +1135,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read certificate file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
SSL_CTX_free(SSL_context);
return -1;
@@ -1234,7 +1234,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not load SSL engine \"%s\": %s",
- engine_str, err);
+ engine_str, err);
SSLerrfree(err);
free(engine_str);
return -1;
@@ -1245,7 +1245,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not initialize SSL engine \"%s\": %s",
- engine_str, err);
+ engine_str, err);
SSLerrfree(err);
ENGINE_free(conn->engine);
conn->engine = NULL;
@@ -1260,7 +1260,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read private SSL key \"%s\" from engine \"%s\": %s",
- engine_colon, engine_str, err);
+ engine_colon, engine_str, err);
SSLerrfree(err);
ENGINE_finish(conn->engine);
ENGINE_free(conn->engine);
@@ -1273,7 +1273,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not load private SSL key \"%s\" from engine \"%s\": %s",
- engine_colon, engine_str, err);
+ engine_colon, engine_str, err);
SSLerrfree(err);
ENGINE_finish(conn->engine);
ENGINE_free(conn->engine);
@@ -1310,10 +1310,10 @@ initialize_SSL(PGconn *conn)
{
if (errno == ENOENT)
libpq_append_conn_error(conn, "certificate present, but not private key file \"%s\"",
- fnbuf);
+ fnbuf);
else
libpq_append_conn_error(conn, "could not stat private key file \"%s\": %m",
- fnbuf);
+ fnbuf);
return -1;
}
@@ -1321,7 +1321,7 @@ initialize_SSL(PGconn *conn)
if (!S_ISREG(buf.st_mode))
{
libpq_append_conn_error(conn, "private key file \"%s\" is not a regular file",
- fnbuf);
+ fnbuf);
return -1;
}
@@ -1378,7 +1378,7 @@ initialize_SSL(PGconn *conn)
if (SSL_use_PrivateKey_file(conn->ssl, fnbuf, SSL_FILETYPE_ASN1) != 1)
{
libpq_append_conn_error(conn, "could not load private key file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
return -1;
}
@@ -1394,7 +1394,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "certificate does not match private key file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
return -1;
}
@@ -1447,7 +1447,7 @@ open_client_SSL(PGconn *conn)
if (r == -1)
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
else
libpq_append_conn_error(conn, "SSL SYSCALL error: EOF detected");
pgtls_close(conn);
@@ -1489,12 +1489,12 @@ open_client_SSL(PGconn *conn)
case SSL_R_VERSION_TOO_LOW:
#endif
libpq_append_conn_error(conn, "This may indicate that the server does not support any SSL protocol version between %s and %s.",
- conn->ssl_min_protocol_version ?
- conn->ssl_min_protocol_version :
- MIN_OPENSSL_TLS_VERSION,
- conn->ssl_max_protocol_version ?
- conn->ssl_max_protocol_version :
- MAX_OPENSSL_TLS_VERSION);
+ conn->ssl_min_protocol_version ?
+ conn->ssl_min_protocol_version :
+ MIN_OPENSSL_TLS_VERSION,
+ conn->ssl_max_protocol_version ?
+ conn->ssl_max_protocol_version :
+ MAX_OPENSSL_TLS_VERSION);
break;
default:
break;
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 66e401bf3d9..8069e381424 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -255,14 +255,14 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
case EPIPE:
case ECONNRESET:
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
break;
default:
libpq_append_conn_error(conn, "could not receive data from server: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
break;
}
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d94b648ea5b..712d572373c 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -888,8 +888,8 @@ extern char *libpq_ngettext(const char *msgid, const char *msgid_plural, unsigne
*/
#undef _
-extern void libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...) pg_attribute_printf(2, 3);
-extern void libpq_append_conn_error(PGconn *conn, const char *fmt, ...) pg_attribute_printf(2, 3);
+extern void libpq_append_error(PQExpBuffer errorMessage, const char *fmt,...) pg_attribute_printf(2, 3);
+extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3);
/*
* These macros are needed to let error-handling code be portable between
--
2.34.1
From 71bce9441ed2ba4311cbbcb0fa51f5d89766b701 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <jelte.fennema@microsoft.com>
Date: Fri, 13 Jan 2023 17:11:00 +0100
Subject: [PATCH v7 4/4] Share prng state between all PGconns in process
Instead of having a PRNG state per connection this adds a process wide
prng state that is protected by a mutex.
---
src/interfaces/libpq/fe-connect.c | 37 ++++++++++++++++++++++++++-----
src/interfaces/libpq/libpq-int.h | 1 -
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 69ed891703a..ec974ac6b84 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -442,6 +442,10 @@ static bool parse_int_param(const char *value, int *result, PGconn *conn,
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
+static pglock_t prng_lock = PGLOCK_INITIALIZER;
+static bool prng_seed_set = false;
+static pg_prng_state prng_state;
+
/*
* pqDropConnection
@@ -1028,16 +1032,25 @@ parse_comma_separated_list(char **startptr, bool *more)
static bool
libpq_prng_init(PGconn *conn)
{
+ if (!pglock(&prng_lock))
+ return false;
+
+ if (likely(prng_seed_set))
+ return pgunlock(&prng_lock);
+
if (unlikely(conn->randomseed))
{
int rseed;
if (!parse_int_param(conn->randomseed, &rseed, conn, "random_seed"))
+ {
+ pgunlock(&prng_lock);
return false;
+ }
- pg_prng_seed(&conn->prng_state, rseed);
+ pg_prng_seed(&prng_state, rseed);
}
- else if (unlikely(!pg_prng_strong_seed(&conn->prng_state)))
+ else if (unlikely(!pg_prng_strong_seed(&prng_state)))
{
uint64 rseed;
time_t now = time(NULL);
@@ -1053,8 +1066,12 @@ libpq_prng_init(PGconn *conn)
((uint64) now << 12) ^
((uint64) now >> 20);
- pg_prng_seed(&conn->prng_state, rseed);
+ pg_prng_seed(&prng_state, rseed);
}
+
+ prng_seed_set = true;
+ if (!pgunlock(&prng_lock))
+ return false;
return true;
}
@@ -1477,6 +1494,9 @@ connectOptions2(PGconn *conn)
if (!libpq_prng_init(conn))
return false;
+ if (!pglock(&prng_lock))
+ return false;
+
/*
* Shuffle connhost with a Durstenfeld/Knuth version of the
* Fisher-Yates shuffle. Source:
@@ -1484,12 +1504,14 @@ connectOptions2(PGconn *conn)
*/
for (i = conn->nconnhost - 1; i > 0; i--)
{
- int j = pg_prng_uint64_range(&conn->prng_state, 0, i);
+ int j = pg_prng_uint64_range(&prng_state, 0, i);
pg_conn_host temp = conn->connhost[j];
conn->connhost[j] = conn->connhost[i];
conn->connhost[i] = temp;
}
+ if (!pgunlock(&prng_lock))
+ return false;
}
/*
@@ -4175,6 +4197,9 @@ store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist)
if (conn->load_balance_type == LOAD_BALANCE_RANDOM)
{
+ if (!pglock(&prng_lock))
+ return false;
+
/*
* Shuffle addr with a Durstenfeld/Knuth version of the Fisher-Yates
* shuffle. Source:
@@ -4185,12 +4210,14 @@ store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist)
*/
for (int i = conn->naddr - 1; i > 0; i--)
{
- int j = pg_prng_uint64_range(&conn->prng_state, 0, i);
+ int j = pg_prng_uint64_range(&prng_state, 0, i);
AddrInfo temp = conn->addr[j];
conn->addr[j] = conn->addr[i];
conn->addr[i] = temp;
}
+ if (!pgunlock(&prng_lock))
+ return false;
}
return true;
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 01dd4190f33..0ecac062090 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -492,7 +492,6 @@ struct pg_conn
PGVerbosity verbosity; /* error/notice message verbosity */
PGContextVisibility show_context; /* whether to show CONTEXT field */
PGlobjfuncs *lobjfuncs; /* private state for large-object access fns */
- pg_prng_state prng_state; /* prng state for load balancing connections */
/* Buffer for data received from backend and not yet processed */
--
2.34.1
From c92a730bff697d7e2ce694013ca677bcd947e3c6 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <github-tech@jeltef.nl>
Date: Mon, 12 Sep 2022 09:44:06 +0200
Subject: [PATCH v7 2/4] Support load balancing in libpq
This adds support for load balancing to libpq using the newly added
load_balance_hosts parameter. When setting the load_balance_hosts
parameter to random, hosts and addresses will be connected to in a
random order. This then results in load balancing across these
hosts/addresses if multiple clients do this at the same time.
This patch implements two levels of random load balancing:
1. The given hosts are randomly shuffled, before resolving them
one-by-one.
2. Once a host its addresses get resolved, those addresses are shuffled,
before trying to connect to them one-by-one.
---
.cirrus.yml | 14 ++
doc/src/sgml/libpq.sgml | 69 +++++++
src/include/libpq/pqcomm.h | 6 +
src/interfaces/libpq/fe-connect.c | 215 ++++++++++++++++++----
src/interfaces/libpq/libpq-int.h | 21 ++-
src/interfaces/libpq/meson.build | 1 +
src/interfaces/libpq/t/003_loadbalance.pl | 167 +++++++++++++++++
7 files changed, 460 insertions(+), 33 deletions(-)
create mode 100644 src/interfaces/libpq/t/003_loadbalance.pl
diff --git a/.cirrus.yml b/.cirrus.yml
index 69837bcd5ad..f71e93451fa 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -308,6 +308,14 @@ task:
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
+
+ setup_hosts_file_script: |
+ cat >> /etc/hosts <<-EOF
+ 127.0.0.1 pg-loadbalancetest
+ 127.0.0.2 pg-loadbalancetest
+ 127.0.0.3 pg-loadbalancetest
+ EOF
+
setup_additional_packages_script: |
#apt-get update
#DEBIAN_FRONTEND=noninteractive apt-get -y install ...
@@ -557,6 +565,12 @@ task:
setup_additional_packages_script: |
REM choco install -y --no-progress ...
+ setup_hosts_file_script: |
+ echo 127.0.0.1 pg-loadbalancetest >> c:\Windows\System32\Drivers\etc\hosts
+ echo 127.0.0.2 pg-loadbalancetest >> c:\Windows\System32\Drivers\etc\hosts
+ echo 127.0.0.3 pg-loadbalancetest >> c:\Windows\System32\Drivers\etc\hosts
+ type c:\Windows\System32\Drivers\etc\hosts
+
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c706..c540554ade7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1964,6 +1964,75 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="libpq-connect-load-balance-hosts" xreflabel="load_balance_hosts">
+ <term><literal>load_balance_hosts</literal></term>
+ <listitem>
+ <para>
+ Controls the order in which the client tries to connect to the available
+ hosts and addresses. It's typically used in combination with multiple
+ host names or a DNS record that returns multiple IPs. This parameter be
+ used in combination with <xref linkend="libpq-connect-target-session-attrs"/>
+ to, for example, load balance over stanby servers only. Once successfully
+ connected, subsequent queries on the returned connection will all be
+ sent to the same server. There are currently two modes:
+ <variablelist>
+ <varlistentry>
+ <term><literal>disable</literal> (default)</term>
+ <listitem>
+ <para>
+ Hosts are tried in the order in which they are provided and
+ addresses are tried in the order they are received from DNS or a
+ hosts file.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>random</literal></term>
+ <listitem>
+ <para>
+ The provided hosts and the addresses that they resolve to are
+ tried in random order. This value is mostly useful when opening
+ multiple connections at the same time, possibly from different
+ machines. This way connections can be load balanced across multiple
+ Postgres servers.
+ </para>
+ <para>
+ This algorithm uses two levels of random choices: First the hosts
+ will be resolved in random order. Then before resolving the next
+ host, all resolved addresses for the current host will be tried in
+ random order. This behaviour can lead to non-uniform address
+ selection in certain cases, for instance when some hosts resolve to
+ more addresses than others. So if you want uniform load balancing,
+ this is something to keep in mind. However, non-uniform load
+ balancing also can be used to your advantage, e.g. by providing the
+ hostname of a larger server multiple times in the host string so it
+ gets more connections.
+ </para>
+ <para>
+ When using this value it's recommended to also configure a reasonable
+ value for <xref linkend="libpq-connect-connect-timeout"/>. Because then,
+ if one of the nodes that are used for load balancing is not responding,
+ a new node will be tried.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-random-seed" xreflabel="random_seed">
+ <term><literal>random_seed</literal></term>
+ <listitem>
+ <para>
+ Sets the random seed that is used by <xref linkend="libpq-connect-load-balance-hosts"/>
+ to randomize the host order. This option is mostly useful when running
+ tests that require a stable random order.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</sect2>
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 66ba359390f..ee28e223bd7 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -27,6 +27,12 @@ typedef struct
socklen_t salen;
} SockAddr;
+typedef struct
+{
+ int family;
+ SockAddr addr;
+} AddrInfo;
+
/* Configure the UNIX socket location for the well known port. */
#define UNIXSOCK_PATH(path, port, sockdir) \
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 773e9e1f3a2..18a07d810dc 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -123,6 +123,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
#define DefaultChannelBinding "disable"
#endif
#define DefaultTargetSessionAttrs "any"
+#define DefaultLoadBalanceHosts "disable"
#ifdef USE_SSL
#define DefaultSSLMode "prefer"
#else
@@ -341,6 +342,15 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"Target-Session-Attrs", "", 15, /* sizeof("prefer-standby") = 15 */
offsetof(struct pg_conn, target_session_attrs)},
+ {"load_balance_hosts", "PGLOADBALANCEHOSTS",
+ DefaultLoadBalanceHosts, NULL,
+ "Load-Balance-Hosts", "", 8, /* sizeof("disable") = 8 */
+ offsetof(struct pg_conn, load_balance_hosts)},
+
+ {"random_seed", NULL, NULL, NULL,
+ "Random-Seed", "", 10, /* strlen(INT32_MAX) == 10 */
+ offsetof(struct pg_conn, randomseed)},
+
/* Terminating entry --- MUST BE LAST */
{NULL, NULL, NULL, NULL,
NULL, NULL, 0}
@@ -379,6 +389,7 @@ static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
static void freePGconn(PGconn *conn);
static void closePGconn(PGconn *conn);
static void release_conn_addrinfo(PGconn *conn);
+static bool store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist);
static void sendTerminateConn(PGconn *conn);
static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
static PQconninfoOption *parse_connection_string(const char *connstr,
@@ -424,6 +435,8 @@ static void pgpassfileWarning(PGconn *conn);
static void default_threadlock(int acquire);
static bool sslVerifyProtocolVersion(const char *version);
static bool sslVerifyProtocolRange(const char *min, const char *max);
+static bool parse_int_param(const char *value, int *result, PGconn *conn,
+ const char *context);
/* global variable because fe-auth.c needs to access it */
@@ -1007,6 +1020,44 @@ parse_comma_separated_list(char **startptr, bool *more)
return p;
}
+/*
+ * Initializes the prng_state field of the connection. We want something
+ * unpredictable, so if possible, use high-quality random bits for the
+ * seed. Otherwise, fall back to a seed based on timestamp and PID.
+ */
+static bool
+libpq_prng_init(PGconn *conn)
+{
+ if (unlikely(conn->randomseed))
+ {
+ int rseed;
+
+ if (!parse_int_param(conn->randomseed, &rseed, conn, "random_seed"))
+ return false;
+
+ pg_prng_seed(&conn->prng_state, rseed);
+ }
+ else if (unlikely(!pg_prng_strong_seed(&conn->prng_state)))
+ {
+ uint64 rseed;
+ time_t now = time(NULL);
+
+ /*
+ * Since PIDs and timestamps tend to change more frequently in their
+ * least significant bits, shift the timestamp left to allow a larger
+ * total number of seeds in a given time period. Since that would
+ * leave only 20 bits of the timestamp that cycle every ~1 second,
+ * also mix in some higher bits.
+ */
+ rseed = ((uint64) getpid()) ^
+ ((uint64) now << 12) ^
+ ((uint64) now >> 20);
+
+ pg_prng_seed(&conn->prng_state, rseed);
+ }
+ return true;
+}
+
/*
* connectOptions2
*
@@ -1400,6 +1451,47 @@ connectOptions2(PGconn *conn)
else
conn->target_server_type = SERVER_TYPE_ANY;
+ /*
+ * validate load_balance_hosts option, and set load_balance_type
+ */
+ if (conn->load_balance_hosts)
+ {
+ if (strcmp(conn->load_balance_hosts, "disable") == 0)
+ conn->load_balance_type = LOAD_BALANCE_DISABLE;
+ else if (strcmp(conn->load_balance_hosts, "random") == 0)
+ conn->load_balance_type = LOAD_BALANCE_RANDOM;
+ else
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+ "load_balance_hosts",
+ conn->load_balance_hosts);
+ return false;
+ }
+ }
+ else
+ conn->load_balance_type = LOAD_BALANCE_DISABLE;
+
+ if (conn->load_balance_type == LOAD_BALANCE_RANDOM)
+ {
+ if (!libpq_prng_init(conn))
+ return false;
+
+ /*
+ * Shuffle connhost with a Durstenfeld/Knuth version of the
+ * Fisher-Yates shuffle. Source:
+ * https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm
+ */
+ for (i = conn->nconnhost - 1; i > 0; i--)
+ {
+ int j = pg_prng_uint64_range(&conn->prng_state, 0, i);
+ pg_conn_host temp = conn->connhost[j];
+
+ conn->connhost[j] = conn->connhost[i];
+ conn->connhost[i] = temp;
+ }
+ }
+
/*
* Resolve special "auto" client_encoding from the locale
*/
@@ -2077,7 +2169,7 @@ connectDBComplete(PGconn *conn)
time_t finish_time = ((time_t) -1);
int timeout = 0;
int last_whichhost = -2; /* certainly different from whichhost */
- struct addrinfo *last_addr_cur = NULL;
+ int last_whichaddr = -2; /* certainly different from whichaddr */
if (conn == NULL || conn->status == CONNECTION_BAD)
return 0;
@@ -2121,11 +2213,11 @@ connectDBComplete(PGconn *conn)
if (flag != PGRES_POLLING_OK &&
timeout > 0 &&
(conn->whichhost != last_whichhost ||
- conn->addr_cur != last_addr_cur))
+ conn->whichaddr != last_whichaddr))
{
finish_time = time(NULL) + timeout;
last_whichhost = conn->whichhost;
- last_addr_cur = conn->addr_cur;
+ last_whichaddr = conn->whichaddr;
}
/*
@@ -2272,9 +2364,9 @@ keep_going: /* We will come back to here until there is
/* Time to advance to next address, or next host if no more addresses? */
if (conn->try_next_addr)
{
- if (conn->addr_cur && conn->addr_cur->ai_next)
+ if (conn->whichaddr < conn->naddr)
{
- conn->addr_cur = conn->addr_cur->ai_next;
+ conn->whichaddr++;
reset_connection_state_machine = true;
}
else
@@ -2287,6 +2379,7 @@ keep_going: /* We will come back to here until there is
{
pg_conn_host *ch;
struct addrinfo hint;
+ struct addrinfo *addrlist;
int thisport;
int ret;
char portstr[MAXPGPATH];
@@ -2327,7 +2420,7 @@ keep_going: /* We will come back to here until there is
/* Initialize hint structure */
MemSet(&hint, 0, sizeof(hint));
hint.ai_socktype = SOCK_STREAM;
- conn->addrlist_family = hint.ai_family = AF_UNSPEC;
+ hint.ai_family = AF_UNSPEC;
/* Figure out the port number we're going to use. */
if (ch->port == NULL || ch->port[0] == '\0')
@@ -2350,8 +2443,8 @@ keep_going: /* We will come back to here until there is
{
case CHT_HOST_NAME:
ret = pg_getaddrinfo_all(ch->host, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not translate host name \"%s\" to address: %s",
ch->host, gai_strerror(ret));
@@ -2362,8 +2455,8 @@ keep_going: /* We will come back to here until there is
case CHT_HOST_ADDRESS:
hint.ai_flags = AI_NUMERICHOST;
ret = pg_getaddrinfo_all(ch->hostaddr, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not parse network address \"%s\": %s",
ch->hostaddr, gai_strerror(ret));
@@ -2372,7 +2465,7 @@ keep_going: /* We will come back to here until there is
break;
case CHT_UNIX_SOCKET:
- conn->addrlist_family = hint.ai_family = AF_UNIX;
+ hint.ai_family = AF_UNIX;
UNIXSOCK_PATH(portstr, thisport, ch->host);
if (strlen(portstr) >= UNIXSOCK_PATH_BUFLEN)
{
@@ -2387,8 +2480,8 @@ keep_going: /* We will come back to here until there is
* name as a Unix-domain socket path.
*/
ret = pg_getaddrinfo_all(NULL, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not translate Unix-domain socket path \"%s\" to address: %s",
portstr, gai_strerror(ret));
@@ -2397,8 +2490,14 @@ keep_going: /* We will come back to here until there is
break;
}
- /* OK, scan this addrlist for a working server address */
- conn->addr_cur = conn->addrlist;
+ if (!store_conn_addrinfo(conn, addrlist))
+ {
+ pg_freeaddrinfo_all(hint.ai_family, addrlist);
+ libpq_append_conn_error(conn, "out of memory");
+ goto error_return;
+ }
+ pg_freeaddrinfo_all(hint.ai_family, addrlist);
+
reset_connection_state_machine = true;
conn->try_next_host = false;
}
@@ -2455,30 +2554,29 @@ keep_going: /* We will come back to here until there is
{
/*
* Try to initiate a connection to one of the addresses
- * returned by pg_getaddrinfo_all(). conn->addr_cur is the
+ * returned by pg_getaddrinfo_all(). conn->whichaddr is the
* next one to try.
*
* The extra level of braces here is historical. It's not
* worth reindenting this whole switch case to remove 'em.
*/
{
- struct addrinfo *addr_cur = conn->addr_cur;
char host_addr[NI_MAXHOST];
+ AddrInfo *addr_cur;
/*
* Advance to next possible host, if we've tried all of
* the addresses for the current host.
*/
- if (addr_cur == NULL)
+ if (conn->whichaddr == conn->naddr)
{
conn->try_next_host = true;
goto keep_going;
}
+ addr_cur = &conn->addr[conn->whichaddr];
/* Remember current address for possible use later */
- memcpy(&conn->raddr.addr, addr_cur->ai_addr,
- addr_cur->ai_addrlen);
- conn->raddr.salen = addr_cur->ai_addrlen;
+ memcpy(&conn->raddr, &addr_cur->addr, sizeof(SockAddr));
/*
* Set connip, too. Note we purposely ignore strdup
@@ -2494,7 +2592,7 @@ keep_going: /* We will come back to here until there is
conn->connip = strdup(host_addr);
/* Try to create the socket */
- conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
+ conn->sock = socket(addr_cur->family, SOCK_STREAM, 0);
if (conn->sock == PGINVALID_SOCKET)
{
int errorno = SOCK_ERRNO;
@@ -2505,7 +2603,7 @@ keep_going: /* We will come back to here until there is
* cases where the address list includes both IPv4 and
* IPv6 but kernel only accepts one family.
*/
- if (addr_cur->ai_next != NULL ||
+ if (conn->whichaddr < conn->naddr ||
conn->whichhost + 1 < conn->nconnhost)
{
conn->try_next_addr = true;
@@ -2531,7 +2629,7 @@ keep_going: /* We will come back to here until there is
* TCP sockets, nonblock mode, close-on-exec. Try the
* next address if any of this fails.
*/
- if (addr_cur->ai_family != AF_UNIX)
+ if (addr_cur->family != AF_UNIX)
{
if (!connectNoDelay(conn))
{
@@ -2558,7 +2656,7 @@ keep_going: /* We will come back to here until there is
}
#endif /* F_SETFD */
- if (addr_cur->ai_family != AF_UNIX)
+ if (addr_cur->family != AF_UNIX)
{
#ifndef WIN32
int on = 1;
@@ -2650,8 +2748,8 @@ keep_going: /* We will come back to here until there is
* Start/make connection. This should not block, since we
* are in nonblock mode. If it does, well, too bad.
*/
- if (connect(conn->sock, addr_cur->ai_addr,
- addr_cur->ai_addrlen) < 0)
+ if (connect(conn->sock, (struct sockaddr *) &addr_cur->addr.addr,
+ addr_cur->addr.salen) < 0)
{
if (SOCK_ERRNO == EINPROGRESS ||
#ifdef WIN32
@@ -4035,12 +4133,68 @@ freePGconn(PGconn *conn)
free(conn->outBuffer);
free(conn->rowBuf);
free(conn->target_session_attrs);
+ free(conn->load_balance_hosts);
+ free(conn->randomseed);
termPQExpBuffer(&conn->errorMessage);
termPQExpBuffer(&conn->workBuffer);
free(conn);
}
+/*
+ * Copies over the AddrInfos from addrlist to the PGconn.
+ */
+static bool
+store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist)
+{
+ struct addrinfo *ai = addrlist;
+
+ conn->whichaddr = 0;
+
+ conn->naddr = 0;
+ while (ai)
+ {
+ ai = ai->ai_next;
+ conn->naddr++;
+ }
+
+ conn->addr = calloc(conn->naddr, sizeof(AddrInfo));
+ if (conn->addr == NULL)
+ return false;
+
+ ai = addrlist;
+ for (int i = 0; i < conn->naddr; i++)
+ {
+ conn->addr[i].family = ai->ai_family;
+
+ memcpy(&conn->addr[i].addr.addr, ai->ai_addr,
+ ai->ai_addrlen);
+ conn->addr[i].addr.salen = ai->ai_addrlen;
+ ai = ai->ai_next;
+ }
+
+ if (conn->load_balance_type == LOAD_BALANCE_RANDOM)
+ {
+ /*
+ * Shuffle addr with a Durstenfeld/Knuth version of the Fisher-Yates
+ * shuffle. Source:
+ * https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm
+ *
+ * We don't need to initialize conn->prng_state here, because that
+ * already happened in connectOptions2.
+ */
+ for (int i = conn->naddr - 1; i > 0; i--)
+ {
+ int j = pg_prng_uint64_range(&conn->prng_state, 0, i);
+ AddrInfo temp = conn->addr[j];
+
+ conn->addr[j] = conn->addr[i];
+ conn->addr[i] = temp;
+ }
+ }
+ return true;
+}
+
/*
* release_conn_addrinfo
* - Free any addrinfo list in the PGconn.
@@ -4048,11 +4202,10 @@ freePGconn(PGconn *conn)
static void
release_conn_addrinfo(PGconn *conn)
{
- if (conn->addrlist)
+ if (conn->addr)
{
- pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
- conn->addrlist = NULL;
- conn->addr_cur = NULL; /* for safety */
+ free(conn->addr);
+ conn->addr = NULL;
}
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 712d572373c..86dd1d6d405 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -82,6 +82,8 @@ typedef struct
#endif
#endif /* USE_OPENSSL */
+#include "common/pg_prng.h"
+
/*
* POSTGRES backend dependent Constants.
*/
@@ -242,6 +244,13 @@ typedef enum
SERVER_TYPE_PREFER_STANDBY_PASS2 /* second pass - behaves same as ANY */
} PGTargetServerType;
+/* Target server type (decoded value of load_balance_hosts) */
+typedef enum
+{
+ LOAD_BALANCE_DISABLE = 0, /* Use the existing host order (default) */
+ LOAD_BALANCE_RANDOM, /* Read-write server */
+} PGLoadBalanceType;
+
/* Boolean value plus a not-known state, for GUCs we might have to fetch */
typedef enum
{
@@ -396,6 +405,8 @@ struct pg_conn
char *ssl_min_protocol_version; /* minimum TLS protocol version */
char *ssl_max_protocol_version; /* maximum TLS protocol version */
char *target_session_attrs; /* desired session properties */
+ char *load_balance_hosts; /* load balance over hosts */
+ char *randomseed; /* seed for randomization of load balancing */
/* Optional file to write trace info to */
FILE *Pfdebug;
@@ -459,10 +470,14 @@ struct pg_conn
/* Transient state needed while establishing connection */
PGTargetServerType target_server_type; /* desired session properties */
+ PGLoadBalanceType load_balance_type; /* desired load balancing
+ * algorithm */
bool try_next_addr; /* time to advance to next address/host? */
bool try_next_host; /* time to advance to next connhost[]? */
- struct addrinfo *addrlist; /* list of addresses for current connhost */
- struct addrinfo *addr_cur; /* the one currently being tried */
+ int naddr; /* number of addresses returned by getaddrinfo */
+ int whichaddr; /* the address currently being tried */
+ AddrInfo *addr; /* the array of addresses for the currently
+ * tried host */
int addrlist_family; /* needed to know how to free addrlist */
bool send_appname; /* okay to send application_name? */
@@ -477,6 +492,8 @@ struct pg_conn
PGVerbosity verbosity; /* error/notice message verbosity */
PGContextVisibility show_context; /* whether to show CONTEXT field */
PGlobjfuncs *lobjfuncs; /* private state for large-object access fns */
+ pg_prng_state prng_state; /* prng state for load balancing connections */
+
/* Buffer for data received from backend and not yet processed */
char *inBuffer; /* currently allocated buffer */
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 573fd9b6ea4..52b327500c7 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -116,6 +116,7 @@ tests += {
'tests': [
't/001_uri.pl',
't/002_api.pl',
+ 't/003_loadbalance.pl',
],
'env': {'with_ssl': get_option('ssl')},
},
diff --git a/src/interfaces/libpq/t/003_loadbalance.pl b/src/interfaces/libpq/t/003_loadbalance.pl
new file mode 100644
index 00000000000..1b4e8fd8133
--- /dev/null
+++ b/src/interfaces/libpq/t/003_loadbalance.pl
@@ -0,0 +1,167 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+use strict;
+use warnings;
+use Config;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use File::Spec::Functions 'catfile';
+use Test::More;
+
+# This tests two different methods of load balancing from libpq
+# 1. Load balancing by providing multiple host and port combinations in the
+# libpq connection string.
+# 2. By using a hosts file where hostname maps to multiple different IP
+# addresses. Regular Postgres users wouldn't usually use such a host file,
+# but this is the easiest way to immitate behaviour of a DNS server that
+# returns multiple IP addresses for the same DNS record.
+#
+# Testing method 1 is supported on all platforms and works out of the box. But
+# testing method 2 has some more requirements, both on the platform and on the
+# initial setup. If any of these requirements are not met, then method 2 is
+# simply not tested.
+#
+# The requirements to test method 2 are as follows:
+# 1. Windows or Linux should be used.
+# 2. The OS hosts file at /etc/hosts or c:\Windows\System32\Drivers\etc\hosts
+# should contain the following contents:
+#
+# 127.0.0.1 pg-loadbalancetest
+# 127.0.0.2 pg-loadbalancetest
+# 127.0.0.3 pg-loadbalancetest
+#
+#
+# Windows or Linux are required to test method 2 because these OSes allow
+# binding to 127.0.0.2 and 127.0.0.3 addresess by default, but other OSes
+# don't. We need to bind to different IP addresses, so that we can use these
+# different IP addresses in the hosts file.
+#
+# The hosts file needs to be prepared before running this test. We don't do it
+# on the fly, because it requires root permissions to change the hosts file. In
+# CI we set up the previously mentioned rules in the hosts file, so that this
+# load balancing method is tested.
+
+
+# Cluster setup which is shared for testing both load balancing methods
+my $can_bind_to_127_0_0_2 = $Config{osname} eq 'linux' || $PostgreSQL::Test::Utils::windows_os;
+
+if ($can_bind_to_127_0_0_2)
+{
+ $PostgreSQL::Test::Cluster::use_tcp = 1;
+ $PostgreSQL::Test::Cluster::test_pghost = '127.0.0.1';
+}
+my $port = PostgreSQL::Test::Cluster::get_free_port();
+my $node1 = PostgreSQL::Test::Cluster->new('node1', port => $port);
+my $node2 = PostgreSQL::Test::Cluster->new('node2', port => $port, own_host => 1);
+my $node3 = PostgreSQL::Test::Cluster->new('node3', port => $port, own_host => 1);
+
+# Create a data directory with initdb
+$node1->init();
+$node2->init();
+$node3->init();
+
+# Start the PostgreSQL server
+$node1->start();
+$node2->start();
+$node3->start();
+
+# Start the tests for load balancing method 1
+my $hostlist = $node1->host . ',' . $node2->host . ',' . $node3->host;
+my $portlist = "$port,$port,$port";
+
+$node1->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=123",
+ "seed 123 selects node 1 first",
+ sql => "SELECT 'connect1'",
+ log_like => [qr/statement: SELECT 'connect1'/]);
+
+$node2->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=123",
+ "seed 123 does not select node 2 first",
+ sql => "SELECT 'connect1'",
+ log_unlike => [qr/statement: SELECT 'connect1'/]);
+
+$node3->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=123",
+ "seed 123 does not select node 3 first",
+ sql => "SELECT 'connect1'",
+ log_unlike => [qr/statement: SELECT 'connect1'/]);
+
+$node3->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=42",
+ "seed 42 selects node 3 first",
+ sql => "SELECT 'connect2'",
+ log_like => [qr/statement: SELECT 'connect2'/]);
+
+$node1->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=42",
+ "seed 42 does not select node 1 first",
+ sql => "SELECT 'connect2'",
+ log_unlike => [qr/statement: SELECT 'connect2'/]);
+
+$node2->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=42",
+ "seed 42 does not select node 2 first",
+ sql => "SELECT 'connect2'",
+ log_unlike => [qr/statement: SELECT 'connect2'/]);
+
+$node3->stop();
+
+$node1->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=42",
+ "seed 42 does select node 1 second",
+ sql => "SELECT 'connect3'",
+ log_like => [qr/statement: SELECT 'connect3'/]);
+
+$node2->connect_ok("host=$hostlist port=$portlist load_balance_hosts=random random_seed=42",
+ "seed 42 does not select node 2 second",
+ sql => "SELECT 'connect3'",
+ log_unlike => [qr/statement: SELECT 'connect3'/]);
+
+$node3->start();
+
+# Checks for the requirements for testing load balancing method 2
+if (!$can_bind_to_127_0_0_2) {
+ # The OS requirement is not met
+ done_testing();
+ exit;
+}
+
+my $hosts_path;
+if ($windows_os) {
+ $hosts_path = 'c:\Windows\System32\Drivers\etc\hosts';
+}
+else
+{
+ $hosts_path = '/etc/hosts';
+}
+
+my $hosts_content = PostgreSQL::Test::Utils::slurp_file($hosts_path);
+
+if ($hosts_content !~ m/pg-loadbalancetest/) {
+ # Host file is not prepared for this test
+ done_testing();
+ exit;
+}
+
+# Start the tests for load balancing method 2
+$node2->connect_ok("host=pg-loadbalancetest port=$port load_balance_hosts=random random_seed=44",
+ "seed 44 selects node 2 first",
+ sql => "SELECT 'connect4'",
+ log_like => [qr/statement: SELECT 'connect4'/]);
+
+$node1->connect_ok("host=pg-loadbalancetest port=$port load_balance_hosts=random random_seed=44",
+ "seed 44 does not select node 1 first",
+ sql => "SELECT 'connect4'",
+ log_unlike => [qr/statement: SELECT 'connect4'/]);
+
+$node3->connect_ok("host=pg-loadbalancetest port=$port load_balance_hosts=random random_seed=44",
+ "seed 44 does not select node 3 first",
+ sql => "SELECT 'connect4'",
+ log_unlike => [qr/statement: SELECT 'connect4'/]);
+
+$node2->stop();
+
+$node1->connect_ok("host=pg-loadbalancetest port=$port load_balance_hosts=random random_seed=44",
+ "seed 44 does select node 1 second",
+ sql => "SELECT 'connect5'",
+ log_like => [qr/statement: SELECT 'connect5'/]);
+
+$node3->connect_ok("host=pg-loadbalancetest port=$port load_balance_hosts=random random_seed=44",
+ "seed 44 does not select node 3 second",
+ sql => "SELECT 'connect5'",
+ log_unlike => [qr/statement: SELECT 'connect5'/]);
+
+done_testing();
--
2.34.1
