Mutable listen_addresses GUC
Started by Ivan Kovmir4 months ago2 messages
Hello.
It is necessary to restart PostgreSQL to bind to a different network
interface, thus breaking the active connections. This patch draft
makes it no longer necessary, just assign the new value to
`listen_addresses` GUC and send SIGHUP to Postmaster.
Do you think it is useful? Is there any chance to push it upstream?
Overall thoughts?
Attachments:
0001-Change-bound-network-interfaces-without-restart.patchtext/x-patch; charset=US-ASCII; name=0001-Change-bound-network-interfaces-without-restart.patchDownload
From 97b35aa15902a9b055475fdfb016547096a43667 Mon Sep 17 00:00:00 2001
From: Ivan Kovmir <ivan.kovmir@cybertec.at>
Date: Mon, 29 Sep 2025 15:53:00 +0200
Subject: [PATCH 1/1] Change bound network interfaces without restart
Assign a different GUC `listen_addresses` value and send
SIGHUP to Postmaster to change bound network interface without
restarting the server, thus breaking currently held connections.
```
alter system set listen_addresses='SOME.OTHER.IP.ADDRESS';
select pg_reload_conf();
```
Multiple comma-separated addresses as well as `*` are possible.
---
src/backend/postmaster/postmaster.c | 188 ++++++++++++++++++++++
src/backend/utils/misc/guc_parameters.dat | 3 +-
src/include/utils/guc_hooks.h | 1 +
3 files changed, 191 insertions(+), 1 deletion(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e1d643b013d..fa0d45b844f 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -72,6 +72,7 @@
#include <ctype.h>
#include <sys/stat.h>
#include <sys/socket.h>
+#include <arpa/inet.h>
#include <fcntl.h>
#include <sys/param.h>
#include <netdb.h>
@@ -94,6 +95,7 @@
#include "access/xlogrecovery.h"
#include "common/file_perm.h"
#include "common/pg_prng.h"
+#include "common/ip.h"
#include "lib/ilist.h"
#include "libpq/libpq.h"
#include "libpq/pqsignal.h"
@@ -122,6 +124,7 @@
#include "utils/pidfile.h"
#include "utils/timestamp.h"
#include "utils/varlena.h"
+#include "utils/guc_hooks.h"
#ifdef EXEC_BACKEND
#include "common/file_utils.h"
@@ -418,6 +421,7 @@ static void CloseServerPorts(int status, Datum arg);
static void unlink_external_pid_file(int status, Datum arg);
static void getInstallationPaths(const char *argv0);
static void checkControlFile(void);
+static void ConfigurePostmasterWaitSet(bool accept_connections);
static void handle_pm_pmsignal_signal(SIGNAL_ARGS);
static void handle_pm_child_exit_signal(SIGNAL_ARGS);
static void handle_pm_reload_request_signal(SIGNAL_ARGS);
@@ -1445,6 +1449,190 @@ CloseServerPorts(int status, Datum arg)
*/
}
+/*
+ * Attempt bind to all the hostnames in *newval,
+ * then determine which ones should be closed and close them.
+ */
+void
+assign_listen_addresses(const char *newval, void *extra)
+{
+ int status;
+ int n_bound, n_closed;
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ char portNumberStr[32];
+ int retval;
+ int i, j;
+ struct addrinfo *addrs = NULL, *addr;
+ struct addrinfo hint;
+ struct sockaddr_storage sock_addr;
+ socklen_t sock_addr_len = sizeof(sock_addr);
+ /* Marks which interfaces we should remain bound to. */
+ bool remain_bound[MAXLISTEN];
+
+ char ip_str[INET6_ADDRSTRLEN];
+
+ /* Is there a better test? */
+ if (ListenSockets == NULL)
+ return; /* Not postmaster process. */
+
+ /* Need a modifiable copy of ListenAddresses. */
+ rawstring = pstrdup(newval);
+
+ /* Parse the string into a list of hostnames. */
+ if (!SplitGUCList(rawstring, ',', &elemlist))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid list syntax in parameter \"%s\"",
+ "listen_addresses")));
+ }
+ /* Bind to each hostname individually. */
+ n_bound = 0;
+ foreach(l, elemlist)
+ {
+ /* This foreach() is very similar to one within PostmasterMain().
+ * Should it be taken out into a common function call?
+ * I doubt it since the code snippets are not identical. */
+ char *curhost = (char *) lfirst(l);
+
+ if (strcmp(curhost, "*") == 0)
+ {
+ status = ListenServerPort(AF_UNSPEC, NULL,
+ (unsigned short) PostPortNumber,
+ NULL,
+ ListenSockets,
+ &NumListenSockets,
+ MAXLISTEN);
+ }
+ else
+ {
+ status = ListenServerPort(AF_UNSPEC, curhost,
+ (unsigned short) PostPortNumber,
+ NULL,
+ ListenSockets,
+ &NumListenSockets,
+ MAXLISTEN);
+ }
+
+ if (status == STATUS_OK)
+ {
+ n_bound++;
+ /* Should it be like that? The comment atop the function says
+ * it may be dangerous to overwrite the string with a shorter
+ * one. */
+ if (n_bound == 1)
+ AddToDataDirLockFile(LOCK_FILE_LINE_LISTEN_ADDR, curhost);
+ }
+ }
+
+ /* Done binding, now drop the no longer needed sockets... */
+
+ MemSet(&hint, 0, sizeof(hint));
+ hint.ai_family = AF_UNSPEC; /* Empty search condition for getaddrinfo().*/
+
+ for (i = 0; i < MAXLISTEN; i++)
+ remain_bound[i] = false; /* Reset. */
+
+ /* Should port number be mutable too? */
+ snprintf(portNumberStr, sizeof(portNumberStr), "%d", PostPortNumber);
+ /* The following foreach(){} determines which sockets are to be closed.
+ * We resolve hosts twice: once to bind, and once to close them. Ugly? */
+ foreach(l, elemlist) /* Iterate through hostnames. */
+ {
+ char *curhost = (char *) lfirst(l);
+
+ retval = pg_getaddrinfo_all(curhost, portNumberStr, &hint, &addrs);
+ if (retval != 0)
+ {
+ ereport(ERROR,
+ (errmsg("could not translate host name \"%s\", service \"%s\" to address: %s",
+ curhost, portNumberStr, gai_strerror(retval))));
+ }
+
+ /* Iterate through resolved addresses. */
+ for (addr = addrs; addr; addr = addr->ai_next)
+ {
+ /* Iterate through bound addresses. */
+ for (i = 0; i < NumListenSockets; i++)
+ {
+ /* Is it portable? */
+ getsockname(ListenSockets[i], (struct sockaddr *)&sock_addr,
+ &sock_addr_len);
+
+ /* Ignore UNIX sockets. */
+ if (sock_addr.ss_family == AF_UNIX)
+ {
+ remain_bound[i] = true;
+ continue;
+ }
+
+ /* If whatever we are bound to matches with the requested
+ * address/hostname, then stay bound to it. */
+ if ( ((struct sockaddr_in *)&sock_addr)->sin_addr.s_addr ==
+ ((struct sockaddr_in *)addr->ai_addr)->sin_addr.s_addr )
+ remain_bound[i] = true;
+
+ /* IPv6 not handled? */
+ }
+ }
+
+ /* Needed or should be delegated to memory contexts? */
+ pg_freeaddrinfo_all(hint.ai_family, addrs);
+ }
+
+ /* Close whatever sockets should be closed.
+ * There could be a better algorithm. */
+ n_closed = 0;
+ /* Iterate through and close... */
+ for (i = 0; i < NumListenSockets; i++)
+ {
+ if (remain_bound[i] == true)
+ continue;
+
+ ///* Get human-readable IP address for the log entry. */
+ //if (getsockname(ListenSockets[i], (struct sockaddr*)&sock_addr, &sock_addr_len) == -1) {
+ // ereport(ERROR, (errmsg("getsockname()")) );
+ //}
+ //
+ //if (sock_addr.ss_family == AF_INET) { /* IPv4 */
+ // struct sockaddr_in *s = (struct sockaddr_in *)&addr;
+ // inet_ntop(AF_INET, &s->sin_addr, ip_str, sizeof(ip_str));
+ //}
+ //else
+ //{ /* IPv6 */
+ // struct sockaddr_in6 *s6 = (struct sockaddr_in6 *)&addr;
+ // inet_ntop(AF_INET6, &s6->sin6_addr, ip_str, sizeof(ip_str));
+ //} /* We know we will not encounter UNIX sockets. */
+ ///* Any IPv4 interface is 0.0.0.0 on my machine. Why? */
+ //ereport(LOG, (errmsg("closing %s", ip_str)) );
+
+ closesocket(ListenSockets[i]);
+ ListenSockets[i] = -1;
+ n_closed++;
+ }
+ /* Having closed the sockets we created empty slots in between the array
+ * elements, get rid of them... */
+ j = 0;
+ for (i = 0; i < NumListenSockets; i++)
+ {
+ if (ListenSockets[i] != -1)
+ ListenSockets[j++] = ListenSockets[i];
+ }
+ NumListenSockets = j;
+
+ /* Resubscribe to socket updates. */
+ /* Is the if statement needed? */
+ if (n_bound > 0 || n_closed > 0)
+ ConfigurePostmasterWaitSet(true);
+
+ /* Should memory be handled by memory contexts? */
+ list_free(elemlist);
+ pfree(rawstring);
+}
+
/*
* on_proc_exit callback to delete external_pid_file
*/
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 6bc6be13d2a..e87cf690d4f 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -2943,11 +2943,12 @@
boot_val => 'DEFAULT_PGSOCKET_DIR',
},
-{ name => 'listen_addresses', type => 'string', context => 'PGC_POSTMASTER', group => 'CONN_AUTH_SETTINGS',
+{ name => 'listen_addresses', type => 'string', context => 'PGC_SIGHUP', group => 'CONN_AUTH_SETTINGS',
short_desc => 'Sets the host name or IP address(es) to listen to.',
flags => 'GUC_LIST_INPUT',
variable => 'ListenAddresses',
boot_val => '"localhost"',
+ assign_hook => 'assign_listen_addresses',
},
# Can't be set by ALTER SYSTEM as it can lead to recursive definition
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 82ac8646a8d..6b38d5c8e9c 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -81,6 +81,7 @@ extern bool check_log_stats(bool *newval, void **extra, GucSource source);
extern bool check_log_timezone(char **newval, void **extra, GucSource source);
extern void assign_log_timezone(const char *newval, void *extra);
extern const char *show_log_timezone(void);
+extern void assign_listen_addresses(const char *newval, void *extra);
extern void assign_maintenance_io_concurrency(int newval, void *extra);
extern void assign_io_max_combine_limit(int newval, void *extra);
extern void assign_io_combine_limit(int newval, void *extra);
--
2.51.0
Re: Mutable listen_addresses GUC
Ivan Kovmir <ivan.kovmir@cybertec.at> writes:
It is necessary to restart PostgreSQL to bind to a different network
interface, thus breaking the active connections.
On most platforms, if you set listen_addresses to "*" then there's no
problem, the kernel will automatically cope with IP address changes.
Do we really need more than that?
The reason I'm pushing back is that this patch looks extremely
complicated and hard-to-test. It also violates one of the fundamental
precepts of the GUC subsystem, namely that assign hooks shalt not
fail. And there is a lot of stuff you've not covered, such as the
logic around whether to register for Bonjour. We could possibly
make this feature work, but the cost-benefit ratio looks quite poor.
regards, tom lane