On Fri, Jul 11, 2014 at 12:40:09PM +0300, Christoph Berg wrote: > > > > > > I believe pg_upgrade itself still needs a fix. While it's not a > > > > > > security problem to put the socket in $CWD while upgrading (it is > > > > > > using -c unix_socket_permissions=0700), this behavior is pretty > > > > > > unexpected, and does fail if your $CWD is > 107 bytes.
> > Here's the patch. Proposed commit message: > > > > Create pg_upgrade sockets in temp directories > > > > pg_upgrade used to use the current directory for UNIX sockets to > > access the old/new cluster. This fails when the current path is > > > 107 bytes. Fix by reusing the tempdir code from pg_regress > > introduced in be76a6d39e2832d4b88c0e1cc381aa44a7f86881. For cleanup, > > we need to remember up to two directories. Thanks. Preliminary questions: > +#ifdef HAVE_UNIX_SOCKETS > +/* make_temp_sockdir() is invoked at most twice from pg_upgrade.c via > get_sock_dir() */ > +#define MAX_TEMPDIRS 2 > +static int n_tempdirs = 0; /* actual number of directories created */ > +static const char *temp_sockdir[MAX_TEMPDIRS]; > +#endif get_sock_dir() currently returns the same directory, the CWD, for both calls; can't it continue to do so? We already have good reason not to start two postmasters simultaneously during pg_upgrade. > +/* > + * Remove the socket temporary directories. pg_ctl waits for postmaster > + * shutdown, so we expect the directory to be empty, unless we are > interrupted > + * by a signal, in which case the postmaster will clean up the sockets, but > + * there's a race condition with us removing the directory. What's the reason for addressing that race condition in pg_regress and not addressing it in pg_upgrade? -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers