On 2020-11-18 04:35, David G. Johnston wrote:
Given that "port" is a postgresql.conf setting its use here (and
elsewhere) should be taken to mean the value of that specific variable.
To that end, I find the current description of port to be lacking - it
should mention its usage as a qualifier when dealing with unix socket
files (in addition to the existing wording under unix_socket_directories).
If we are going to debate semantics here "bind unix address" doesn't
seem correct. could not create Unix socket file /tmp/.s.PGSQL.5432, it
already exists.
The hint would be better written: Is another postmaster running with
unix_socket_directories = /tmp and port = 5432? If not, remove the unix
socket file /tmp/.s.PGSQL.5432 and retry.
I don't see much benefit in trying to share logic/wording between the
various messages and hints for the different ways the server can
establish communication points.
I agree that there isn't a useful hint for the abstract case as it
shouldn't happen unless there is indeed another running instance with
the same configuration. Though a hint similar to the above, but without
the "remove and retry" bit, probably wouldn't hurt.
I think we are getting a bit sidetracked here with the message wording.
The reason I looked at this was that "remove socket file and retry" is
never an appropriate action with abstract sockets. And on further
analysis, it is never an appropriate action with any Unix-domain socket
(because with file system namespace sockets, you never get an
EADDRINUSE, so it's dead code). So my proposal here is to just delete
that line from the hint and leave the rest the same. There could be
value in further refining and rephrasing this, but it ought to be a
separate thread.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
From a4967dd79e490390bc148d3bae92452ba4a34c9c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 22 Oct 2020 08:47:52 +0200
Subject: [PATCH v3 1/2] Add support for abstract Unix-domain sockets
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
Discussion:
https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0...@2ndquadrant.com
---
doc/src/sgml/config.sgml | 24 +++++++++++++++++++++++-
doc/src/sgml/libpq.sgml | 5 ++++-
src/backend/libpq/pqcomm.c | 8 ++++++++
src/bin/psql/command.c | 15 +++++----------
src/bin/psql/prompt.c | 3 ++-
src/common/ip.c | 24 +++++++++++++++++++++++-
src/include/libpq/pqcomm.h | 9 +++++++++
src/interfaces/libpq/fe-connect.c | 4 ++--
8 files changed, 76 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a632cf98ba..d05045fb20 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -749,6 +749,21 @@ <title>Connection Settings</title>
An empty value
specifies not listening on any Unix-domain sockets, in which case
only TCP/IP sockets can be used to connect to the server.
+ </para>
+
+ <para>
+ A value that starts with <literal>@</literal> specifies that a
+ Unix-domain socket in the abstract namespace should be created
+ (currently supported on Linux and Windows). In that case, this value
+ does not specify a <quote>directory</quote> but a prefix from which
+ the actual socket name is computed in the same manner as for the
+ file-system namespace. While the abstract socket name prefix can be
+ chosen freely, since it is not a file-system location, the convention
+ is to nonetheless use file-system-like values such as
+ <literal>@/tmp</literal>.
+ </para>
+
+ <para>
The default value is normally
<filename>/tmp</filename>, but that can be changed at build time.
On Windows, the default is empty, which means no Unix-domain socket is
@@ -763,6 +778,7 @@ <title>Connection Settings</title>
named <literal>.s.PGSQL.<replaceable>nnnn</replaceable>.lock</literal>
will be
created in each of the <varname>unix_socket_directories</varname>
directories.
Neither file should ever be removed manually.
+ For sockets in the abstract namespace, no lock file is created.
</para>
</listitem>
</varlistentry>
@@ -787,7 +803,8 @@ <title>Connection Settings</title>
<para>
This parameter is not supported on Windows. Any setting will be
- ignored.
+ ignored. Also, sockets in the abstract namespace have no file owner,
+ so this setting is also ignored in that case.
</para>
</listitem>
</varlistentry>
@@ -834,6 +851,11 @@ <title>Connection Settings</title>
similar effect by pointing <varname>unix_socket_directories</varname>
to a
directory having search permission limited to the desired audience.
</para>
+
+ <para>
+ Sockets in the abstract namespace have no file permissions, so this
+ setting is also ignored in that case.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9d4b6ab4a8..06bd412044 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1031,7 +1031,10 @@ <title>Parameter Key Words</title>
communication; the value is the name of the directory in which the
socket file is stored. (On Unix, an absolute path name begins with a
slash. On Windows, paths starting with drive letters are also
- recognized.) The default behavior when <literal>host</literal> is not
+ recognized.) If the host name starts with <literal>@</literal>, it is
+ taken as a Unix-domain socket in the abstract namespace (currently
+ supported on Linux and Windows).
+ The default behavior when <literal>host</literal> is not
specified, or is empty, is to connect to a Unix-domain
socket<indexterm><primary>Unix domain socket</primary></indexterm> in
<filename>/tmp</filename> (or whatever socket directory was specified
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index ac986c0505..84568508c5 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -611,6 +611,10 @@ StreamServerPort(int family, const char *hostName,
unsigned short portNumber,
static int
Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
{
+ /* no lock file for abstract sockets */
+ if (unixSocketPath[0] == '@')
+ return STATUS_OK;
+
/*
* Grab an interlock file associated with the socket file.
*
@@ -642,6 +646,10 @@ Lock_AF_UNIX(const char *unixSocketDir, const char
*unixSocketPath)
static int
Setup_AF_UNIX(const char *sock_path)
{
+ /* no file system permissions for abstract sockets */
+ if (sock_path[0] == '@')
+ return STATUS_OK;
+
/*
* Fix socket ownership/permission if requested. Note we must do this
* before we listen() to avoid a window where unwanted connections could
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c7a83d5dfc..55b349d55a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -37,6 +37,7 @@
#include "input.h"
#include "large_obj.h"
#include "libpq-fe.h"
+#include "libpq/pqcomm.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -604,12 +605,9 @@ exec_command_conninfo(PsqlScanState scan_state, bool
active_branch)
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is
via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
if (hostaddr && *hostaddr)
printf(_("You are connected to database
\"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"),
db, PQuser(pset.db),
hostaddr, PQport(pset.db));
@@ -3407,12 +3405,9 @@ do_connect(enum trivalue reuse_previous_specification,
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is
via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
if (hostaddr && *hostaddr)
printf(_("You are now connected to
database \"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"),
PQdb(pset.db),
PQuser(pset.db), hostaddr, PQport(pset.db));
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index ef503ec41b..f42c3dfc74 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/string.h"
#include "input.h"
+#include "libpq/pqcomm.h"
#include "prompt.h"
#include "settings.h"
@@ -136,7 +137,7 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
const char *host =
PQhost(pset.db);
/* INET socket */
- if (host && host[0] &&
!is_absolute_path(host))
+ if (host && host[0] &&
!is_unixsock_path(host))
{
strlcpy(buf, host,
sizeof(buf));
if (*p == 'm')
diff --git a/src/common/ip.c b/src/common/ip.c
index 69fcca8479..bcc779e00c 100644
--- a/src/common/ip.c
+++ b/src/common/ip.c
@@ -217,6 +217,21 @@ getaddrinfo_unix(const char *path, const struct addrinfo
*hintsp,
strcpy(unp->sun_path, path);
+ /*
+ * If the supplied path starts with @, replace that with a zero byte for
+ * the internal representation. In that mode, the entire sun_path is
the
+ * address, including trailing zero bytes. But we set the address
length
+ * to only include the length of the original string. That way the
+ * trailing zero bytes won't show up in any network or socket lists of
the
+ * operating system. This is just a convention, also followed by other
+ * packages.
+ */
+ if (path[0] == '@')
+ {
+ unp->sun_path[0] = '\0';
+ aip->ai_addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(path);
+ }
+
#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp->sun_len = sizeof(struct sockaddr_un);
#endif
@@ -249,7 +264,14 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
if (service)
{
- ret = snprintf(service, servicelen, "%s", sa->sun_path);
+ /*
+ * Check whether it looks like an abstract socket, but it could
also
+ * just be an empty string.
+ */
+ if (sa->sun_path[0] == '\0' && sa->sun_path[1] != '\0')
+ ret = snprintf(service, servicelen, "@%s", sa->sun_path
+ 1);
+ else
+ ret = snprintf(service, servicelen, "%s", sa->sun_path);
if (ret < 0 || ret >= servicelen)
return EAI_MEMORY;
}
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 781d86c8ef..cf967c3987 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -85,6 +85,15 @@ typedef struct
*/
#define UNIXSOCK_PATH_BUFLEN sizeof(((struct sockaddr_un *) NULL)->sun_path)
+/*
+ * A host that looks either like an absolute path or starts with @ is
+ * interpreted as a Unix-domain socket address.
+ */
+static inline bool
+is_unixsock_path(const char *path)
+{
+ return is_absolute_path(path) || path[0] == '@';
+}
/*
* These manipulate the frontend/backend protocol version number.
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index e7781d010f..7d04d3664e 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1093,7 +1093,7 @@ connectOptions2(PGconn *conn)
{
ch->type = CHT_HOST_NAME;
#ifdef HAVE_UNIX_SOCKETS
- if (is_absolute_path(ch->host))
+ if (is_unixsock_path(ch->host))
ch->type = CHT_UNIX_SOCKET;
#endif
}
@@ -6945,7 +6945,7 @@ passwordFromFile(const char *hostname, const char *port,
const char *dbname,
/* 'localhost' matches pghost of '' or the default socket directory */
if (hostname == NULL || hostname[0] == '\0')
hostname = DefaultHost;
- else if (is_absolute_path(hostname))
+ else if (is_unixsock_path(hostname))
/*
* We should probably use canonicalize_path(), but then we have
to
base-commit: 16f96c74d48e65da23d28665103e2c4c9d3414cc
--
2.29.2
From d996e9431adb50d0a45437d6aca14e8d7dcc7448 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 20 Nov 2020 15:59:18 +0100
Subject: [PATCH v3 2/2] Make error hint from bind() failure more accurate
The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address. In other cases it is misleading. So only show that hint
if errno == EADDRINUSE.
Also, since Unix-domain sockets in the file-system namespace never
report EADDRINUSE for an existing file (they would just overwrite it),
the part of the hint saying "If not, remove socket file \"%s\" and
retry." can never happen, so remove it. Unix-domain sockets in the
abstract namespace can report EADDRINUSE, but in that case there is no
file to remove, so the hint doesn't work there either.
Discussion:
https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0...@2ndquadrant.com
---
src/backend/libpq/pqcomm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 84568508c5..0e4efc8c10 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -530,18 +530,20 @@ StreamServerPort(int family, const char *hostName,
unsigned short portNumber,
err = bind(fd, addr->ai_addr, addr->ai_addrlen);
if (err < 0)
{
+ int saved_errno = errno;
+
ereport(LOG,
(errcode_for_socket_access(),
/* translator: first %s is IPv4, IPv6, or Unix */
errmsg("could not bind %s address
\"%s\": %m",
familyDesc, addrDesc),
- (IS_AF_UNIX(addr->ai_family)) ?
- errhint("Is another postmaster already
running on port %d?"
- " If not, remove
socket file \"%s\" and retry.",
- (int) portNumber,
service) :
- errhint("Is another postmaster already
running on port %d?"
- " If not, wait a few
seconds and retry.",
- (int) portNumber)));
+ saved_errno == EADDRINUSE ?
+ (IS_AF_UNIX(addr->ai_family) ?
+ errhint("Is another postmaster
already running on port %d?",
+ (int) portNumber) :
+ errhint("Is another postmaster
already running on port %d?"
+ " If not, wait a few
seconds and retry.",
+ (int) portNumber)) :
0));
closesocket(fd);
continue;
}
--
2.29.2
