Ability to listen on two unix sockets
Hi,
before I ask the main question, just a little background for one issue
we're currently having in Fedora 17:
PrivateTmp is a systemd's feature, which allows to have private /tmp
directory for services, which in turn means that such services aren't
able to access systems's /tmp directory. It's been enabled by some
services already, including Apache, while PostgreSQL uses system's /tmp
directory, where its unix socket is located. Naturally, it resulted in a
state, where Apache or other services with PrivateTmp enabled are not
able to communicate with PostgreSQL using the socket.
Since we don't want just to move socket for compatibility reasons, I'm
going to prepare a draft patch to allow PostgreSQL to use a second unix
socket at a time. A question I'd like to ask now is: Do we need a new
configuration variable for this or it's enough to have the location
hard-coded? What are your opinions?
Regards,
Honza
On Jun6, 2012, at 15:50 , Honza Horak wrote:
before I ask the main question, just a little background for one issue we're currently having in Fedora 17:
PrivateTmp is a systemd's feature, which allows to have private /tmp directory for services, which in turn means that such services aren't able to access systems's /tmp directory. It's been enabled by some services already, including Apache, while PostgreSQL uses system's /tmp directory, where its unix socket is located. Naturally, it resulted in a state, where Apache or other services with PrivateTmp enabled are not able to communicate with PostgreSQL using the socket.
Couldn't you simply tell postgres to put it's socket in, say, /var/run, and create a symlink to that socket in the global /tmp directory?
Since we don't want just to move socket for compatibility reasons, I'm going to prepare a draft patch to allow PostgreSQL to use a second unix socket at a time. A question I'd like to ask now is: Do we need a new configuration variable for this or it's enough to have the location hard-coded? What are your opinions?
If we're going to have this at all, we should go all the way and support an arbitrary number of sockets. But then, is there any advantage in providing this feature natively compare to simply creating symlinks?
best regards,
Florian Pflug
Florian Pflug <fgp@phlo.org> writes:
Couldn't you simply tell postgres to put it's socket in, say, /var/run, and create a symlink to that socket in the global /tmp directory?
FYI, this proposal emerged out of a discussion between Honza and
myself. "Use a symlink" was my first idea too, but on reflection
it seems like it will take less new code to support two sockets.
We already support multiple TCP sockets, so multiple Unix sockets
shouldn't be that much extra trouble.
The reasons a symlink doesn't seem attractive are:
1. The code to create/delete it has to be in the postmaster. If we
tried to make the Fedora-specific startup script manage it, we would
first have to teach that script how to know which port number the
postmaster will select, which means parsing config files. Ugh.
2. What if two postmasters try to create a symlink in the same place?
Or we're just trying to decide if the previous creator crashed without
removing it? So we need a lockfile beside it. So at this point we are
building a whole bunch of new infrastructure to create symlinks, whereas
we can probably just call the same subroutine twice if we go with the
two-socket design.
If we're going to have this at all, we should go all the way and
support an arbitrary number of sockets.
Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.
You might think we should design this exactly like the TCP-socket
multiple-listen-addresses case, ie just have a config variable
containing a list of directory names. The sticking point there is
that the directories aren't really interchangeable. In particular,
there is still going to be one directory that is the one hard-wired
into libpq. So whereas multiple TCP sockets really are pretty much
interchangeable, I think in the Unix-socket case we are going to have
to think of it as being a primary socket and one or more alternate
sockets. Is there a reason to have more than one alternate, and if
so what is the use-case?
(BTW, we would probably just adopt the Debian solution if we were
sure there were no non-libpq clients out there; but we aren't.)
regards, tom lane
On Wednesday, June 06, 2012 04:38:42 PM Tom Lane wrote:
Florian Pflug <fgp@phlo.org> writes:
If we're going to have this at all, we should go all the way and
support an arbitrary number of sockets.Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.
I had the need to make pg available from multiple chroots via unix sockets.
The same might come up more frequently with the availability of filesystem
namespaces...
You might think we should design this exactly like the TCP-socket
multiple-listen-addresses case, ie just have a config variable
containing a list of directory names. The sticking point there is
that the directories aren't really interchangeable. In particular,
there is still going to be one directory that is the one hard-wired
into libpq.
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Excerpts from Tom Lane's message of mié jun 06 10:38:42 -0400 2012:
Florian Pflug <fgp@phlo.org> writes:
Couldn't you simply tell postgres to put it's socket in, say, /var/run, and create a symlink to that socket in the global /tmp directory?
FYI, this proposal emerged out of a discussion between Honza and
myself. "Use a symlink" was my first idea too, but on reflection
it seems like it will take less new code to support two sockets.
We already support multiple TCP sockets, so multiple Unix sockets
shouldn't be that much extra trouble.The reasons a symlink doesn't seem attractive are:
1. The code to create/delete it has to be in the postmaster. If we
tried to make the Fedora-specific startup script manage it, we would
first have to teach that script how to know which port number the
postmaster will select, which means parsing config files. Ugh.
Well, you could use
postmaster -C port
The other reason seems compelling enough, though ... particularly,
handling a lockfile sounds messy; if it's a symlink and it's created by
the script, then it would need a separate lockfile, and filling its data
wouldn't be exactly trivial.
(BTW, we would probably just adopt the Debian solution if we were
sure there were no non-libpq clients out there; but we aren't.)
Maybe this is a good time to make the /var/run socket location (Debian's
choice) the primary one, and /tmp be the alternate.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On 06/06/2012 04:50 PM, Andres Freund wrote:
On Wednesday, June 06, 2012 04:38:42 PM Tom Lane wrote:
Florian Pflug<fgp@phlo.org> writes:
If we're going to have this at all, we should go all the way and
support an arbitrary number of sockets.Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.I had the need to make pg available from multiple chroots via unix sockets.
The same might come up more frequently with the availability of filesystem
namespaces...
It seems you were not alone with such need:
http://archives.postgresql.org/pgsql-novice/2006-09/msg00172.php
Honza
Alvaro Herrera <alvherre@commandprompt.com> writes:
Excerpts from Tom Lane's message of mié jun 06 10:38:42 -0400 2012:
(BTW, we would probably just adopt the Debian solution if we were
sure there were no non-libpq clients out there; but we aren't.)
Maybe this is a good time to make the /var/run socket location (Debian's
choice) the primary one, and /tmp be the alternate.
I'm not really in favor of making /var/run be the out-of-the-box
default, because it would discriminate against personal/testing
installations (ie, you couldn't set it up without root privileges).
It's a reasonable choice for distro-provided packages, but not so
much for one-off builds --- think about the buildfarm if nothing else.
Having said that, if we made it easier to configure things that way than
by patching the source, I bet Martin Pitt isn't going to object.
regards, tom lane
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
On Wednesday, June 06, 2012 04:38:42 PM Tom Lane wrote:
Florian Pflug<fgp@phlo.org> writes:
If we're going to have this at all, we should go all the way and
support an arbitrary number of sockets.
Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.
I had the need to make pg available from multiple chroots via unix sockets.
The same might come up more frequently with the availability of filesystem
namespaces...
It seems you were not alone with such need:
http://archives.postgresql.org/pgsql-novice/2006-09/msg00172.php
I had forgotten that conversation, but it does seem like there is
interest in this type of configuration. Can anybody confirm that
dropping a socket into a chroot or jail would actually work, ie
make it possible to connect from inside the chroot to a postmaster
running outside? If that's real and not just wishful thinking,
it seems like enough of an argument to justify supporting N sockets.
regards, tom lane
On Wed, Jun 06, 2012 at 11:32:45AM -0400, Tom Lane wrote:
I had forgotten that conversation, but it does seem like there is
interest in this type of configuration. Can anybody confirm that
dropping a socket into a chroot or jail would actually work, ie
make it possible to connect from inside the chroot to a postmaster
running outside? If that's real and not just wishful thinking,
it seems like enough of an argument to justify supporting N sockets.
We need to deal with exactly this sort of issue with schroot, where
we may want to provide programs in the chroot with access to
facilities outside the chroot. We generally just bind mount in the
minimal set of stuff needed. This might mean binding just the socket,
or it could be /var/run/postgresql. We do this for the X11 socket
for our desktop configuration profile to permit X11 programs to run
in a chroot, though we currently bind mount all of /tmp rather than
just the socket, since we want that as well in any case.
(http://people.debian.org/~rleigh/schroot.pdf)
While not exactly what was proposed (multiple sockets), this allows
one to re-use a single socket without the daemon requiring any
special support for it.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' schroot and sbuild http://alioth.debian.org/projects/buildd-tools
`- GPG Public Key F33D 281D 470A B443 6756 147C 07B3 C8BC 4083 E800
On Wed, Jun 6, 2012 at 10:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.You might think we should design this exactly like the TCP-socket
multiple-listen-addresses case, ie just have a config variable
containing a list of directory names. The sticking point there is
that the directories aren't really interchangeable. In particular,
there is still going to be one directory that is the one hard-wired
into libpq. So whereas multiple TCP sockets really are pretty much
interchangeable, I think in the Unix-socket case we are going to have
to think of it as being a primary socket and one or more alternate
sockets. Is there a reason to have more than one alternate, and if
so what is the use-case?(BTW, we would probably just adopt the Debian solution if we were
sure there were no non-libpq clients out there; but we aren't.)
I recently had an urge to make it possible for the postmaster to
listen on multiple ports and even went so far as to code up a patch to
allow that. It still applies, with offsets, so I'll attach it here.
So I guess I'm +1 on the idea of allowing N UNIX sockets rather than
limiting it to N=2, and really I'd like to do one better and allow
listening on multiple TCP ports as well. Since the PID file contains
the port number, multiple TCP sockets stop being interchangeable as
soon as you allow multiple ports, but that's not very difficult to
handle. Now, you might ask whether this has any real-world value, and
obviously I'm going to say yes or I wouldn't be proposing it. The
reason for wanting multiple UNIX sockets is because those sockets
might be in different places that are not all equally accessible to
everyone, because of things like chroot. But of course the same thing
is possible in the network space using iptables and similar tools.
For example, you might want to have users connect to application A
using port 5432, and to application B using port 15432. Now you can
use network monitoring tools to see how much data each application is
sending and receiving, without needing deep packet inspection. You
can firewall those ports differently to provide access to different
groups of users. And you can even decide, if the database gets
overloaded, to cut off access to one of those ports, so that the
application causing the problem becomes inaccessible but the rest of
the database ceases being overloaded and you can still operate. Of
course, you could also do that by changing pg_hba.conf, but for some
people it might be more convenient (or feel more bullet-proof) to do
it using network management tools. There are probably other use
cases, as well.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
additional-sockets.patchapplication/octet-stream; name=additional-sockets.patchDownload
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index dd7dd55..a4745cc 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -156,9 +156,16 @@ static Backend *ShmemBackendArray;
/* The socket number we are listening for connections on */
int PostPortNumber;
+
+/* UNIX socket directory */
char *UnixSocketDir;
+
+/* Addresses to bind to, for port number above */
char *ListenAddresses;
+/* Additional sockets */
+char *additional_sockets;
+
/*
* ReservedBackends is the number of backends reserved for superuser use.
* This number is taken out of the pool size given by MaxBackends so
@@ -896,6 +903,86 @@ PostmasterMain(int argc, char *argv[])
pfree(rawstring);
}
+ if (additional_sockets)
+ {
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+
+ /* Need a modifiable copy of additional_sockets */
+ rawstring = pstrdup(additional_sockets);
+
+ /* Parse string into list of identifiers */
+ if (!SplitIdentifierString(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid list syntax for \"additional_sockets\"")));
+ }
+
+ foreach(l, elemlist)
+ {
+ char *cursocket = (char *) lfirst(l);
+ char *colon;
+ char *host;
+ char *port;
+ int64 portnum;
+ char *endptr;
+ bool bad_strtol;
+
+ /* syntax is port or host:port */
+ colon = strchr(cursocket, ':');
+ if (colon != NULL)
+ {
+ host = cursocket;
+ port = colon + 1;
+ *colon = '\0';
+ }
+ else
+ {
+ host = NULL;
+ port = cursocket;
+ }
+
+ /*
+ * convert port number to integer, ignoring optional trailing
+ * whitespace). strtol should ignore leading whitespace, too.
+ */
+ portnum = strtol(port, &endptr, 0);
+ bad_strtol = (errno == ERANGE);
+ while (isspace((unsigned char) *endptr))
+ endptr++;
+ if (*endptr != '\0')
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" invalid in \"additional_sockets\"", port)));
+ if (bad_strtol || portnum < 1 || portnum > 65535)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" out of range in \"additional_sockets\"", port)));
+
+
+ status = StreamServerPort(AF_UNSPEC, host,
+ (unsigned short) portnum,
+ UnixSocketDir,
+ ListenSocket, MAXLISTEN);
+
+ if (status != STATUS_OK)
+ {
+ /* put the colon back, for error-reporting purposes */
+ if (colon != NULL)
+ *colon = ':';
+ ereport(WARNING,
+ (errmsg("could not create additional socket for \"%s\"",
+ cursocket)));
+ }
+ }
+
+ list_free(elemlist);
+ pfree(rawstring);
+ }
+
#ifdef USE_BONJOUR
/* Register for Bonjour only if we opened TCP socket(s) */
if (enable_bonjour && ListenSocket[0] != PGINVALID_SOCKET)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d75ab43..2429011 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2650,6 +2650,16 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
+ {
+ {"additional_sockets", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop("Listen on additional sockets."),
+ NULL
+ },
+ &additional_sockets,
+ "",
+ NULL, NULL, NULL
+ },
+
/* See main.c about why defaults for LC_foo are not all alike */
{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index fa75d00..ce1c385 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -61,6 +61,8 @@
# defaults to 'localhost'; use '*' for all
# (change requires restart)
#port = 5432 # (change requires restart)
+#additional_sockets = '' # comma-separated list of [host:]port
+ # (change requires restart)
#max_connections = 100 # (change requires restart)
# Note: Increasing max_connections costs ~400 bytes of shared memory per
# connection slot, plus lock space (see max_locks_per_transaction).
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index dded0e6..3d9ea41 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -17,6 +17,7 @@
extern bool EnableSSL;
extern int ReservedBackends;
extern int PostPortNumber;
+extern char *additional_sockets;
extern int Unix_socket_permissions;
extern char *Unix_socket_group;
extern char *UnixSocketDir;
On Wed, Jun 6, 2012 at 1:55 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 6, 2012 at 10:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Well, that's what I wanted to discuss before Honza starts coding.
It's not obvious that there are any use-cases for more than two.
It's also not clear whether there is any value in supporting run-time
rather than build-time configuration of the socket locations. The
Fedora use-case has no need of that, but if people can point to other
cases where it would be sensible, we can write the patch that way.You might think we should design this exactly like the TCP-socket
multiple-listen-addresses case, ie just have a config variable
containing a list of directory names. The sticking point there is
that the directories aren't really interchangeable. In particular,
there is still going to be one directory that is the one hard-wired
into libpq. So whereas multiple TCP sockets really are pretty much
interchangeable, I think in the Unix-socket case we are going to have
to think of it as being a primary socket and one or more alternate
sockets. Is there a reason to have more than one alternate, and if
so what is the use-case?(BTW, we would probably just adopt the Debian solution if we were
sure there were no non-libpq clients out there; but we aren't.)I recently had an urge to make it possible for the postmaster to
listen on multiple ports and even went so far as to code up a patch to
allow that. It still applies, with offsets, so I'll attach it here.
So I guess I'm +1 on the idea of allowing N UNIX sockets rather than
limiting it to N=2, and really I'd like to do one better and allow
listening on multiple TCP ports as well. Since the PID file contains
the port number, multiple TCP sockets stop being interchangeable as
soon as you allow multiple ports, but that's not very difficult to
handle. Now, you might ask whether this has any real-world value, and
obviously I'm going to say yes or I wouldn't be proposing it. The
reason for wanting multiple UNIX sockets is because those sockets
might be in different places that are not all equally accessible to
everyone, because of things like chroot. But of course the same thing
is possible in the network space using iptables and similar tools.
For example, you might want to have users connect to application A
using port 5432, and to application B using port 15432. Now you can
use network monitoring tools to see how much data each application is
sending and receiving, without needing deep packet inspection. You
can firewall those ports differently to provide access to different
groups of users. And you can even decide, if the database gets
overloaded, to cut off access to one of those ports, so that the
application causing the problem becomes inaccessible but the rest of
the database ceases being overloaded and you can still operate. Of
course, you could also do that by changing pg_hba.conf, but for some
people it might be more convenient (or feel more bullet-proof) to do
it using network management tools. There are probably other use
cases, as well.
+1 for multiple TCP port numbers.
A few days ago I started working on enabling Postgres to communicate using
WebSockets protocol (acting as a wrapper around FE/BE), and I found it
difficult (not impossible) to use the same port for communicating FE/BE
protocol and for https+WebSockets too. It would have been a lot simpler if
I could say that WebSockets is enabled on 5431 and FE/BE on 5432.
Regards,
--
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
On 06/06/2012 04:50 PM, Andres Freund wrote:
On Wednesday, June 06, 2012 04:38:42 PM Tom Lane wrote:
You might think we should design this exactly like the TCP-socket
multiple-listen-addresses case, ie just have a config variable
containing a list of directory names. The sticking point there is
that the directories aren't really interchangeable. In particular,
there is still going to be one directory that is the one hard-wired
into libpq.I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.
I guess so. Let's say we add additional socket support and some server
uses one e.g. at /var/run/postgresql. Then a client can either (1)
specify the same path explicitly and then we don't need to specify any
additional sockets on the client side or (2) stick to the default path,
which is hard-coded, currently to /tmp.
Going back to the original problem (inaccessible /tmp directory), it is
the case (2) -- a client uses the default path. So any additional
client-side socket option won't probably help here, but we would
probably need a second hard-coded path e.g. at /var/run/postgresql.
Regards,
Honza
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.
I guess so.
I don't really want to go there. Some use cases have been shown in
this thread for having a server listen in multiple places, but that does
not translate to saying that clients need to support automatically
looking in multiple places. I think that mainly introduces questions we
could do without, like which server did you actually end up contacting.
regards, tom lane
On Thursday, June 07, 2012 05:55:11 PM Tom Lane wrote:
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.I guess so.
I don't really want to go there. Some use cases have been shown in
this thread for having a server listen in multiple places, but that does
not translate to saying that clients need to support automatically
looking in multiple places. I think that mainly introduces questions we
could do without, like which server did you actually end up contacting.
It would be really nice to have a development psql connect to a distro
installed psql and vice versa without having to specify -h /var/run/psql and -
h /tmp all the time...
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Jun 7, 2012 at 11:57 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On Thursday, June 07, 2012 05:55:11 PM Tom Lane wrote:
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.I guess so.
I don't really want to go there. Some use cases have been shown in
this thread for having a server listen in multiple places, but that does
not translate to saying that clients need to support automatically
looking in multiple places. I think that mainly introduces questions we
could do without, like which server did you actually end up contacting.It would be really nice to have a development psql connect to a distro
installed psql and vice versa without having to specify -h /var/run/psql and -
h /tmp all the time...
This is true, but you have this problem already. It might be worth
fixing, but it seems like a separate issue from the topic of this
thread, which is where the server listens.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Andres Freund <andres@2ndquadrant.com> writes:
On Thursday, June 07, 2012 05:55:11 PM Tom Lane wrote:
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.
I don't really want to go there.
It would be really nice to have a development psql connect to a distro
installed psql and vice versa without having to specify -h /var/run/psql and -
h /tmp all the time...
I don't find that "nice" at all. Which server did you actually connect
to? How do you control it? You're going to end up needing the -h
switch anyway.
regards, tom lane
On Thursday, June 07, 2012 06:20:32 PM Tom Lane wrote:
Andres Freund <andres@2ndquadrant.com> writes:
On Thursday, June 07, 2012 05:55:11 PM Tom Lane wrote:
Honza Horak <hhorak@redhat.com> writes:
On 06/06/2012 04:50 PM, Andres Freund wrote:
I wonder if the whole issue doesn't require libpq to also try multiple
hardcoded socket locations.I don't really want to go there.
It would be really nice to have a development psql connect to a distro
installed psql and vice versa without having to specify -h /var/run/psql
and - h /tmp all the time...I don't find that "nice" at all. Which server did you actually connect
to? How do you control it? You're going to end up needing the -h
switch anyway.
They can't run on the same port anyway unless you disable listening on
localhost. Changing a single port number is far less effort than typing -h
/var/run/postgresql ;)
Anyway, I am not wed to this, and I don't plan to put work into it so I better
shut up ;)
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On ons, 2012-06-06 at 11:21 -0400, Tom Lane wrote:
I'm not really in favor of making /var/run be the out-of-the-box
default, because it would discriminate against personal/testing
installations (ie, you couldn't set it up without root privileges).
It's a reasonable choice for distro-provided packages, but not so
much for one-off builds --- think about the buildfarm if nothing else.Having said that, if we made it easier to configure things that way
than by patching the source, I bet Martin Pitt isn't going to object.
We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.
Show quoted text
On Saturday, June 09, 2012 11:43:53 PM Peter Eisentraut wrote:
On ons, 2012-06-06 at 11:21 -0400, Tom Lane wrote:
I'm not really in favor of making /var/run be the out-of-the-box
default, because it would discriminate against personal/testing
installations (ie, you couldn't set it up without root privileges).
It's a reasonable choice for distro-provided packages, but not so
much for one-off builds --- think about the buildfarm if nothing else.Having said that, if we made it easier to configure things that way
than by patching the source, I bet Martin Pitt isn't going to object.We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.
That doesn't help libpq using clients.
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Peter Eisentraut <peter_e@gmx.net> writes:
We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.
Um, no, it's not quite that simple. In particular, what do you think
should happen on the client side?
I'm inclined to think that we should (continue to) have a hardwired
"primary" directory, which is the one that libpq is also configured
to look in by default. But we could add a run-time-configured list
of secondary directories to establish sockets in.
regards, tom lane
On lör, 2012-06-09 at 23:45 +0200, Andres Freund wrote:
On Saturday, June 09, 2012 11:43:53 PM Peter Eisentraut wrote:
On ons, 2012-06-06 at 11:21 -0400, Tom Lane wrote:
I'm not really in favor of making /var/run be the out-of-the-box
default, because it would discriminate against personal/testing
installations (ie, you couldn't set it up without root privileges).
It's a reasonable choice for distro-provided packages, but not so
much for one-off builds --- think about the buildfarm if nothing else.Having said that, if we made it easier to configure things that way
than by patching the source, I bet Martin Pitt isn't going to object.We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.That doesn't help libpq using clients.
There is no mandate here to do anything about that.
On lör, 2012-06-09 at 18:02 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.Um, no, it's not quite that simple. In particular, what do you think
should happen on the client side?
Nothing changes there.
I'm inclined to think that we should (continue to) have a hardwired
"primary" directory, which is the one that libpq is also configured
to look in by default.
Yes.
But we could add a run-time-configured list of secondary directories to establish sockets in.
Yes, I'm just pointing out that we already have that list
(unix_socket_directory in postgresql.conf), except it's currently
limited to length 1, because no one has needed a longer one until now.
On Sunday, June 10, 2012 12:09:30 AM Peter Eisentraut wrote:
On lör, 2012-06-09 at 23:45 +0200, Andres Freund wrote:
On Saturday, June 09, 2012 11:43:53 PM Peter Eisentraut wrote:
On ons, 2012-06-06 at 11:21 -0400, Tom Lane wrote:
I'm not really in favor of making /var/run be the out-of-the-box
default, because it would discriminate against personal/testing
installations (ie, you couldn't set it up without root privileges).
It's a reasonable choice for distro-provided packages, but not so
much for one-off builds --- think about the buildfarm if nothing
else.Having said that, if we made it easier to configure things that way
than by patching the source, I bet Martin Pitt isn't going to object.We already have the ability to configure the Unix socket directory in
postgresql.conf. All you need to do is turn that into a list.That doesn't help libpq using clients.
There is no mandate here to do anything about that.
Well, Martin Pitt/the debian package is patching exactly that. Youre saying
that everything that needs to be done to make that easier is to make
unix_socket_dir a list. So there seems to be some disparity there ;)
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Peter Eisentraut <peter_e@gmx.net> writes:
On lör, 2012-06-09 at 18:02 -0400, Tom Lane wrote:
I'm inclined to think that we should (continue to) have a hardwired
"primary" directory, which is the one that libpq is also configured
to look in by default.
Yes.
But we could add a run-time-configured list of secondary directories to establish sockets in.
Yes, I'm just pointing out that we already have that list
(unix_socket_directory in postgresql.conf), except it's currently
limited to length 1, because no one has needed a longer one until now.
That's not actually quite the same thing as what I suggest above.
Currently, unix_socket_directory *overrides* the compiled-in choice.
I'm suggesting that it would be better to invent a list that is *added
to* the compiled-in choice. If we think it would be best to still be
able to override that, then I'd vote for keeping unix_socket_directory
as is, and then adding a list named something like
"secondary_socket_directories". But if we just turn
unix_socket_directory into a list, I think the lack of separation
between primary and secondary directories will be confusing.
Or maybe I'm wrong and it's better doing it as you suggest, but I
think this needs consideration.
regards, tom lane
On sön, 2012-06-10 at 00:25 +0200, Andres Freund wrote:
We already have the ability to configure the Unix socket
directory in
postgresql.conf. All you need to do is turn that into a list.
That doesn't help libpq using clients.
There is no mandate here to do anything about that.
Well, Martin Pitt/the debian package is patching exactly that. Youre
saying
that everything that needs to be done to make that easier is to make
unix_socket_dir a list. So there seems to be some disparity there ;)
The Debian package doesn't need any change or assistance, really. You
can change the default location of the socket by patching
pg_config_manual.h, and that's a one-liner. The only thing that would
make that slightly easier or better is making it a configure option, but
that was intentionally decided against in the old days (not by me).
On lör, 2012-06-09 at 18:26 -0400, Tom Lane wrote:
Yes, I'm just pointing out that we already have that list
(unix_socket_directory in postgresql.conf), except it's currently
limited to length 1, because no one has needed a longer one untilnow.
That's not actually quite the same thing as what I suggest above.
Currently, unix_socket_directory *overrides* the compiled-in choice.
I'm suggesting that it would be better to invent a list that is *added
to* the compiled-in choice. If we think it would be best to still be
able to override that, then I'd vote for keeping unix_socket_directory
as is, and then adding a list named something like
"secondary_socket_directories". But if we just turn
unix_socket_directory into a list, I think the lack of separation
between primary and secondary directories will be confusing.
By that logic, any list-valued parameter should be split into a primary
and secondary setting. That could actually be moderately useful in some
cases (think search_path, or if we get there, multiple port settings),
but then we should put this into the grammar or processing logic of
postgresql.conf, not invent a bunch of new settings. (E.g.,
unix_socket_directory += ...).
Peter Eisentraut <peter_e@gmx.net> writes:
On lör, 2012-06-09 at 18:26 -0400, Tom Lane wrote:
That's not actually quite the same thing as what I suggest above.
Currently, unix_socket_directory *overrides* the compiled-in choice.
I'm suggesting that it would be better to invent a list that is *added
to* the compiled-in choice. If we think it would be best to still be
able to override that, then I'd vote for keeping unix_socket_directory
as is, and then adding a list named something like
"secondary_socket_directories". But if we just turn
unix_socket_directory into a list, I think the lack of separation
between primary and secondary directories will be confusing.
By that logic, any list-valued parameter should be split into a primary
and secondary setting.
Well, no: the key point here is that there will be one directory that is
special because it's the one baked into libpq. I agree that for the
purposes of the backend in isolation, we might as well just have a list.
What's less clear is whether, when considering the backend+client
ecosystem as a whole, the special status of the configure-time socket
directory ought to be reflected in the way we set up the GUCs. I have
to admit that I'm not totally sold on either approach.
regards, tom lane
On Sun, Jun 10, 2012 at 8:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
On lör, 2012-06-09 at 18:26 -0400, Tom Lane wrote:
That's not actually quite the same thing as what I suggest above.
Currently, unix_socket_directory *overrides* the compiled-in choice.
I'm suggesting that it would be better to invent a list that is *added
to* the compiled-in choice. If we think it would be best to still be
able to override that, then I'd vote for keeping unix_socket_directory
as is, and then adding a list named something like
"secondary_socket_directories". But if we just turn
unix_socket_directory into a list, I think the lack of separation
between primary and secondary directories will be confusing.By that logic, any list-valued parameter should be split into a primary
and secondary setting.Well, no: the key point here is that there will be one directory that is
special because it's the one baked into libpq. I agree that for the
purposes of the backend in isolation, we might as well just have a list.
What's less clear is whether, when considering the backend+client
ecosystem as a whole, the special status of the configure-time socket
directory ought to be reflected in the way we set up the GUCs. I have
to admit that I'm not totally sold on either approach.
I think we should consider this in the context of allowing both
additional UNIX sockets and additional TCP ports. In the case of TCP
ports, it's clearly no good to turn "port" into a list, because one
port number has to be primary, since it goes into the PID file and
also affects the naming of any UNIX sockets created. If we add
secondary_socket_dirs, I think we will also need secondary_ports. One
idea might be to have one new GUC that serves both purposes:
additional_sockets = '12345, /foo'
I'm sure there are other ways to skin the cat as well.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On sön, 2012-06-10 at 09:41 -0400, Robert Haas wrote:
I think we should consider this in the context of allowing both
additional UNIX sockets and additional TCP ports. In the case of TCP
ports, it's clearly no good to turn "port" into a list, because one
port number has to be primary, since it goes into the PID file
Not necessarily. The port number in the PID file is only used for
pg_ctl to contact the server, and for that it only needs some port, not
the primary one. Also, we write the first listen_address into the PID
file for the same reason. So if you think there should be a primary
port, then there should also be a primary listen_addresses.
and also affects the naming of any UNIX sockets created.
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created.
If we add
secondary_socket_dirs, I think we will also need secondary_ports. One
idea might be to have one new GUC that serves both purposes:additional_sockets = '12345, /foo'
I was getting around to that, although I don't follow the specific
syntax you have chosen here.
I would like something where you set host and port together, so you can
listen on port 5432 on localhost, and port 5433 on *, for example. So
maybe
listen_addresses = localhost:5432,*:5433
Web servers usually allow that sort of thing, but if you dig deep there,
the configuration settings and their interactions can get pretty
complicated. For example, you can usually set a default port and then
override it in the listen_addresses equivalent.
On Sun, Jun 10, 2012 at 5:06 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On sön, 2012-06-10 at 09:41 -0400, Robert Haas wrote:
I think we should consider this in the context of allowing both
additional UNIX sockets and additional TCP ports. In the case of TCP
ports, it's clearly no good to turn "port" into a list, because one
port number has to be primary, since it goes into the PID fileNot necessarily. The port number in the PID file is only used for
pg_ctl to contact the server, and for that it only needs some port, not
the primary one. Also, we write the first listen_address into the PID
file for the same reason. So if you think there should be a primary
port, then there should also be a primary listen_addresses.
Fair enough, as far as this part goes, but...
and also affects the naming of any UNIX sockets created.
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created.
...I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.
If we add
secondary_socket_dirs, I think we will also need secondary_ports. One
idea might be to have one new GUC that serves both purposes:additional_sockets = '12345, /foo'
I was getting around to that, although I don't follow the specific
syntax you have chosen here.
I was thinking that each element could be of the form /path or port.
But I guess it should really be /path or host:port.
I would like something where you set host and port together, so you can
listen on port 5432 on localhost, and port 5433 on *, for example. So
maybelisten_addresses = localhost:5432,*:5433
Web servers usually allow that sort of thing, but if you dig deep there,
the configuration settings and their interactions can get pretty
complicated. For example, you can usually set a default port and then
override it in the listen_addresses equivalent.
That seems like the obvious syntax, but I'm fuzzy on the details. We
could let 'port' continue to mean the default port, and then
listen_addresses can contain either unadorned addresses (in which case
we bind to that address using the default port) or address:port
designators (in which case we bind to the given address and port).
But then support port = 1234 and listen_addresses = '5.5.5.5:6789'.
Presumably the UNIX socket is still /tmp/.s.PGSQL.1234, but then what
ends up in the lock file? PostmasterMain's current algorithm for
figuring that out would write 5.5.5.5 for the host and 1234 for the
port, which seems like nonsense. Such confusion is why I thought we
might fall back on listing all the additional listen locations in a
new, separate GUC. But perhaps there is a way to make it work.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Jun 10, 2012 at 5:06 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On s�n, 2012-06-10 at 09:41 -0400, Robert Haas wrote:
If we add
secondary_socket_dirs, I think we will also need secondary_ports. �One
idea might be to have one new GUC that serves both purposes:additional_sockets = '12345, /foo'
I was getting around to that, although I don't follow the specific
syntax you have chosen here.
I was thinking that each element could be of the form /path or port.
But I guess it should really be /path or host:port.
I'm uncomfortable with the potential for ambiguity there. I think we'd
be much better off having two lists, one for TCP addresses and one for
Unix socket directories.
I'm unconvinced that allowing multiple port numbers is worth the
amount of confusion it will cause. In particular, we've traditionally
used "the port number" as part of the key for resources such as shared
memory. I think we'd want the number used for that purpose to be what
is written into the lock file ... but then what if the postmaster is not
actually listening on *any* actual socket with that number? pg_ctl will
not be happy.
regards, tom lane
On Sun, Jun 10, 2012 at 5:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Jun 10, 2012 at 5:06 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On sön, 2012-06-10 at 09:41 -0400, Robert Haas wrote:
If we add
secondary_socket_dirs, I think we will also need secondary_ports. One
idea might be to have one new GUC that serves both purposes:additional_sockets = '12345, /foo'
I was getting around to that, although I don't follow the specific
syntax you have chosen here.I was thinking that each element could be of the form /path or port.
But I guess it should really be /path or host:port.I'm uncomfortable with the potential for ambiguity there. I think we'd
be much better off having two lists, one for TCP addresses and one for
Unix socket directories.
I suggested it mostly because we already use that convention in libpq:
leading slash = pathname.
I'm unconvinced that allowing multiple port numbers is worth the
amount of confusion it will cause. In particular, we've traditionally
used "the port number" as part of the key for resources such as shared
memory. I think we'd want the number used for that purpose to be what
is written into the lock file ... but then what if the postmaster is not
actually listening on *any* actual socket with that number? pg_ctl will
not be happy.
Well, that's why I shied away from the syntax Peter is proposing. I
think if we leave the semantics of listen_addresses and port alone,
and just add a new GUC for extra places to listen, there's no problem.
If you look at the patch I posted upthread, you'll see that I set
things up so that we'll still fail if the primary port can't be
listened on; once we've established that we can listen there, we'll
try to set up the other sockets as well. I think that's a pretty sane
way to allow this (which a number of people, not only me, seem to
support) without creating surprising semantics.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On sön, 2012-06-10 at 17:24 -0400, Robert Haas wrote:
and also affects the naming of any UNIX sockets created.
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created....I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.
How else would it work?
If I say, syntax aside, listen on "ports" 5432 and 5433, and use socket
directories /tmp and /var/run/postgresql, then a libpq-using client
would expect to be able to connect using
-h /tmp -p 5432
-h /tmp -p 5433
-h /var/run/postgresql -p 5432
-h /var/run/postgresql -p 5433
So you do need to create M*N sockets.
I don't really see a problem with that.
On sön, 2012-06-10 at 17:35 -0400, Tom Lane wrote:
I'm unconvinced that allowing multiple port numbers is worth the
amount of confusion it will cause.
Well, it's a feature that people have asked for. I would love to have
it. Much more than multiple Unix-domain socket locations.
In particular, we've traditionally
used "the port number" as part of the key for resources such as shared
memory.
But it hasn't been a requirement for a long time that those match up
exactly. It's already possible that they don't, if you configure
postmasters with the same port and non-conflicting IP addresses or
Unix-socket locations.
I think we'd want the number used for that purpose to be what
is written into the lock file ... but then what if the postmaster is
not actually listening on *any* actual socket with that number?
pg_ctl will not be happy.
I'm not sure why pg_ctl needs to know about the shared memory business.
We write the shared memory key into the lock file, so the port number in
the lock file should just be a port number for pg_ctl to use. Of course
you can configure things so that pg_ctl cannot contact the postmaster,
but this problem already exists in a more likely fashion with
listen_addresses. Adding an extra port doesn't make it more likely.
Peter Eisentraut <peter_e@gmx.net> writes:
On sön, 2012-06-10 at 17:24 -0400, Robert Haas wrote:
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created.
...I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.
How else would it work?
If I say, syntax aside, listen on "ports" 5432 and 5433, and use socket
directories /tmp and /var/run/postgresql, then a libpq-using client
would expect to be able to connect using
This argument seems quite circular to me: you are assuming that we will
adopt exactly the behavior that Robert is questioning.
What would make more sense to me is
(1) there is still a *single* "port" parameter, which is what we use for
things like shared memory keys;
(2) listen_addresses (and the hypothetical socket_directories list)
grows the ability to specify a port number in any list element. The
primary port number parameter sets the default.
So for instance
port = 5432
listen_addresses = '*, 127.0.0.1:5433'
results in listening on *:5432 and 127.0.0.1:5433.
So you do need to create M*N sockets.
I don't really see a problem with that.
I do: first, it's a lotta sockets, and second, it's not real hard to
foresee cases where somebody actively doesn't want that cross-product.
regards, tom lane
On Mon, Jun 11, 2012 at 4:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On sön, 2012-06-10 at 17:24 -0400, Robert Haas wrote:
and also affects the naming of any UNIX sockets created.
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created....I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.How else would it work?
If I say, syntax aside, listen on "ports" 5432 and 5433, and use socket
directories /tmp and /var/run/postgresql, then a libpq-using client
would expect to be able to connect using-h /tmp -p 5432
-h /tmp -p 5433
-h /var/run/postgresql -p 5432
-h /var/run/postgresql -p 5433So you do need to create M*N sockets.
I don't really see a problem with that.
What about entries in pg_hba.conf? Will they need to be able to specify
both the directory and the port number?
--
Mike Nolan
On 06/11/2012 11:47 PM, Peter Eisentraut wrote:
On sön, 2012-06-10 at 17:24 -0400, Robert Haas wrote:
and also affects the naming of any UNIX sockets created.
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created....I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.How else would it work?
If I say, syntax aside, listen on "ports" 5432 and 5433, and use socket
directories /tmp and /var/run/postgresql, then a libpq-using client
would expect to be able to connect using-h /tmp -p 5432
-h /tmp -p 5433
-h /var/run/postgresql -p 5432
-h /var/run/postgresql -p 5433
This could be true in case all listening ports are equal, which I guess
isn't a good idea, because IIUIC the port number as a part of the socket
name is used for distinguish sockets of various postmasters in the same
directory. In that scenario every client should know which port to
connect and also which one is primary.
Regards,
Honza
On Wed, Jun 06, 2012 at 10:38:42AM -0400, Tom Lane wrote:
Florian Pflug <fgp@phlo.org> writes:
Couldn't you simply tell postgres to put it's socket in, say, /var/run, and create a symlink to that socket in the global /tmp directory?
FYI, this proposal emerged out of a discussion between Honza and
myself. "Use a symlink" was my first idea too, but on reflection
it seems like it will take less new code to support two sockets.
We already support multiple TCP sockets, so multiple Unix sockets
---------------------------------------
shouldn't be that much extra trouble.
We do? I didn't think listening on multiple interfaces meant we
listened on multiple sockets. Is there something else?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
On Wed, Jun 06, 2012 at 10:38:42AM -0400, Tom Lane wrote:
We already support multiple TCP sockets, so multiple Unix sockets
shouldn't be that much extra trouble.
We do? I didn't think listening on multiple interfaces meant we
listened on multiple sockets. Is there something else?
There's one socket for each entry in the listen_addresses list,
plus one for the Unix socket.
regards, tom lane
On Tue, Jun 12, 2012 at 05:48:58PM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Wed, Jun 06, 2012 at 10:38:42AM -0400, Tom Lane wrote:
We already support multiple TCP sockets, so multiple Unix sockets
shouldn't be that much extra trouble.We do? I didn't think listening on multiple interfaces meant we
listened on multiple sockets. Is there something else?There's one socket for each entry in the listen_addresses list,
plus one for the Unix socket.
Oh, how do we handle '*'? We pass that to the kernel, I assume. Shame
there is "wildcard" ability for unix domain sockets, which would use any
directory --- guess that wouldn't work out well. ;-)
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
On 06/10/2012 12:37 AM, Peter Eisentraut wrote:
On sön, 2012-06-10 at 00:25 +0200, Andres Freund wrote:
We already have the ability to configure the Unix socket
directory in
postgresql.conf. All you need to do is turn that into a list.
That doesn't help libpq using clients.
There is no mandate here to do anything about that.
Well, Martin Pitt/the debian package is patching exactly that. Youre
saying
that everything that needs to be done to make that easier is to make
unix_socket_dir a list. So there seems to be some disparity there ;)The Debian package doesn't need any change or assistance, really. You
can change the default location of the socket by patching
pg_config_manual.h, and that's a one-liner. The only thing that would
make that slightly easier or better is making it a configure option, but
that was intentionally decided against in the old days (not by me).
Since systemd with it's PrivateTmp feature is going to be used in more
and more distros, there will probably be a bigger need to solve
in-accessible default unix socket directory /tmp in the future.
Thus, I'd like to ask if anybody is aware of any issue connected with
simply patching pg_config_manual.h, same as Debian does it already? For
example, is there any piece of software, that simply rely on /tmp
location of the socket and doesn't use libpg for the communication?
Regards,
Honza
On 06/10/2012 03:41 PM, Robert Haas wrote:
On Sun, Jun 10, 2012 at 8:36 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut<peter_e@gmx.net> writes:
On lör, 2012-06-09 at 18:26 -0400, Tom Lane wrote:
That's not actually quite the same thing as what I suggest above.
Currently, unix_socket_directory *overrides* the compiled-in choice.
I'm suggesting that it would be better to invent a list that is *added
to* the compiled-in choice. If we think it would be best to still be
able to override that, then I'd vote for keeping unix_socket_directory
as is, and then adding a list named something like
"secondary_socket_directories". But if we just turn
unix_socket_directory into a list, I think the lack of separation
between primary and secondary directories will be confusing.By that logic, any list-valued parameter should be split into a primary
and secondary setting.Well, no: the key point here is that there will be one directory that is
special because it's the one baked into libpq. I agree that for the
purposes of the backend in isolation, we might as well just have a list.
What's less clear is whether, when considering the backend+client
ecosystem as a whole, the special status of the configure-time socket
directory ought to be reflected in the way we set up the GUCs. I have
to admit that I'm not totally sold on either approach.I think we should consider this in the context of allowing both
additional UNIX sockets and additional TCP ports. In the case of TCP
ports, it's clearly no good to turn "port" into a list, because one
port number has to be primary, since it goes into the PID file and
also affects the naming of any UNIX sockets created. If we add
secondary_socket_dirs, I think we will also need secondary_ports. One
idea might be to have one new GUC that serves both purposes:additional_sockets = '12345, /foo'
I'm sure there are other ways to skin the cat as well.
Going through the thread, I'd like to sum it up choosing approach with
less potential issues and would like to find a consensus if possible.
It seems unix_socket_directory could be turned into list and probably
renamed to unix_socket_directories, since it would be confusing if a
list value is in singular. On the other hand, we probably don't want to
specify listening ports together with additional unix sockets in one
configuration option, so it seems better to add a new configuration
option to distinguish the primary listening port from additional ports.
Regards,
Honza
On Jun13, 2012, at 15:14 , Honza Horak wrote:
Since systemd with it's PrivateTmp feature is going to be used in more and more distros, there will probably be a bigger need to solve in-accessible default unix socket directory /tmp in the future.
Thus, I'd like to ask if anybody is aware of any issue connected with simply patching pg_config_manual.h, same as Debian does it already? For example, is there any piece of software, that simply rely on /tmp location of the socket and doesn't use libpg for the communication?
I've used self-compiled postgres version on debian for years, and debian's way of doing things is major PITA in that situation. You end up having to specify that full path to the socket directory *everywhere*, because otherwise things start to break if you recompile something and it suddenly happens to use the debian-supplied libpq instead of your own.
Supporting sockets in multiple directories would solve that, once and for all.
best regards,
Florian Pflug
On ons, 2012-06-13 at 15:14 +0200, Honza Horak wrote:
Thus, I'd like to ask if anybody is aware of any issue connected with
simply patching pg_config_manual.h, same as Debian does it already?
For example, is there any piece of software, that simply rely on /tmp
location of the socket and doesn't use libpg for the communication?
If you're asking, should Red Hat/Fedora simply fix the issue locally by
patching pg_config_manual.h, then yes, that would work, see Debian, but
it has its annoyances, especially with additional software compiled from
source, or third-party binary installers.
On mån, 2012-06-11 at 18:07 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
On sön, 2012-06-10 at 17:24 -0400, Robert Haas wrote:
Why would that matter? If you configure M ports and N Unix socket
locations, you get M*N actual sockets created....I *seriously* doubt that this is the behavior anyone wants.
Creating M sockets per directory seems patently silly.How else would it work?
If I say, syntax aside, listen on "ports" 5432 and 5433, and use socket
directories /tmp and /var/run/postgresql, then a libpq-using client
would expect to be able to connect usingThis argument seems quite circular to me: you are assuming that we will
adopt exactly the behavior that Robert is questioning.What would make more sense to me is
(1) there is still a *single* "port" parameter, which is what we use for
things like shared memory keys;(2) listen_addresses (and the hypothetical socket_directories list)
grows the ability to specify a port number in any list element. The
primary port number parameter sets the default.So for instance
port = 5432
listen_addresses = '*, 127.0.0.1:5433'results in listening on *:5432 and 127.0.0.1:5433.
So you do need to create M*N sockets.
I don't really see a problem with that.I do: first, it's a lotta sockets, and second, it's not real hard to
foresee cases where somebody actively doesn't want that cross-product.
Well, it's fine if we provide ways not to have the cross-product, but
there should also be an easy way to get it. I can easily see cases in
systems I have administered where I would have liked to use two unix
sockets, two IP sockets, and two ports. Maybe I actually would have
needed only 7 out of those 8 sockets, but it's far easier to configure,
document, and explain if I just set up all 8 of them.
On mån, 2012-06-11 at 18:45 -0500, Michael Nolan wrote:
What about entries in pg_hba.conf? Will they need to be able to specify
both the directory and the port number?
I think the philosophy behind pg_hba.conf is that you distinguish client
*systems*. So one client might be Kerberos-capable, or one client might
be Windows and use SSPI. For that, it doesn't matter which of multiple
ports or local sockets it uses. So I don't think there are any changes
needed in this area.
On tis, 2012-06-12 at 14:47 +0200, Honza Horak wrote:
This could be true in case all listening ports are equal, which I
guess isn't a good idea, because IIUIC the port number as a part of
the socket name is used for distinguish sockets of various postmasters
in the same directory. In that scenario every client should know which
port to connect and also which one is primary.
Why? The client won't care which port is primary or secondary or
whatever as long as it reaches the server.
On ons, 2012-06-13 at 15:25 +0200, Honza Horak wrote:
It seems unix_socket_directory could be turned into list and probably
renamed to unix_socket_directories, since it would be confusing if a
list value is in singular.
Well, it would also be annoying to rename the parameter name for a
marginal change in functionality.
On the other hand, we probably don't want to specify listening ports
together with additional unix sockets in one
configuration option, so it seems better to add a new configuration
option to distinguish the primary listening port from additional
ports.
I think it would be wiser if you left the port business out of this.
There are more issues hidden in there than you need to deal with.
On Thu, Jun 14, 2012 at 01:31:31AM +0300, Peter Eisentraut wrote:
On ons, 2012-06-13 at 15:25 +0200, Honza Horak wrote:
It seems unix_socket_directory could be turned into list and probably
renamed to unix_socket_directories, since it would be confusing if a
list value is in singular.Well, it would also be annoying to rename the parameter name for a
marginal change in functionality.
We often rename admin-only config variables for clarity, and this seems
to be such a case.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Peter Eisentraut <peter_e@gmx.net> writes:
On mån, 2012-06-11 at 18:07 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
So you do need to create M*N sockets.
I don't really see a problem with that.
I do: first, it's a lotta sockets, and second, it's not real hard to
foresee cases where somebody actively doesn't want that cross-product.
Well, it's fine if we provide ways not to have the cross-product, but
there should also be an easy way to get it. I can easily see cases in
systems I have administered where I would have liked to use two unix
sockets, two IP sockets, and two ports. Maybe I actually would have
needed only 7 out of those 8 sockets, but it's far easier to configure,
document, and explain if I just set up all 8 of them.
Allow me to doubt that people are going to need cross-product socket
sets that are so large that it's painful to enumerate all the cases.
I can believe your 4x2 example, but not ones that are much bigger than
that.
regards, tom lane
On Thu, Jun 14, 2012 at 12:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
On mån, 2012-06-11 at 18:07 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
So you do need to create M*N sockets.
I don't really see a problem with that.I do: first, it's a lotta sockets, and second, it's not real hard to
foresee cases where somebody actively doesn't want that cross-product.Well, it's fine if we provide ways not to have the cross-product, but
there should also be an easy way to get it. I can easily see cases in
systems I have administered where I would have liked to use two unix
sockets, two IP sockets, and two ports. Maybe I actually would have
needed only 7 out of those 8 sockets, but it's far easier to configure,
document, and explain if I just set up all 8 of them.Allow me to doubt that people are going to need cross-product socket
sets that are so large that it's painful to enumerate all the cases.
I can believe your 4x2 example, but not ones that are much bigger than
that.
Same here. I can't really understand why someone would want to have,
say, six socket directories with four completely interchangeable
sockets in each one. At any rate I have no problem with allowing it,
but I think it's marginal enough that we can sanely require that a
system admin who needs that has to list out all 24 sockets explicitly.
Maybe:
listen_addresses = { host | host:port | * | *:port } [, ...]
unix_socket_directory = { directory | directory:port ] [,...]
...except that colon is a valid character in a directory name. Not
sure what to do about that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
Maybe:
listen_addresses = { host | host:port | * | *:port } [, ...]
unix_socket_directory = { directory | directory:port ] [,...]
...except that colon is a valid character in a directory name. Not
sure what to do about that.
Do we need to do anything? There are no standard scenarios in which a
colon would appear in such paths, and I don't see why we need to cater
for it. (Remember that Windows doesn't enter into this ...)
regards, tom lane
On Thu, Jun 14, 2012 at 1:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Maybe:
listen_addresses = { host | host:port | * | *:port } [, ...]
unix_socket_directory = { directory | directory:port ] [,...]...except that colon is a valid character in a directory name. Not
sure what to do about that.Do we need to do anything? There are no standard scenarios in which a
colon would appear in such paths, and I don't see why we need to cater
for it. (Remember that Windows doesn't enter into this ...)
True. And, we should be able to design the parsing algorithm so that
they can fix it by adding an otherwise-redundant port specification -
i.e. if you want to put the socket in a directory called /me:1, then
write /me:1:5432
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 06/13/2012 03:25 PM, Honza Horak wrote:
Going through the thread, I'd like to sum it up choosing approach with
less potential issues and would like to find a consensus if possible.It seems unix_socket_directory could be turned into list and probably
renamed to unix_socket_directories, since it would be confusing if a
list value is in singular. On the other hand, we probably don't want to
specify listening ports together with additional unix sockets in one
configuration option, so it seems better to add a new configuration
option to distinguish the primary listening port from additional ports.Regards,
Honza
A draft patch is attached. It renames unix_socket_directory to
unix_socket_directories and allows to use directory:port to be able to
create more sockets in one directory with different port number in the
socket name.
Regards,
Honza
Attachments:
multiple-socket-dirs.patchtext/x-patch; name=multiple-socket-dirs.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cfdb33a..679c40a 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -838,7 +838,7 @@ omicron bryanh guest1
<varname>unix_socket_permissions</varname> (and possibly
<varname>unix_socket_group</varname>) configuration parameters as
described in <xref linkend="runtime-config-connection">. Or you
- could set the <varname>unix_socket_directory</varname>
+ could set the <varname>unix_socket_directories</varname>
configuration parameter to place the socket file in a suitably
restricted directory.
</para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 074afee..7634682 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -453,17 +453,23 @@ SET ENABLE_SEQSCAN TO OFF;
</listitem>
</varlistentry>
- <varlistentry id="guc-unix-socket-directory" xreflabel="unix_socket_directory">
- <term><varname>unix_socket_directory</varname> (<type>string</type>)</term>
+ <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories">
+ <term><varname>unix_socket_directories</varname> (<type>string</type>)</term>
<indexterm>
- <primary><varname>unix_socket_directory</> configuration parameter</primary>
+ <primary><varname>unix_socket_directories</> configuration parameter</primary>
</indexterm>
<listitem>
<para>
- Specifies the directory of the Unix-domain socket on which the
+ Specifies the directories of the Unix-domain sockets on which the
server is to listen for
connections from client applications. The default is normally
<filename>/tmp</filename>, but can be changed at build time.
+ Directories are separated by ',' and additional <replaceable>port</>
+ number can be set, separated from directory by ':'. Port number will
+ only be used as a part of the socket file name. For example,
+ <literal>'/var/run, /tmp:5431'</literal> would create socket files
+ <literal>/var/run/.s.PGSQL.5432</literal> and
+ <literal>/tmp/.s.PGSQL.5431</literal>.
This parameter can only be set at server start.
</para>
@@ -472,7 +478,7 @@ SET ENABLE_SEQSCAN TO OFF;
<literal>.s.PGSQL.<replaceable>nnnn</></literal> where
<replaceable>nnnn</> is the server's port number, an ordinary file
named <literal>.s.PGSQL.<replaceable>nnnn</>.lock</literal> will be
- created in the <varname>unix_socket_directory</> directory. Neither
+ created in the <varname>unix_socket_directories</> directories. Neither
file should ever be removed manually.
</para>
@@ -6593,7 +6599,7 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
</row>
<row>
<entry><option>-k <replaceable>x</replaceable></option></entry>
- <entry><literal>unix_socket_directory = <replaceable>x</replaceable></></entry>
+ <entry><literal>unix_socket_directories = <replaceable>x</replaceable></></entry>
</row>
<row>
<entry><option>-l</option></entry>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 7ba18f0..6c74844 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1784,7 +1784,7 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
<para>
The simplest way to prevent spoofing for <literal>local</>
connections is to use a Unix domain socket directory (<xref
- linkend="guc-unix-socket-directory">) that has write permission only
+ linkend="guc-unix-socket-directories">) that has write permission only
for a trusted local user. This prevents a malicious user from creating
their own socket file in that directory. If you are concerned that
some applications might still reference <filename>/tmp</> for the
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index e3ae92d..72505e3 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -349,7 +349,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
/* If standalone, create lockfile for data directory */
if (!IsUnderPostmaster)
- CreateDataDirLockFile(false);
+ CreateDataDirLockFile(false, NULL);
SetProcessingMode(BootstrapProcessing);
IgnoreSystemIndexes = true;
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 5272811..cf1e157 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -103,8 +103,8 @@ int Unix_socket_permissions;
char *Unix_socket_group;
-/* Where the Unix socket file is */
-static char sock_path[MAXPGPATH];
+/* Where the Unix socket files are */
+static List *sock_paths = NIL;
/*
@@ -140,8 +140,8 @@ static int internal_flush(void);
static void pq_set_nonblocking(bool nonblocking);
#ifdef HAVE_UNIX_SOCKETS
-static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName);
-static int Setup_AF_UNIX(void);
+static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName, char **sock_path);
+static int Setup_AF_UNIX(char *sock_path);
#endif /* HAVE_UNIX_SOCKETS */
@@ -234,14 +234,23 @@ pq_close(int code, Datum arg)
/* StreamDoUnlink()
* Shutdown routine for backend connection
- * If a Unix socket is used for communication, explicitly close it.
+ * If any Unix sockets are used for communication, explicitly close them.
*/
#ifdef HAVE_UNIX_SOCKETS
static void
StreamDoUnlink(int code, Datum arg)
{
- Assert(sock_path[0]);
- unlink(sock_path);
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
+ {
+ cursocket = (char *) lfirst(l);
+ unlink(cursocket);
+ }
+ list_free(sock_paths);
+ sock_paths = NIL;
}
#endif /* HAVE_UNIX_SOCKETS */
@@ -286,10 +295,9 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (family == AF_UNIX)
{
- /* Lock_AF_UNIX will also fill in sock_path. */
- if (Lock_AF_UNIX(portNumber, unixSocketName) != STATUS_OK)
+ /* Lock_AF_UNIX will also fill in service. */
+ if (Lock_AF_UNIX(portNumber, unixSocketName, &service) != STATUS_OK)
return STATUS_ERROR;
- service = sock_path;
}
else
#endif /* HAVE_UNIX_SOCKETS */
@@ -432,7 +440,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
(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, sock_path) :
+ (int) portNumber, service) :
errhint("Is another postmaster already running on port %d?"
" If not, wait a few seconds and retry.",
(int) portNumber)));
@@ -443,7 +451,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (addr->ai_family == AF_UNIX)
{
- if (Setup_AF_UNIX() != STATUS_OK)
+ if (Setup_AF_UNIX(service) != STATUS_OK)
{
closesocket(fd);
break;
@@ -490,9 +498,13 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
* Lock_AF_UNIX -- configure unix socket file path
*/
static int
-Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
+Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName, char **sock_path)
{
- UNIXSOCK_PATH(sock_path, portNumber, unixSocketName);
+ char new_sock[MAXPGPATH];
+
+ UNIXSOCK_PATH(new_sock, portNumber, unixSocketName);
+
+ *sock_path = pstrdup(new_sock);
/*
* Grab an interlock file associated with the socket file.
@@ -502,13 +514,14 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* more portable, and second, it lets us remove any pre-existing socket
* file without race conditions.
*/
- CreateSocketLockFile(sock_path, true);
+ CreateSocketLockFile(*sock_path, true, unixSocketName);
/*
* Once we have the interlock, we can safely delete any pre-existing
* socket file to avoid failure at bind() time.
*/
- unlink(sock_path);
+ unlink(*sock_path);
+ sock_paths = lappend(sock_paths, *sock_path);
return STATUS_OK;
}
@@ -518,7 +531,7 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* Setup_AF_UNIX -- configure unix socket permissions
*/
static int
-Setup_AF_UNIX(void)
+Setup_AF_UNIX(char *sock_path)
{
/* Arrange to unlink the socket file at exit */
on_proc_exit(StreamDoUnlink, 0);
@@ -707,17 +720,21 @@ StreamClose(pgsocket sock)
* TouchSocketFile -- mark socket file as recently accessed
*
* This routine should be called every so often to ensure that the socket
- * file has a recent mod date (ordinary operations on sockets usually won't
- * change the mod date). That saves it from being removed by
+ * files have a recent mod date (ordinary operations on sockets usually won't
+ * change the mod date). That saves them from being removed by
* overenthusiastic /tmp-directory-cleaner daemons. (Another reason we should
- * never have put the socket file in /tmp...)
+ * never have put the socket files in /tmp...)
*/
void
TouchSocketFile(void)
{
- /* Do nothing if we did not create a socket... */
- if (sock_path[0] != '\0')
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
{
+ cursocket = (char *) lfirst(l);
/*
* utime() is POSIX standard, utimes() is a common alternative. If we
* have neither, there's no way to affect the mod or access time of
@@ -726,10 +743,10 @@ TouchSocketFile(void)
* In either path, we ignore errors; there's no point in complaining.
*/
#ifdef HAVE_UTIME
- utime(sock_path, NULL);
+ utime(cursocket, NULL);
#else /* !HAVE_UTIME */
#ifdef HAVE_UTIMES
- utimes(sock_path, NULL);
+ utimes(cursocket, NULL);
#endif /* HAVE_UTIMES */
#endif /* HAVE_UTIME */
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index eeea933..f91dcdd 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -156,7 +156,7 @@ static Backend *ShmemBackendArray;
/* The socket number we are listening for connections on */
int PostPortNumber;
-char *UnixSocketDir;
+char *UnixSocketDirs;
char *ListenAddresses;
/*
@@ -499,6 +499,10 @@ PostmasterMain(int argc, char *argv[])
char *userDoption = NULL;
bool listen_addr_saved = false;
int i;
+#ifdef HAVE_UNIX_SOCKETS
+ List *socketsList;
+ char *mainSocket = NULL;
+#endif
MyProcPid = PostmasterPid = getpid();
@@ -607,7 +611,7 @@ PostmasterMain(int argc, char *argv[])
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+ SetConfigOption("unix_socket_directories", optarg, PGC_POSTMASTER, PGC_S_ARGV);
break;
case 'l':
@@ -807,6 +811,40 @@ PostmasterMain(int argc, char *argv[])
}
/*
+ * We need to parse UnixSocketDirs here, because we want only
+ * the first socket directory be used in postmaster.pid, which is done
+ * in CreateDataDirLockFile().
+ */
+#ifdef HAVE_UNIX_SOCKETS
+ if (UnixSocketDirs)
+ {
+ char *rawSocketsString;
+ ListCell *l;
+
+ /* Need a modifiable copy of UnixSocketDirs */
+ rawSocketsString = pstrdup(UnixSocketDirs);
+
+ /* Parse string into list of directories */
+ if (!SplitDirectoriesString(rawSocketsString, ',', &socketsList))
+ {
+ /* syntax error in list */
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid list syntax for \"unix_socket_directories\"")));
+ }
+
+ pfree(rawSocketsString);
+
+ if (l = list_head(socketsList))
+ mainSocket = (char *) lfirst(l);
+ else
+ mainSocket = UnixSocketDirs;
+ }
+ else
+ mainSocket = UnixSocketDirs;
+#endif
+
+ /*
* Create lockfile for data directory.
*
* We want to do this before we try to grab the input sockets, because the
@@ -815,7 +853,7 @@ PostmasterMain(int argc, char *argv[])
* For the same reason, it's best to grab the TCP socket(s) before the
* Unix socket.
*/
- CreateDataDirLockFile(true);
+ CreateDataDirLockFile(true, mainSocket);
/*
* Initialize SSL library, if specified.
@@ -862,12 +900,12 @@ PostmasterMain(int argc, char *argv[])
if (strcmp(curhost, "*") == 0)
status = StreamServerPort(AF_UNSPEC, NULL,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ UnixSocketDirs,
ListenSocket, MAXLISTEN);
else
status = StreamServerPort(AF_UNSPEC, curhost,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ UnixSocketDirs,
ListenSocket, MAXLISTEN);
if (status == STATUS_OK)
@@ -933,13 +971,89 @@ PostmasterMain(int argc, char *argv[])
#endif
#ifdef HAVE_UNIX_SOCKETS
- status = StreamServerPort(AF_UNIX, NULL,
- (unsigned short) PostPortNumber,
- UnixSocketDir,
- ListenSocket, MAXLISTEN);
- if (status != STATUS_OK)
- ereport(WARNING,
- (errmsg("could not create Unix-domain socket")));
+ /*
+ * We can specify several directories for Unix sockets to listen on,
+ * separated with ','. Socket name itself is the same in all cases.
+ */
+ if (!UnixSocketDirs)
+ {
+ status = StreamServerPort(AF_UNIX, NULL,
+ (unsigned short) PostPortNumber,
+ UnixSocketDirs,
+ ListenSocket, MAXLISTEN);
+ if (status != STATUS_OK)
+ ereport(WARNING,
+ (errmsg("could not create Unix-domain socket")));
+
+ }
+ else
+ {
+ ListCell *l;
+
+ /* We have parse string into list of directories earlier */
+ foreach(l, socketsList)
+ {
+ char *cursocket = (char *) lfirst(l);
+ char *colon;
+ char *port;
+ int64 portnum;
+ char *endptr;
+ bool bad_strtol;
+
+ /* syntax is directory or directory:port */
+ colon = strrchr(cursocket, ':');
+ if (colon != NULL)
+ {
+ port = colon + 1;
+ *colon = '\0';
+
+ /*
+ * convert port number to integer, ignoring optional trailing
+ * whitespace). strtol should ignore leading whitespace, too.
+ */
+ portnum = strtol(port, &endptr, 0);
+ bad_strtol = (errno == ERANGE);
+
+ while (isspace((unsigned char) *endptr))
+ endptr++;
+
+ if (*endptr != '\0')
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" invalid in \"unix_socket_directories\"", port)));
+
+ if (bad_strtol || portnum < 1 || portnum > 65535)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" out of range in \"unix_socket_directories\"", port)));
+ }
+ else
+ {
+ portnum = PostPortNumber;
+ }
+
+ /* only absolute paths are allowed */
+ canonicalize_path(cursocket);
+ if (!is_absolute_path(cursocket))
+ {
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("directory \"%s\" is not an absolute path", cursocket)));
+ }
+ else
+ {
+ status = StreamServerPort(AF_UNIX, NULL,
+ (unsigned short) portnum,
+ cursocket,
+ ListenSocket, MAXLISTEN);
+ if (status != STATUS_OK)
+ ereport(FATAL,
+ (errmsg("could not create secondary Unix-domain socket at \"%s\"", cursocket)));
+ }
+ }
+
+ list_free(socketsList);
+ }
#endif
/*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 51b6df5..837d0e8 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3343,7 +3343,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, ctx, gucsource);
+ SetConfigOption("unix_socket_directories", optarg, ctx, gucsource);
break;
case 'l':
@@ -3660,7 +3660,7 @@ PostgresMain(int argc, char *argv[], const char *username)
/*
* Create lockfile for data directory.
*/
- CreateDataDirLockFile(false);
+ CreateDataDirLockFile(false, NULL);
}
/* Early initialization */
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index e1b57ba..ffc7e2b 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2445,6 +2445,107 @@ SplitIdentifierString(char *rawstring, char separator,
return true;
}
+/*
+ * SplitDirectoriesString --- parse a string containing directories
+ *
+ * Inputs:
+ * rawstring: the input string; must be overwritable! On return, it's
+ * been modified to contain the separated directories.
+ * separator: the separator punctuation expected between directories
+ * (typically ',' or ';'). Whitespace may also appear around
+ * directories.
+ * Outputs:
+ * namelist: filled with a palloc'd list of pointers to directories within
+ * rawstring. Caller should list_free() this even on error return.
+ *
+ * Returns TRUE if okay, FALSE if there is a syntax error in the string.
+ *
+ * Note that an empty string is considered okay here.
+ */
+bool
+SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist)
+{
+ char *nextp = rawstring;
+ bool done = false;
+
+ *namelist = NIL;
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace */
+
+ if (*nextp == '\0')
+ return true; /* allow empty string */
+
+ /* At the top of the loop, we are at start of a new directories. */
+ do
+ {
+ char *curname;
+ char *endp;
+
+ if (*nextp == '\"')
+ {
+ /* Quoted name --- collapse quote-quote pairs */
+ curname = nextp + 1;
+ for (;;)
+ {
+ endp = strchr(nextp + 1, '\"');
+ if (endp == NULL)
+ return false; /* mismatched quotes */
+ if (endp[1] != '\"')
+ break; /* found end of quoted name */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(endp, endp + 1, strlen(endp));
+ nextp = endp;
+ }
+ /* endp now points at the terminating quote */
+ nextp = endp + 1;
+ }
+ else
+ {
+ /* Unquoted name --- extends to separator or whitespace */
+ curname = nextp;
+ while (*nextp && *nextp != separator &&
+ !isspace((unsigned char) *nextp))
+ nextp++;
+ endp = nextp;
+ if (curname == nextp)
+ return false; /* empty unquoted name not allowed */
+ }
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip trailing whitespace */
+
+ if (*nextp == separator)
+ {
+ nextp++;
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace for next */
+ /* we expect another name, so done remains false */
+ }
+ else if (*nextp == '\0')
+ done = true;
+ else
+ return false; /* invalid syntax */
+
+ /* Now safe to overwrite separator with a null */
+ *endp = '\0';
+
+ /* Truncate path if it's overlength */
+ if (strlen(curname) >= MAXPGPATH)
+ curname[MAXPGPATH-1] = '\0';
+
+ /*
+ * Finished isolating current name --- add it to list
+ */
+ *namelist = lappend(*namelist, pstrdup(curname));
+
+ /* Loop back if we didn't reach end of string */
+ } while (!done);
+
+ return true;
+}
+
/*****************************************************************************
* Comparison Functions used for bytea
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index fb376a0..704c33c 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -663,10 +663,11 @@ UnlinkLockFile(int status, Datum filename)
* filename is the name of the lockfile to create.
* amPostmaster is used to determine how to encode the output PID.
* isDDLock and refName are used to determine what error message to produce.
+ * socketPath is the path to the Unix socket we want to lock.
*/
static void
CreateLockFile(const char *filename, bool amPostmaster,
- bool isDDLock, const char *refName)
+ bool isDDLock, const char *refName, const char *socketPath)
{
int fd;
char buffer[MAXPGPATH * 2 + 256];
@@ -892,7 +893,8 @@ CreateLockFile(const char *filename, bool amPostmaster,
(long) MyStartTime,
PostPortNumber,
#ifdef HAVE_UNIX_SOCKETS
- (*UnixSocketDir != '\0') ? UnixSocketDir : DEFAULT_PGSOCKET_DIR
+ (socketPath && *socketPath != '\0') ? socketPath
+ : DEFAULT_PGSOCKET_DIR
#else
""
#endif
@@ -947,21 +949,22 @@ CreateLockFile(const char *filename, bool amPostmaster,
* helps ensure that we are locking the directory we should be.
*/
void
-CreateDataDirLockFile(bool amPostmaster)
+CreateDataDirLockFile(bool amPostmaster, const char *socketDir)
{
- CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir);
+ CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir, socketDir);
}
/*
* Create a lockfile for the specified Unix socket file.
*/
void
-CreateSocketLockFile(const char *socketfile, bool amPostmaster)
+CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir)
{
char lockfile[MAXPGPATH];
snprintf(lockfile, sizeof(lockfile), "%s.lock", socketfile);
- CreateLockFile(lockfile, amPostmaster, false, socketfile);
+ CreateLockFile(lockfile, amPostmaster, false, socketfile, socketDir);
/* Save name of lockfile for TouchSocketLockFile */
strcpy(socketLockFile, lockfile);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b756e58..bf90aff 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2894,14 +2894,15 @@ static struct config_string ConfigureNamesString[] =
},
{
- {"unix_socket_directory", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
- gettext_noop("Sets the directory where the Unix-domain socket will be created."),
- NULL,
+ {"unix_socket_directories", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop("Sets the directories where the Unix-domain socket will be created."),
+ gettext_noop("Directories are separated by \",\" and an additional "
+ "port number can be set, separated from directory by \":\"."),
GUC_SUPERUSER_ONLY
},
- &UnixSocketDir,
+ &UnixSocketDirs,
"",
- check_canonical_path, NULL, NULL
+ NULL, NULL, NULL
},
{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index fa75d00..b7f9ac9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -65,7 +65,8 @@
# Note: Increasing max_connections costs ~400 bytes of shared memory per
# connection slot, plus lock space (see max_locks_per_transaction).
#superuser_reserved_connections = 3 # (change requires restart)
-#unix_socket_directory = '' # (change requires restart)
+#unix_socket_directories = '' # comma-separated list of directories
+ # (change requires restart)
#unix_socket_group = '' # (change requires restart)
#unix_socket_permissions = 0777 # begin with 0 to use octal notation
# (change requires restart)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index d7b8367..a407684 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -521,14 +521,27 @@ test_postmaster_connection(bool do_checkpoint)
hostaddr = optlines[LOCK_FILE_LINE_LISTEN_ADDR - 1];
/*
- * While unix_socket_directory can accept relative
+ * While unix_socket_directories can accept relative
* directories, libpq's host parameter must have a
* leading slash to indicate a socket directory. So,
* ignore sockdir if it's relative, and try to use TCP
* instead.
*/
if (sockdir[0] == '/')
- strlcpy(host_str, sockdir, sizeof(host_str));
+ {
+ char *rawstring;
+ char *comma;
+
+ /* Need a modifiable copy of UnixSocketDirs */
+ rawstring = xstrdup(sockdir);
+
+ /* We want only the first socket, in case more are defined */
+ if ((comma = strchr(rawstring, ',')) != NULL)
+ *comma = '\0';
+
+ strlcpy(host_str, rawstring, sizeof(host_str));
+ free(rawstring);
+ }
else
strlcpy(host_str, hostaddr, sizeof(host_str));
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b186eed..40802cf 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -398,8 +398,9 @@ extern char *local_preload_libraries_string;
#define LOCK_FILE_LINE_LISTEN_ADDR 6
#define LOCK_FILE_LINE_SHMEM_KEY 7
-extern void CreateDataDirLockFile(bool amPostmaster);
-extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster);
+extern void CreateDataDirLockFile(bool amPostmaster, const char *socketDir);
+extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir);
extern void TouchSocketLockFile(void);
extern void AddToDataDirLockFile(int target_line, const char *str);
extern void ValidatePgVersion(const char *path);
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 683ce3c..f19a3f8 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -19,7 +19,7 @@ extern int ReservedBackends;
extern int PostPortNumber;
extern int Unix_socket_permissions;
extern char *Unix_socket_group;
-extern char *UnixSocketDir;
+extern char *UnixSocketDirs;
extern char *ListenAddresses;
extern bool ClientAuthInProgress;
extern int PreAuthDelay;
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index d1e8370..65201fc 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -752,6 +752,8 @@ extern int varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid);
extern List *textToQualifiedNameList(text *textval);
extern bool SplitIdentifierString(char *rawstring, char separator,
List **namelist);
+extern bool SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist);
extern Datum replace_text(PG_FUNCTION_ARGS);
extern text *replace_text_regexp(text *src_text, void *regexp,
text *replace_text, bool glob);
On 06/13/2012 03:25 PM, Honza Horak wrote:
A draft patch is attached. It renames unix_socket_directory to
unix_socket_directories and allows to use directory:port to be able
to
create more sockets in one directory with different port number in
the
socket name.
I realized the patch has some difficulties -- namely the socket path in the data dir lock file, which currently uses one port for socket and the same for interface. So to allow users to use arbitrary port for all unix sockets, we'd need to add another line only for unix socket, which doesn't apply for other platforms. Or we could just say that the first socket will allways use the default port (PostPortNumber), which is a solution I prefer currently, but will be glad for any other opinion. This is also why there is still un-necesary string splitting in pg_ctl.c, which will be removed after the issue above is solved.
Regards,
Honza
On 06/15/2012 05:40 PM, Honza Horak wrote:
I realized the patch has some difficulties -- namely the socket path in the data dir lock file, which currently uses one port for socket and the same for interface. So to allow users to use arbitrary port for all unix sockets, we'd need to add another line only for unix socket, which doesn't apply for other platforms. Or we could just say that the first socket will allways use the default port (PostPortNumber), which is a solution I prefer currently, but will be glad for any other opinion. This is also why there is still un-necesary string splitting in pg_ctl.c, which will be removed after the issue above is solved.
This is an enhanced patch, which forbids using a port number in the
first socket directory entry.
Honza
Attachments:
multiple-socket-dirs.patchtext/x-patch; name=multiple-socket-dirs.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cfdb33a..679c40a 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -838,7 +838,7 @@ omicron bryanh guest1
<varname>unix_socket_permissions</varname> (and possibly
<varname>unix_socket_group</varname>) configuration parameters as
described in <xref linkend="runtime-config-connection">. Or you
- could set the <varname>unix_socket_directory</varname>
+ could set the <varname>unix_socket_directories</varname>
configuration parameter to place the socket file in a suitably
restricted directory.
</para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 074afee..d1727f8 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -453,17 +453,26 @@ SET ENABLE_SEQSCAN TO OFF;
</listitem>
</varlistentry>
- <varlistentry id="guc-unix-socket-directory" xreflabel="unix_socket_directory">
- <term><varname>unix_socket_directory</varname> (<type>string</type>)</term>
+ <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories">
+ <term><varname>unix_socket_directories</varname> (<type>string</type>)</term>
<indexterm>
- <primary><varname>unix_socket_directory</> configuration parameter</primary>
+ <primary><varname>unix_socket_directories</> configuration parameter</primary>
</indexterm>
<listitem>
<para>
- Specifies the directory of the Unix-domain socket on which the
+ Specifies the directories of the Unix-domain sockets on which the
server is to listen for
connections from client applications. The default is normally
<filename>/tmp</filename>, but can be changed at build time.
+ Directories are separated by ',' and additional <replaceable>port</>
+ number can be set, separated from directory by ':'. Port number will
+ only be used as a part of the socket file name. For example,
+ <literal>'/var/run, /tmp:5431'</literal> would create socket files
+ <literal>/var/run/.s.PGSQL.5432</literal> and
+ <literal>/tmp/.s.PGSQL.5431</literal>.
+ It is not possible to define <replaceable>port</> number in the first
+ entry. If set, it will be ignored and default <varname>port</>
+ will be used.
This parameter can only be set at server start.
</para>
@@ -472,7 +481,7 @@ SET ENABLE_SEQSCAN TO OFF;
<literal>.s.PGSQL.<replaceable>nnnn</></literal> where
<replaceable>nnnn</> is the server's port number, an ordinary file
named <literal>.s.PGSQL.<replaceable>nnnn</>.lock</literal> will be
- created in the <varname>unix_socket_directory</> directory. Neither
+ created in the <varname>unix_socket_directories</> directories. Neither
file should ever be removed manually.
</para>
@@ -6593,7 +6602,7 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
</row>
<row>
<entry><option>-k <replaceable>x</replaceable></option></entry>
- <entry><literal>unix_socket_directory = <replaceable>x</replaceable></></entry>
+ <entry><literal>unix_socket_directories = <replaceable>x</replaceable></></entry>
</row>
<row>
<entry><option>-l</option></entry>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 7ba18f0..6c74844 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1784,7 +1784,7 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
<para>
The simplest way to prevent spoofing for <literal>local</>
connections is to use a Unix domain socket directory (<xref
- linkend="guc-unix-socket-directory">) that has write permission only
+ linkend="guc-unix-socket-directories">) that has write permission only
for a trusted local user. This prevents a malicious user from creating
their own socket file in that directory. If you are concerned that
some applications might still reference <filename>/tmp</> for the
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index e3ae92d..50d4167 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -31,6 +31,7 @@
#include "postmaster/bgwriter.h"
#include "postmaster/startup.h"
#include "postmaster/walwriter.h"
+#include "postmaster/postmaster.h"
#include "replication/walreceiver.h"
#include "storage/bufmgr.h"
#include "storage/ipc.h"
@@ -349,7 +350,20 @@ AuxiliaryProcessMain(int argc, char *argv[])
/* If standalone, create lockfile for data directory */
if (!IsUnderPostmaster)
- CreateDataDirLockFile(false);
+ {
+#ifdef HAVE_UNIX_SOCKETS
+ List *socketsList;
+ char *mainSocket = NULL;
+#endif
+ /*
+ * We need to parse UnixSocketDirs here, because we want the first
+ * socket directory, which will be used in postmaster.pid.
+ */
+#ifdef HAVE_UNIX_SOCKETS
+ SplitUnixDirectories(UnixSocketDirs, &socketsList, &mainSocket);
+#endif
+ CreateDataDirLockFile(false, mainSocket);
+ }
SetProcessingMode(BootstrapProcessing);
IgnoreSystemIndexes = true;
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 5272811..cf1e157 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -103,8 +103,8 @@ int Unix_socket_permissions;
char *Unix_socket_group;
-/* Where the Unix socket file is */
-static char sock_path[MAXPGPATH];
+/* Where the Unix socket files are */
+static List *sock_paths = NIL;
/*
@@ -140,8 +140,8 @@ static int internal_flush(void);
static void pq_set_nonblocking(bool nonblocking);
#ifdef HAVE_UNIX_SOCKETS
-static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName);
-static int Setup_AF_UNIX(void);
+static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName, char **sock_path);
+static int Setup_AF_UNIX(char *sock_path);
#endif /* HAVE_UNIX_SOCKETS */
@@ -234,14 +234,23 @@ pq_close(int code, Datum arg)
/* StreamDoUnlink()
* Shutdown routine for backend connection
- * If a Unix socket is used for communication, explicitly close it.
+ * If any Unix sockets are used for communication, explicitly close them.
*/
#ifdef HAVE_UNIX_SOCKETS
static void
StreamDoUnlink(int code, Datum arg)
{
- Assert(sock_path[0]);
- unlink(sock_path);
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
+ {
+ cursocket = (char *) lfirst(l);
+ unlink(cursocket);
+ }
+ list_free(sock_paths);
+ sock_paths = NIL;
}
#endif /* HAVE_UNIX_SOCKETS */
@@ -286,10 +295,9 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (family == AF_UNIX)
{
- /* Lock_AF_UNIX will also fill in sock_path. */
- if (Lock_AF_UNIX(portNumber, unixSocketName) != STATUS_OK)
+ /* Lock_AF_UNIX will also fill in service. */
+ if (Lock_AF_UNIX(portNumber, unixSocketName, &service) != STATUS_OK)
return STATUS_ERROR;
- service = sock_path;
}
else
#endif /* HAVE_UNIX_SOCKETS */
@@ -432,7 +440,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
(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, sock_path) :
+ (int) portNumber, service) :
errhint("Is another postmaster already running on port %d?"
" If not, wait a few seconds and retry.",
(int) portNumber)));
@@ -443,7 +451,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (addr->ai_family == AF_UNIX)
{
- if (Setup_AF_UNIX() != STATUS_OK)
+ if (Setup_AF_UNIX(service) != STATUS_OK)
{
closesocket(fd);
break;
@@ -490,9 +498,13 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
* Lock_AF_UNIX -- configure unix socket file path
*/
static int
-Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
+Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName, char **sock_path)
{
- UNIXSOCK_PATH(sock_path, portNumber, unixSocketName);
+ char new_sock[MAXPGPATH];
+
+ UNIXSOCK_PATH(new_sock, portNumber, unixSocketName);
+
+ *sock_path = pstrdup(new_sock);
/*
* Grab an interlock file associated with the socket file.
@@ -502,13 +514,14 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* more portable, and second, it lets us remove any pre-existing socket
* file without race conditions.
*/
- CreateSocketLockFile(sock_path, true);
+ CreateSocketLockFile(*sock_path, true, unixSocketName);
/*
* Once we have the interlock, we can safely delete any pre-existing
* socket file to avoid failure at bind() time.
*/
- unlink(sock_path);
+ unlink(*sock_path);
+ sock_paths = lappend(sock_paths, *sock_path);
return STATUS_OK;
}
@@ -518,7 +531,7 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* Setup_AF_UNIX -- configure unix socket permissions
*/
static int
-Setup_AF_UNIX(void)
+Setup_AF_UNIX(char *sock_path)
{
/* Arrange to unlink the socket file at exit */
on_proc_exit(StreamDoUnlink, 0);
@@ -707,17 +720,21 @@ StreamClose(pgsocket sock)
* TouchSocketFile -- mark socket file as recently accessed
*
* This routine should be called every so often to ensure that the socket
- * file has a recent mod date (ordinary operations on sockets usually won't
- * change the mod date). That saves it from being removed by
+ * files have a recent mod date (ordinary operations on sockets usually won't
+ * change the mod date). That saves them from being removed by
* overenthusiastic /tmp-directory-cleaner daemons. (Another reason we should
- * never have put the socket file in /tmp...)
+ * never have put the socket files in /tmp...)
*/
void
TouchSocketFile(void)
{
- /* Do nothing if we did not create a socket... */
- if (sock_path[0] != '\0')
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
{
+ cursocket = (char *) lfirst(l);
/*
* utime() is POSIX standard, utimes() is a common alternative. If we
* have neither, there's no way to affect the mod or access time of
@@ -726,10 +743,10 @@ TouchSocketFile(void)
* In either path, we ignore errors; there's no point in complaining.
*/
#ifdef HAVE_UTIME
- utime(sock_path, NULL);
+ utime(cursocket, NULL);
#else /* !HAVE_UTIME */
#ifdef HAVE_UTIMES
- utimes(sock_path, NULL);
+ utimes(cursocket, NULL);
#endif /* HAVE_UTIMES */
#endif /* HAVE_UTIME */
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index eeea933..0cc3521 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -156,7 +156,7 @@ static Backend *ShmemBackendArray;
/* The socket number we are listening for connections on */
int PostPortNumber;
-char *UnixSocketDir;
+char *UnixSocketDirs;
char *ListenAddresses;
/*
@@ -499,6 +499,10 @@ PostmasterMain(int argc, char *argv[])
char *userDoption = NULL;
bool listen_addr_saved = false;
int i;
+#ifdef HAVE_UNIX_SOCKETS
+ List *socketsList;
+ char *mainSocket = NULL;
+#endif
MyProcPid = PostmasterPid = getpid();
@@ -607,7 +611,7 @@ PostmasterMain(int argc, char *argv[])
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+ SetConfigOption("unix_socket_directories", optarg, PGC_POSTMASTER, PGC_S_ARGV);
break;
case 'l':
@@ -807,6 +811,15 @@ PostmasterMain(int argc, char *argv[])
}
/*
+ * We need to parse UnixSocketDirs here, because we want only
+ * the first socket directory be used in postmaster.pid, which is done
+ * in CreateDataDirLockFile().
+ */
+#ifdef HAVE_UNIX_SOCKETS
+ SplitUnixDirectories(UnixSocketDirs, &socketsList, &mainSocket);
+#endif
+
+ /*
* Create lockfile for data directory.
*
* We want to do this before we try to grab the input sockets, because the
@@ -815,7 +828,7 @@ PostmasterMain(int argc, char *argv[])
* For the same reason, it's best to grab the TCP socket(s) before the
* Unix socket.
*/
- CreateDataDirLockFile(true);
+ CreateDataDirLockFile(true, mainSocket);
/*
* Initialize SSL library, if specified.
@@ -862,12 +875,12 @@ PostmasterMain(int argc, char *argv[])
if (strcmp(curhost, "*") == 0)
status = StreamServerPort(AF_UNSPEC, NULL,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ UnixSocketDirs,
ListenSocket, MAXLISTEN);
else
status = StreamServerPort(AF_UNSPEC, curhost,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ UnixSocketDirs,
ListenSocket, MAXLISTEN);
if (status == STATUS_OK)
@@ -933,13 +946,92 @@ PostmasterMain(int argc, char *argv[])
#endif
#ifdef HAVE_UNIX_SOCKETS
- status = StreamServerPort(AF_UNIX, NULL,
- (unsigned short) PostPortNumber,
- UnixSocketDir,
- ListenSocket, MAXLISTEN);
- if (status != STATUS_OK)
- ereport(WARNING,
- (errmsg("could not create Unix-domain socket")));
+ /*
+ * We can specify several directories for Unix sockets to listen on,
+ * separated with ','. Socket name itself is the same in all cases.
+ */
+ if (!UnixSocketDirs)
+ {
+ status = StreamServerPort(AF_UNIX, NULL,
+ (unsigned short) PostPortNumber,
+ UnixSocketDirs,
+ ListenSocket, MAXLISTEN);
+ if (status != STATUS_OK)
+ ereport(WARNING,
+ (errmsg("could not create Unix-domain socket")));
+
+ }
+ else
+ {
+ ListCell *l;
+ bool first = TRUE;
+
+ /* We have parse string into list of directories earlier */
+ foreach(l, socketsList)
+ {
+ char *cursocket = (char *) lfirst(l);
+ char *colon;
+ char *port;
+ int64 portnum;
+ char *endptr;
+ bool bad_strtol;
+
+ /* syntax is directory or directory:port, except the first one */
+ if ((colon = strrchr(cursocket, ':')) != NULL)
+ {
+ port = colon + 1;
+ *colon = '\0';
+
+ /*
+ * convert port number to integer, ignoring optional trailing
+ * whitespace). strtol should ignore leading whitespace, too.
+ */
+ portnum = strtol(port, &endptr, 0);
+ bad_strtol = (errno == ERANGE);
+
+ while (isspace((unsigned char) *endptr))
+ endptr++;
+
+ if (*endptr != '\0')
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" invalid in \"unix_socket_directories\"", port)));
+
+ if (bad_strtol || portnum < 1 || portnum > 65535)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("port number \"%s\" out of range in \"unix_socket_directories\"", port)));
+ if (first)
+ portnum = PostPortNumber;
+ }
+ else
+ {
+ portnum = PostPortNumber;
+ }
+ first = FALSE;
+
+ /* only absolute paths are allowed */
+ canonicalize_path(cursocket);
+ if (!is_absolute_path(cursocket))
+ {
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("directory \"%s\" is not an absolute path", cursocket)));
+ }
+ else
+ {
+ status = StreamServerPort(AF_UNIX, NULL,
+ (unsigned short) portnum,
+ cursocket,
+ ListenSocket, MAXLISTEN);
+ if (status != STATUS_OK)
+ ereport(FATAL,
+ (errmsg("could not create secondary Unix-domain socket at \"%s\"", cursocket)));
+ }
+ }
+
+ list_free(socketsList);
+ }
#endif
/*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 51b6df5..22c8228 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3343,7 +3343,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, ctx, gucsource);
+ SetConfigOption("unix_socket_directories", optarg, ctx, gucsource);
break;
case 'l':
@@ -3647,6 +3647,10 @@ PostgresMain(int argc, char *argv[], const char *username)
if (!IsUnderPostmaster)
{
+#ifdef HAVE_UNIX_SOCKETS
+ List *socketsList;
+ char *mainSocket = NULL;
+#endif
/*
* Validate we have been given a reasonable-looking DataDir (if under
* postmaster, assume postmaster did this already).
@@ -3658,9 +3662,16 @@ PostgresMain(int argc, char *argv[], const char *username)
ChangeToDataDir();
/*
+ * We need to parse UnixSocketDirs here, because we want the first
+ * socket directory, which will be used in postmaster.pid.
+ */
+#ifdef HAVE_UNIX_SOCKETS
+ SplitUnixDirectories(UnixSocketDirs, &socketsList, &mainSocket);
+#endif
+ /*
* Create lockfile for data directory.
*/
- CreateDataDirLockFile(false);
+ CreateDataDirLockFile(false, mainSocket);
}
/* Early initialization */
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index e1b57ba..466d738 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2445,6 +2445,155 @@ SplitIdentifierString(char *rawstring, char separator,
return true;
}
+/*
+ * SplitDirectoriesString --- parse a string containing directories
+ *
+ * Inputs:
+ * rawstring: the input string; must be overwritable! On return, it's
+ * been modified to contain the separated directories.
+ * separator: the separator punctuation expected between directories
+ * (typically ',' or ';'). Whitespace may also appear around
+ * directories.
+ * Outputs:
+ * namelist: filled with a palloc'd list of pointers to directories within
+ * rawstring. Caller should list_free() this even on error return.
+ *
+ * Returns TRUE if okay, FALSE if there is a syntax error in the string.
+ *
+ * Note that an empty string is considered okay here.
+ */
+bool
+SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist)
+{
+ char *nextp = rawstring;
+ bool done = false;
+
+ *namelist = NIL;
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace */
+
+ if (*nextp == '\0')
+ return true; /* allow empty string */
+
+ /* At the top of the loop, we are at start of a new directories. */
+ do
+ {
+ char *curname;
+ char *endp;
+
+ if (*nextp == '\"')
+ {
+ /* Quoted name --- collapse quote-quote pairs */
+ curname = nextp + 1;
+ for (;;)
+ {
+ endp = strchr(nextp + 1, '\"');
+ if (endp == NULL)
+ return false; /* mismatched quotes */
+ if (endp[1] != '\"')
+ break; /* found end of quoted name */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(endp, endp + 1, strlen(endp));
+ nextp = endp;
+ }
+ /* endp now points at the terminating quote */
+ nextp = endp + 1;
+ }
+ else
+ {
+ /* Unquoted name --- extends to separator or whitespace */
+ curname = nextp;
+ while (*nextp && *nextp != separator &&
+ !isspace((unsigned char) *nextp))
+ nextp++;
+ endp = nextp;
+ if (curname == nextp)
+ return false; /* empty unquoted name not allowed */
+ }
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip trailing whitespace */
+
+ if (*nextp == separator)
+ {
+ nextp++;
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace for next */
+ /* we expect another name, so done remains false */
+ }
+ else if (*nextp == '\0')
+ done = true;
+ else
+ return false; /* invalid syntax */
+
+ /* Now safe to overwrite separator with a null */
+ *endp = '\0';
+
+ /* Truncate path if it's overlength */
+ if (strlen(curname) >= MAXPGPATH)
+ curname[MAXPGPATH-1] = '\0';
+
+ /*
+ * Finished isolating current name --- add it to list
+ */
+ *namelist = lappend(*namelist, pstrdup(curname));
+
+ /* Loop back if we didn't reach end of string */
+ } while (!done);
+
+ return true;
+}
+
+/*
+ * parse UnixSocketDirs as a list of directory[:port], return the list
+ * in socketsList and store the first item into mainSocket.
+ */
+void
+SplitUnixDirectories(char *unixSocketDirs, List **socketsList,
+ char **mainSocket)
+{
+#ifdef HAVE_UNIX_SOCKETS
+ if (unixSocketDirs)
+ {
+ char *rawSocketsString;
+ ListCell *l;
+
+ /* Need a modifiable copy of UnixSocketDirs */
+ rawSocketsString = pstrdup(unixSocketDirs);
+
+ /* Parse string into list of directories */
+ if (!SplitDirectoriesString(rawSocketsString, ',', socketsList))
+ {
+ /* syntax error in list */
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid list syntax for \"unix_socket_directories\"")));
+ }
+
+ pfree(rawSocketsString);
+
+ if (l = list_head(*socketsList))
+ {
+ char *colon;
+
+ *mainSocket = (char *) lfirst(l);
+
+ /*
+ * the first unix socket directory cannot contain :port
+ * and the default port is used every-time, so strip the port
+ */
+ if ((colon = strrchr(*mainSocket, ':')) != NULL)
+ *colon = '\0';
+ }
+ else
+ *mainSocket = unixSocketDirs;
+ }
+ else
+ *mainSocket = unixSocketDirs;
+#endif
+}
/*****************************************************************************
* Comparison Functions used for bytea
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index fb376a0..670e1a7 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -663,10 +663,11 @@ UnlinkLockFile(int status, Datum filename)
* filename is the name of the lockfile to create.
* amPostmaster is used to determine how to encode the output PID.
* isDDLock and refName are used to determine what error message to produce.
+ * socketPath is the path to the Unix socket we want to lock.
*/
static void
CreateLockFile(const char *filename, bool amPostmaster,
- bool isDDLock, const char *refName)
+ bool isDDLock, const char *refName, const char *socketPath)
{
int fd;
char buffer[MAXPGPATH * 2 + 256];
@@ -892,7 +893,8 @@ CreateLockFile(const char *filename, bool amPostmaster,
(long) MyStartTime,
PostPortNumber,
#ifdef HAVE_UNIX_SOCKETS
- (*UnixSocketDir != '\0') ? UnixSocketDir : DEFAULT_PGSOCKET_DIR
+ (socketPath && *socketPath != '\0') ? socketPath
+ : DEFAULT_PGSOCKET_DIR
#else
""
#endif
@@ -947,21 +949,22 @@ CreateLockFile(const char *filename, bool amPostmaster,
* helps ensure that we are locking the directory we should be.
*/
void
-CreateDataDirLockFile(bool amPostmaster)
+CreateDataDirLockFile(bool amPostmaster, const char *socketDir)
{
- CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir);
+ CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir, socketDir);
}
/*
* Create a lockfile for the specified Unix socket file.
*/
void
-CreateSocketLockFile(const char *socketfile, bool amPostmaster)
+CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir)
{
char lockfile[MAXPGPATH];
snprintf(lockfile, sizeof(lockfile), "%s.lock", socketfile);
- CreateLockFile(lockfile, amPostmaster, false, socketfile);
+ CreateLockFile(lockfile, amPostmaster, false, socketfile, socketDir);
/* Save name of lockfile for TouchSocketLockFile */
strcpy(socketLockFile, lockfile);
}
@@ -1292,3 +1295,4 @@ pg_bindtextdomain(const char *domain)
}
#endif
}
+
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b756e58..bf90aff 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2894,14 +2894,15 @@ static struct config_string ConfigureNamesString[] =
},
{
- {"unix_socket_directory", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
- gettext_noop("Sets the directory where the Unix-domain socket will be created."),
- NULL,
+ {"unix_socket_directories", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop("Sets the directories where the Unix-domain socket will be created."),
+ gettext_noop("Directories are separated by \",\" and an additional "
+ "port number can be set, separated from directory by \":\"."),
GUC_SUPERUSER_ONLY
},
- &UnixSocketDir,
+ &UnixSocketDirs,
"",
- check_canonical_path, NULL, NULL
+ NULL, NULL, NULL
},
{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index fa75d00..1eea8aa 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -65,7 +65,9 @@
# Note: Increasing max_connections costs ~400 bytes of shared memory per
# connection slot, plus lock space (see max_locks_per_transaction).
#superuser_reserved_connections = 3 # (change requires restart)
-#unix_socket_directory = '' # (change requires restart)
+#unix_socket_directories = '' # comma-separated list of directories:port,
+ # while the first socket's port is the default one
+ # (change requires restart)
#unix_socket_group = '' # (change requires restart)
#unix_socket_permissions = 0777 # begin with 0 to use octal notation
# (change requires restart)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 3826312..6d811ff 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -521,7 +521,7 @@ test_postmaster_connection(bool do_checkpoint)
hostaddr = optlines[LOCK_FILE_LINE_LISTEN_ADDR - 1];
/*
- * While unix_socket_directory can accept relative
+ * While unix_socket_directories can accept relative
* directories, libpq's host parameter must have a
* leading slash to indicate a socket directory. So,
* ignore sockdir if it's relative, and try to use TCP
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b186eed..40802cf 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -398,8 +398,9 @@ extern char *local_preload_libraries_string;
#define LOCK_FILE_LINE_LISTEN_ADDR 6
#define LOCK_FILE_LINE_SHMEM_KEY 7
-extern void CreateDataDirLockFile(bool amPostmaster);
-extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster);
+extern void CreateDataDirLockFile(bool amPostmaster, const char *socketDir);
+extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir);
extern void TouchSocketLockFile(void);
extern void AddToDataDirLockFile(int target_line, const char *str);
extern void ValidatePgVersion(const char *path);
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 683ce3c..f19a3f8 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -19,7 +19,7 @@ extern int ReservedBackends;
extern int PostPortNumber;
extern int Unix_socket_permissions;
extern char *Unix_socket_group;
-extern char *UnixSocketDir;
+extern char *UnixSocketDirs;
extern char *ListenAddresses;
extern bool ClientAuthInProgress;
extern int PreAuthDelay;
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index d1e8370..492a08e 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -752,6 +752,10 @@ extern int varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid);
extern List *textToQualifiedNameList(text *textval);
extern bool SplitIdentifierString(char *rawstring, char separator,
List **namelist);
+extern bool SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist);
+extern void SplitUnixDirectories(char *unixSocketDirs, List **socketsList,
+ char **mainSocket);
extern Datum replace_text(PG_FUNCTION_ARGS);
extern text *replace_text_regexp(text *src_text, void *regexp,
text *replace_text, bool glob);
Honza Horak <hhorak@redhat.com> writes:
On 06/15/2012 05:40 PM, Honza Horak wrote:
I realized the patch has some difficulties -- namely the socket path in the data dir lock file, which currently uses one port for socket and the same for interface. So to allow users to use arbitrary port for all unix sockets, we'd need to add another line only for unix socket, which doesn't apply for other platforms. Or we could just say that the first socket will allways use the default port (PostPortNumber), which is a solution I prefer currently, but will be glad for any other opinion. This is also why there is still un-necesary string splitting in pg_ctl.c, which will be removed after the issue above is solved.
I did a review pass over this patch.
This is an enhanced patch, which forbids using a port number in the
first socket directory entry.
Well, not so much "forbids" as "silently ignores", which doesn't seem like
great user-interface design to me. If we're going to adopt this solution
I think we need it to throw an error instead of just ignoring the port
specification.
On the whole I prefer the solution you mention above: let's generalize
the postmaster.pid format (and pg_ctl) so that we don't need to assume
anything about port numbers matching up. The nearby discussion about
allowing listen_addresses to specify port number would break this
assumption anyway. If we just add two port numbers into postmaster.pid,
one for the Unix socket and one for the TCP port, we could get rid of
the problem entirely.
I read through the patch for awhile and noticed some other smaller
issues:
* the patch does not compile warning-free:
postgres.c: In function 'PostgresMain':
postgres.c:3669:3: warning: implicit declaration of function 'SplitUnixDirectories' [-Wimplicit-function-declaration]
varlena.c: In function 'SplitUnixDirectories':
varlena.c:2577:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
* the changes in bootstrap.c and postgres.c won't compile at all when not
HAVE_UNIX_SOCKETS, since mainSocket is referred to even though it won't
be defined in such a case.
* I'm not especially thrilled with propagating SplitUnixDirectories calls
into those two places anyway, nor with the weird decision for
SplitUnixDirectories to return a separate "mainSocket" value. Perhaps
what would be most sensible is to attach an assign hook to the
unix_socket_directories GUC parameter that would automatically split the
string and store the components into a globally-visible List variable
(which could replace the globally-visible string value we have now).
Then the places that need to reference the "main socket" could just
look at the first list entry if any. Also, the hook could enforce valid
parameter syntax.
* Also, I'm inclined to think it will work better if SplitUnixDirectories
takes care of separating out the port number data, which it could return
in an integer list parallel to the string list of directory names. For
one thing, if you don't then the places that currently are looking at
"mainSocket" are going to need to duplicate the port-splitting logic.
* Also, CreateDataDirLockFile generally gets all its information about GUC
settings by looking directly at the GUCs (eg, DataDir). It seems
inconsistent to pass it just this one value rather than having it find the
info for itself. So on the whole I'd drop the bootstrap.c and postgres.c
changes altogether and do whatever's needful inside CreateLockFile, or
perhaps even better, not write the Unix socket info at file creation but
add it with AddToDataDirLockFile later.
* It might be a good idea to s/unixSocketName/unixSocketDir/ throughout
pqcomm.c, since AFAICS none of those variables are actually the full path
name of the socket file. Also, I'm a bit disturbed that StreamServerPort
is now being called with UnixSocketDirs in a lot of places; that's either
wrong or useless. Maybe we could pass NULL in the places where it's not
meant to be a sensible value?
* Having Lock_AF_UNIX pass back a socket path seems rather grotty.
Possibly better to move the UNIXSOCK_PATH call to its caller and just pass
it a sock_path, similar to Setup_AF_UNIX? The placement of the addition
to the sock_paths list seems a bit random (or at least undercommented),
too.
* In postmaster.c, is it really possible for UnixSocketDirs to be null?
I'm inclined to think that code path is unreachable, since guc.c
initializes the value to empty-string not null. But in any case,
unix_socket_directories being an empty string should specify creating
zero sockets, I think. We need to change the default value to be
DEFAULT_PGSOCKET_DIR, or empty on Windows. (Likewise, if we're going to
pass a socket file name to CreateLockFile, it should be a socket file
name, not something that might need to be replaced by
DEFAULT_PGSOCKET_DIR.)
* Not sure about adding an is_absolute_path() insistence as you have done
in postmaster.c. We never required that before. I'm also inclined to
think that the canonicalize_path work should be done in
SplitDirectoriesString not here.
regards, tom lane
I wrote:
On the whole I prefer the solution you mention above: let's generalize
the postmaster.pid format (and pg_ctl) so that we don't need to assume
anything about port numbers matching up. The nearby discussion about
allowing listen_addresses to specify port number would break this
assumption anyway. If we just add two port numbers into postmaster.pid,
one for the Unix socket and one for the TCP port, we could get rid of
the problem entirely.
After further thought, I think that this approach would make it a good
idea to drop support for alternate port numbers from the present patch.
Let's just deal with alternate socket directories for now. There could
be a follow-on patch that adds support for nondefault port numbers in
both listen_addresses and unix_socket_directories, and fixes up the
postmaster.pid format to support that.
I will admit that part of my desire to do it this way is a narrow Fedora
rationale: in the Fedora package, we are going to want to back-patch the
alternate-directory feature into 9.2 (and maybe 9.1) so as to fix our
problems with systemd's PrivateTmp feature. The alternate-port-number
feature is not necessary for that, and leaving it out would make for a
significantly smaller back-patch. But in any case, it seems like adding
alternate-port-number support for Unix sockets and not doing it for TCP
ports at the same time is just weird. So I think it's a separate
feature and should be a separate patch.
regards, tom lane
On Tue, Jul 3, 2012 at 11:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
On the whole I prefer the solution you mention above: let's generalize
the postmaster.pid format (and pg_ctl) so that we don't need to assume
anything about port numbers matching up. The nearby discussion about
allowing listen_addresses to specify port number would break this
assumption anyway. If we just add two port numbers into postmaster.pid,
one for the Unix socket and one for the TCP port, we could get rid of
the problem entirely.After further thought, I think that this approach would make it a good
idea to drop support for alternate port numbers from the present patch.
Let's just deal with alternate socket directories for now. There could
be a follow-on patch that adds support for nondefault port numbers in
both listen_addresses and unix_socket_directories, and fixes up the
postmaster.pid format to support that.I will admit that part of my desire to do it this way is a narrow Fedora
rationale: in the Fedora package, we are going to want to back-patch the
alternate-directory feature into 9.2 (and maybe 9.1) so as to fix our
problems with systemd's PrivateTmp feature. The alternate-port-number
feature is not necessary for that, and leaving it out would make for a
significantly smaller back-patch. But in any case, it seems like adding
alternate-port-number support for Unix sockets and not doing it for TCP
ports at the same time is just weird. So I think it's a separate
feature and should be a separate patch.
+1
I still find it difficult to think of a good use case for multiple ports.
cheers
andrew
On 07/02/2012 09:45 PM, Tom Lane wrote:
Honza Horak <hhorak@redhat.com> writes:
On 06/15/2012 05:40 PM, Honza Horak wrote:
I realized the patch has some difficulties -- namely the socket path in the data dir lock file, which currently uses one port for socket and the same for interface. So to allow users to use arbitrary port for all unix sockets, we'd need to add another line only for unix socket, which doesn't apply for other platforms. Or we could just say that the first socket will allways use the default port (PostPortNumber), which is a solution I prefer currently, but will be glad for any other opinion. This is also why there is still un-necesary string splitting in pg_ctl.c, which will be removed after the issue above is solved.
I did a review pass over this patch.
I have finally an enhanced patch, see the attachment and feel free to
comment.
Well, not so much "forbids" as "silently ignores", which doesn't seem like
great user-interface design to me. If we're going to adopt this solution
I think we need it to throw an error instead of just ignoring the port
specification.
Alternate-port-number support has been removed from the patch, as per
Tom's e-mail from 07/03/12. It can be add in the future, if we really
need it.
* I'm not especially thrilled with propagating SplitUnixDirectories calls
into those two places anyway, nor with the weird decision for
SplitUnixDirectories to return a separate "mainSocket" value. Perhaps
what would be most sensible is to attach an assign hook to the
unix_socket_directories GUC parameter that would automatically split the
string and store the components into a globally-visible List variable
(which could replace the globally-visible string value we have now).
Replacing the old global string value would probably need a new
configuration type "List" to be added, since otherwise guc works with it
as with a string. Adding that seems like too big overhead to me and thus
it seems better to add a new global (List *) variable and let the
original value of type (char *) to store non-parsed value.
Except that I believe all other Tom's comments have been involved.
Regards,
Honza
Attachments:
postgresql-multiple-socket-dirs.patchtext/x-patch; name=postgresql-multiple-socket-dirs.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cfdb33a..679c40a 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -838,7 +838,7 @@ omicron bryanh guest1
<varname>unix_socket_permissions</varname> (and possibly
<varname>unix_socket_group</varname>) configuration parameters as
described in <xref linkend="runtime-config-connection">. Or you
- could set the <varname>unix_socket_directory</varname>
+ could set the <varname>unix_socket_directories</varname>
configuration parameter to place the socket file in a suitably
restricted directory.
</para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3a0b16d..67997d6 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -445,17 +445,18 @@ SET ENABLE_SEQSCAN TO OFF;
</listitem>
</varlistentry>
- <varlistentry id="guc-unix-socket-directory" xreflabel="unix_socket_directory">
- <term><varname>unix_socket_directory</varname> (<type>string</type>)</term>
+ <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories">
+ <term><varname>unix_socket_directories</varname> (<type>string</type>)</term>
<indexterm>
- <primary><varname>unix_socket_directory</> configuration parameter</primary>
+ <primary><varname>unix_socket_directories</> configuration parameter</primary>
</indexterm>
<listitem>
<para>
- Specifies the directory of the Unix-domain socket on which the
+ Specifies the directories of the Unix-domain sockets on which the
server is to listen for
connections from client applications. The default is normally
<filename>/tmp</filename>, but can be changed at build time.
+ Directories are separated by ','.
This parameter can only be set at server start.
</para>
@@ -464,7 +465,7 @@ SET ENABLE_SEQSCAN TO OFF;
<literal>.s.PGSQL.<replaceable>nnnn</></literal> where
<replaceable>nnnn</> is the server's port number, an ordinary file
named <literal>.s.PGSQL.<replaceable>nnnn</>.lock</literal> will be
- created in the <varname>unix_socket_directory</> directory. Neither
+ created in the <varname>unix_socket_directories</> directories. Neither
file should ever be removed manually.
</para>
@@ -6551,7 +6552,7 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
</row>
<row>
<entry><option>-k <replaceable>x</replaceable></option></entry>
- <entry><literal>unix_socket_directory = <replaceable>x</replaceable></></entry>
+ <entry><literal>unix_socket_directories = <replaceable>x</replaceable></></entry>
</row>
<row>
<entry><option>-l</option></entry>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 8717798..9cc9d42 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1718,7 +1718,7 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
<para>
The simplest way to prevent spoofing for <literal>local</>
connections is to use a Unix domain socket directory (<xref
- linkend="guc-unix-socket-directory">) that has write permission only
+ linkend="guc-unix-socket-directories">) that has write permission only
for a trusted local user. This prevents a malicious user from creating
their own socket file in that directory. If you are concerned that
some applications might still reference <filename>/tmp</> for the
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 5272811..3e22388 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -103,8 +103,8 @@ int Unix_socket_permissions;
char *Unix_socket_group;
-/* Where the Unix socket file is */
-static char sock_path[MAXPGPATH];
+/* Where the Unix socket files are */
+static List *sock_paths = NIL;
/*
@@ -140,8 +140,9 @@ static int internal_flush(void);
static void pq_set_nonblocking(bool nonblocking);
#ifdef HAVE_UNIX_SOCKETS
-static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName);
-static int Setup_AF_UNIX(void);
+static int Lock_AF_UNIX(unsigned short portNumber, char *unixSocketDir,
+ char *unixSocketPath);
+static int Setup_AF_UNIX(char *sock_path);
#endif /* HAVE_UNIX_SOCKETS */
@@ -234,14 +235,23 @@ pq_close(int code, Datum arg)
/* StreamDoUnlink()
* Shutdown routine for backend connection
- * If a Unix socket is used for communication, explicitly close it.
+ * If any Unix sockets are used for communication, explicitly close them.
*/
#ifdef HAVE_UNIX_SOCKETS
static void
StreamDoUnlink(int code, Datum arg)
{
- Assert(sock_path[0]);
- unlink(sock_path);
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
+ {
+ cursocket = (char *) lfirst(l);
+ unlink(cursocket);
+ }
+ list_free(sock_paths);
+ sock_paths = NIL;
}
#endif /* HAVE_UNIX_SOCKETS */
@@ -256,7 +266,7 @@ StreamDoUnlink(int code, Datum arg)
int
StreamServerPort(int family, char *hostName, unsigned short portNumber,
- char *unixSocketName,
+ char *unixSocketDir,
pgsocket ListenSocket[], int MaxListen)
{
pgsocket fd;
@@ -267,6 +277,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
const char *familyDesc;
char familyDescBuf[64];
char *service;
+ char unixSocketPath[MAXPGPATH];
struct addrinfo *addrs = NULL,
*addr;
struct addrinfo hint;
@@ -286,10 +297,14 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (family == AF_UNIX)
{
- /* Lock_AF_UNIX will also fill in sock_path. */
- if (Lock_AF_UNIX(portNumber, unixSocketName) != STATUS_OK)
+ /*
+ * Create unixSocketPath from portNumber and unixSocketDir
+ * and lock that file
+ */
+ UNIXSOCK_PATH(unixSocketPath, portNumber, unixSocketDir);
+ if (Lock_AF_UNIX(portNumber, unixSocketDir, unixSocketPath) != STATUS_OK)
return STATUS_ERROR;
- service = sock_path;
+ service = unixSocketPath;
}
else
#endif /* HAVE_UNIX_SOCKETS */
@@ -432,7 +447,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
(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, sock_path) :
+ (int) portNumber, service) :
errhint("Is another postmaster already running on port %d?"
" If not, wait a few seconds and retry.",
(int) portNumber)));
@@ -443,7 +458,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
#ifdef HAVE_UNIX_SOCKETS
if (addr->ai_family == AF_UNIX)
{
- if (Setup_AF_UNIX() != STATUS_OK)
+ if (Setup_AF_UNIX(service) != STATUS_OK)
{
closesocket(fd);
break;
@@ -490,10 +505,8 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
* Lock_AF_UNIX -- configure unix socket file path
*/
static int
-Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
+Lock_AF_UNIX(unsigned short portNumber, char *unixSocketDir, char *unixSocketPath)
{
- UNIXSOCK_PATH(sock_path, portNumber, unixSocketName);
-
/*
* Grab an interlock file associated with the socket file.
*
@@ -502,13 +515,19 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* more portable, and second, it lets us remove any pre-existing socket
* file without race conditions.
*/
- CreateSocketLockFile(sock_path, true);
+ CreateSocketLockFile(unixSocketPath, true, unixSocketDir);
/*
* Once we have the interlock, we can safely delete any pre-existing
* socket file to avoid failure at bind() time.
*/
- unlink(sock_path);
+ unlink(unixSocketPath);
+
+ /*
+ * Add a new socket file to the list, so we always know which socket
+ * paths exist and should be removed in the end.
+ */
+ sock_paths = lappend(sock_paths, pstrdup(unixSocketPath));
return STATUS_OK;
}
@@ -518,7 +537,7 @@ Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName)
* Setup_AF_UNIX -- configure unix socket permissions
*/
static int
-Setup_AF_UNIX(void)
+Setup_AF_UNIX(char *sock_path)
{
/* Arrange to unlink the socket file at exit */
on_proc_exit(StreamDoUnlink, 0);
@@ -707,17 +726,21 @@ StreamClose(pgsocket sock)
* TouchSocketFile -- mark socket file as recently accessed
*
* This routine should be called every so often to ensure that the socket
- * file has a recent mod date (ordinary operations on sockets usually won't
- * change the mod date). That saves it from being removed by
+ * files have a recent mod date (ordinary operations on sockets usually won't
+ * change the mod date). That saves them from being removed by
* overenthusiastic /tmp-directory-cleaner daemons. (Another reason we should
- * never have put the socket file in /tmp...)
+ * never have put the socket files in /tmp...)
*/
void
TouchSocketFile(void)
{
- /* Do nothing if we did not create a socket... */
- if (sock_path[0] != '\0')
+ ListCell *l;
+ char *cursocket;
+
+ /* Loop through all created sockets... */
+ foreach(l, sock_paths)
{
+ cursocket = (char *) lfirst(l);
/*
* utime() is POSIX standard, utimes() is a common alternative. If we
* have neither, there's no way to affect the mod or access time of
@@ -726,10 +749,10 @@ TouchSocketFile(void)
* In either path, we ignore errors; there's no point in complaining.
*/
#ifdef HAVE_UTIME
- utime(sock_path, NULL);
+ utime(cursocket, NULL);
#else /* !HAVE_UTIME */
#ifdef HAVE_UTIMES
- utimes(sock_path, NULL);
+ utimes(cursocket, NULL);
#endif /* HAVE_UTIMES */
#endif /* HAVE_UTIME */
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 45f6ac6..a89455f 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -156,7 +156,8 @@ static Backend *ShmemBackendArray;
/* The socket number we are listening for connections on */
int PostPortNumber;
-char *UnixSocketDir;
+char *UnixSocketDirs;
+List *UnixSocketDirList;
char *ListenAddresses;
/*
@@ -609,7 +610,7 @@ PostmasterMain(int argc, char *argv[])
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+ SetConfigOption("unix_socket_directories", optarg, PGC_POSTMASTER, PGC_S_ARGV);
break;
case 'l':
@@ -838,6 +839,47 @@ PostmasterMain(int argc, char *argv[])
for (i = 0; i < MAXLISTEN; i++)
ListenSocket[i] = PGINVALID_SOCKET;
+#ifdef HAVE_UNIX_SOCKETS
+ /*
+ * We can specify several directories for Unix sockets to listen on,
+ * separated with ','. Socket name itself is the same in all cases.
+ */
+ {
+ int success = 0;
+ bool listen_socket_saved = false;
+ ListCell *l;
+
+ foreach(l, UnixSocketDirList)
+ {
+ char *cursocket = (char *) lfirst(l);
+
+ status = StreamServerPort(AF_UNIX, NULL,
+ (unsigned short) PostPortNumber,
+ cursocket,
+ ListenSocket, MAXLISTEN);
+
+ if (status == STATUS_OK)
+ {
+ success++;
+ /* record the first successful unix socket in lockfile */
+ if (!listen_socket_saved)
+ {
+ AddToDataDirLockFile(LOCK_FILE_LINE_SOCKET_DIR, cursocket);
+ listen_socket_saved = true;
+ }
+ }
+ else
+ ereport(WARNING,
+ (errmsg("could not create Unix-domain socket at \"%s\"", cursocket)));
+
+ }
+
+ if (!success && list_length(UnixSocketDirList))
+ ereport(FATAL,
+ (errmsg("could not create any Unix-domain sockets")));
+ }
+#endif
+
if (ListenAddresses)
{
char *rawstring;
@@ -864,12 +906,12 @@ PostmasterMain(int argc, char *argv[])
if (strcmp(curhost, "*") == 0)
status = StreamServerPort(AF_UNSPEC, NULL,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ NULL,
ListenSocket, MAXLISTEN);
else
status = StreamServerPort(AF_UNSPEC, curhost,
(unsigned short) PostPortNumber,
- UnixSocketDir,
+ NULL,
ListenSocket, MAXLISTEN);
if (status == STATUS_OK)
@@ -934,16 +976,6 @@ PostmasterMain(int argc, char *argv[])
}
#endif
-#ifdef HAVE_UNIX_SOCKETS
- status = StreamServerPort(AF_UNIX, NULL,
- (unsigned short) PostPortNumber,
- UnixSocketDir,
- ListenSocket, MAXLISTEN);
- if (status != STATUS_OK)
- ereport(WARNING,
- (errmsg("could not create Unix-domain socket")));
-#endif
-
/*
* check that we have some socket to listen on
*/
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 9a5438f..265fad9 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3343,7 +3343,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
break;
case 'k':
- SetConfigOption("unix_socket_directory", optarg, ctx, gucsource);
+ SetConfigOption("unix_socket_directories", optarg, ctx, gucsource);
break;
case 'l':
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index e1b57ba..bddc972 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2445,6 +2445,109 @@ SplitIdentifierString(char *rawstring, char separator,
return true;
}
+/*
+ * SplitDirectoriesString --- parse a string containing directories
+ *
+ * Inputs:
+ * rawstring: the input string; must be overwritable! On return, it's
+ * been modified to contain the separated directories.
+ * separator: the separator punctuation expected between directories
+ * (typically ',' or ';'). Whitespace may also appear around
+ * directories.
+ * Outputs:
+ * namelist: filled with a palloc'd list of pointers to directories within
+ * rawstring. Caller should list_free() this even on error return.
+ *
+ * Returns TRUE if okay, FALSE if there is a syntax error in the string.
+ *
+ * Note that an empty string is considered okay here.
+ */
+bool
+SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist)
+{
+ char *nextp = rawstring;
+ bool done = false;
+ char *tmpname;
+
+ *namelist = NIL;
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace */
+
+ if (*nextp == '\0')
+ return true; /* allow empty string */
+
+ /* At the top of the loop, we are at start of a new directories. */
+ do
+ {
+ char *curname;
+ char *endp;
+
+ if (*nextp == '\"')
+ {
+ /* Quoted name --- collapse quote-quote pairs */
+ curname = nextp + 1;
+ for (;;)
+ {
+ endp = strchr(nextp + 1, '\"');
+ if (endp == NULL)
+ return false; /* mismatched quotes */
+ if (endp[1] != '\"')
+ break; /* found end of quoted name */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(endp, endp + 1, strlen(endp));
+ nextp = endp;
+ }
+ /* endp now points at the terminating quote */
+ nextp = endp + 1;
+ }
+ else
+ {
+ /* Unquoted name --- extends to separator or whitespace */
+ curname = nextp;
+ while (*nextp && *nextp != separator &&
+ !isspace((unsigned char) *nextp))
+ nextp++;
+ endp = nextp;
+ if (curname == nextp)
+ return false; /* empty unquoted name not allowed */
+ }
+
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip trailing whitespace */
+
+ if (*nextp == separator)
+ {
+ nextp++;
+ while (isspace((unsigned char) *nextp))
+ nextp++; /* skip leading whitespace for next */
+ /* we expect another name, so done remains false */
+ }
+ else if (*nextp == '\0')
+ done = true;
+ else
+ return false; /* invalid syntax */
+
+ /* Now safe to overwrite separator with a null */
+ *endp = '\0';
+
+ /* Truncate path if it's overlength */
+ if (strlen(curname) >= MAXPGPATH)
+ curname[MAXPGPATH-1] = '\0';
+
+ /*
+ * Finished isolating current name --- add it to list
+ */
+ tmpname = pstrdup(curname);
+ canonicalize_path(tmpname);
+ *namelist = lappend(*namelist, tmpname);
+
+ /* Loop back if we didn't reach end of string */
+ } while (!done);
+
+ return true;
+}
/*****************************************************************************
* Comparison Functions used for bytea
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index fb376a0..8620ee8 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -663,10 +663,11 @@ UnlinkLockFile(int status, Datum filename)
* filename is the name of the lockfile to create.
* amPostmaster is used to determine how to encode the output PID.
* isDDLock and refName are used to determine what error message to produce.
+ * socketPath is the path to the Unix socket we want to lock.
*/
static void
CreateLockFile(const char *filename, bool amPostmaster,
- bool isDDLock, const char *refName)
+ bool isDDLock, const char *refName, const char *socketPath)
{
int fd;
char buffer[MAXPGPATH * 2 + 256];
@@ -892,7 +893,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
(long) MyStartTime,
PostPortNumber,
#ifdef HAVE_UNIX_SOCKETS
- (*UnixSocketDir != '\0') ? UnixSocketDir : DEFAULT_PGSOCKET_DIR
+ (socketPath) ? socketPath : ""
#else
""
#endif
@@ -949,19 +950,20 @@ CreateLockFile(const char *filename, bool amPostmaster,
void
CreateDataDirLockFile(bool amPostmaster)
{
- CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir);
+ CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir, NULL);
}
/*
* Create a lockfile for the specified Unix socket file.
*/
void
-CreateSocketLockFile(const char *socketfile, bool amPostmaster)
+CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir)
{
char lockfile[MAXPGPATH];
snprintf(lockfile, sizeof(lockfile), "%s.lock", socketfile);
- CreateLockFile(lockfile, amPostmaster, false, socketfile);
+ CreateLockFile(lockfile, amPostmaster, false, socketfile, socketDir);
/* Save name of lockfile for TouchSocketLockFile */
strcpy(socketLockFile, lockfile);
}
@@ -1292,3 +1294,4 @@ pg_bindtextdomain(const char *domain)
}
#endif
}
+
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 087aaf9..5603ce4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -207,6 +207,7 @@ static bool check_application_name(char **newval, void **extra, GucSource source
static void assign_application_name(const char *newval, void *extra);
static const char *show_unix_socket_permissions(void);
static const char *show_log_file_mode(void);
+static void assign_unix_socket_directories(const char *newval, void *extra);
static char *config_enum_get_options(struct config_enum * record,
const char *prefix, const char *suffix,
@@ -2895,14 +2896,18 @@ static struct config_string ConfigureNamesString[] =
},
{
- {"unix_socket_directory", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
- gettext_noop("Sets the directory where the Unix-domain socket will be created."),
- NULL,
+ {"unix_socket_directories", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop("Sets the directories where the Unix-domain socket will be created."),
+ gettext_noop("Directories are separated by \",\"."),
GUC_SUPERUSER_ONLY
},
- &UnixSocketDir,
+ &UnixSocketDirs,
+#ifdef HAVE_UNIX_SOCKETS
+ DEFAULT_PGSOCKET_DIR,
+#else
"",
- check_canonical_path, NULL, NULL
+#endif
+ NULL, assign_unix_socket_directories, NULL
},
{
@@ -8759,4 +8764,34 @@ show_log_file_mode(void)
return buf;
}
+static void
+assign_unix_socket_directories(const char *newval, void *extra)
+{
+#ifdef HAVE_UNIX_SOCKETS
+ char *rawSocketsString;
+ ListCell *l;
+
+ /* Clean previous UnixSocketDirList list if not empty */
+ if (UnixSocketDirList)
+ {
+ list_free(UnixSocketDirList);
+ UnixSocketDirList = NIL;
+ }
+
+ /* Need a modifiable copy of value */
+ rawSocketsString = pstrdup(newval);
+
+ /* Parse string into list of directories */
+ if (!SplitDirectoriesString(rawSocketsString, ',', &UnixSocketDirList))
+ {
+ /* syntax error in list */
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid list syntax for \"unix_socket_directories\"")));
+ }
+
+ pfree(rawSocketsString);
+#endif
+}
+
#include "guc-file.c"
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index fa75d00..42b5e40 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -65,7 +65,8 @@
# Note: Increasing max_connections costs ~400 bytes of shared memory per
# connection slot, plus lock space (see max_locks_per_transaction).
#superuser_reserved_connections = 3 # (change requires restart)
-#unix_socket_directory = '' # (change requires restart)
+#unix_socket_directories = '' # comma-separated list of directories,
+ # (change requires restart)
#unix_socket_group = '' # (change requires restart)
#unix_socket_permissions = 0777 # begin with 0 to use octal notation
# (change requires restart)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 72fc4c1..af8d8b2 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -521,7 +521,7 @@ test_postmaster_connection(bool do_checkpoint)
hostaddr = optlines[LOCK_FILE_LINE_LISTEN_ADDR - 1];
/*
- * While unix_socket_directory can accept relative
+ * While unix_socket_directories can accept relative
* directories, libpq's host parameter must have a
* leading slash to indicate a socket directory. So,
* ignore sockdir if it's relative, and try to use TCP
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 7083cd8..a79200d 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -45,7 +45,7 @@ typedef struct
* prototypes for functions in pqcomm.c
*/
extern int StreamServerPort(int family, char *hostName,
- unsigned short portNumber, char *unixSocketName, pgsocket ListenSocket[],
+ unsigned short portNumber, char *unixSocketDir, pgsocket ListenSocket[],
int MaxListen);
extern int StreamConnection(pgsocket server_fd, Port *port);
extern void StreamClose(pgsocket sock);
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b186eed..edfa161 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -399,7 +399,8 @@ extern char *local_preload_libraries_string;
#define LOCK_FILE_LINE_SHMEM_KEY 7
extern void CreateDataDirLockFile(bool amPostmaster);
-extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster);
+extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
+ const char *socketDir);
extern void TouchSocketLockFile(void);
extern void AddToDataDirLockFile(int target_line, const char *str);
extern void ValidatePgVersion(const char *path);
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 7d01d3d..aa549e6 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -13,13 +13,16 @@
#ifndef _POSTMASTER_H
#define _POSTMASTER_H
+#include "utils/builtins.h"
+
/* GUC options */
extern bool EnableSSL;
extern int ReservedBackends;
extern int PostPortNumber;
extern int Unix_socket_permissions;
extern char *Unix_socket_group;
-extern char *UnixSocketDir;
+extern char *UnixSocketDirs;
+extern List *UnixSocketDirList;
extern char *ListenAddresses;
extern bool ClientAuthInProgress;
extern int PreAuthDelay;
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 1063403..adab4ec 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -752,6 +752,8 @@ extern int varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid);
extern List *textToQualifiedNameList(text *textval);
extern bool SplitIdentifierString(char *rawstring, char separator,
List **namelist);
+extern bool SplitDirectoriesString(char *rawstring, char separator,
+ List **namelist);
extern Datum replace_text(PG_FUNCTION_ARGS);
extern text *replace_text_regexp(text *src_text, void *regexp,
text *replace_text, bool glob);
Honza Horak <hhorak@redhat.com> writes:
Alternate-port-number support has been removed from the patch, as per
Tom's e-mail from 07/03/12. It can be add in the future, if we really
need it.
I've reviewed and committed this. There were some cosmetic things
I adjusted, as well as a couple of fairly large non-cosmetic things:
* I did not like rearranging the order in which TCP and Unix sockets
get opened. It's possible that this comment in postmaster.c
* For the same reason, it's best to grab the TCP socket(s) before the
* Unix socket.
is no longer relevant, but I'm doubtful of that. The reason you had
switched them appeared to be to update the SOCKET_DIR line in
postmaster.pid before updating the LISTEN_ADDR line, but the only reason
to do that is the implementation restriction in AddToDataDirLockFile
that it can't update a non-last line in the pidfile. That's not that
hard to get rid of, and it's something we'd probably want someday
anyway, so I fixed that function and put the socket opening order back
as it had been.
* The code in pqcomm.c queued another on_proc_exit function for each
socket. There was no purpose in that since the first one would do all
the work, but the postmaster's on_proc_exit array isn't very large,
and it's not hard to foresee the useless entries causing a failure by
overflowing the array. Similarly, miscinit.c queued an on_proc_exit
function for each lock file it had to get rid of, which was fine as
long as there was an upper bound of 2, but now not so much. I fixed
them to use similar logic of keeping a list of file names and queueing
the on_proc_exit function when adding the first list entry.
* You'd fixed TouchSocketFiles to touch all the sockets, but missed
making TouchSocketLockFiles touch all their lock files. That would
be problematic if /tmp was a non-first entry in the list.
I also simplified the GUC interactions by leaving the GUC variable as a
simple string and splitting it at the point of use, so that the code is
more parallel to what we do with listen_addresses.
regards, tom lane