Re: Noah Misch 2014-07-12 <20140712170151.ga1985...@tornado.leadboat.com> > 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?
I didn't want to have too many arrays for additionally storing the socket and lockfile names, and unlike pg_regress, pg_upgrade usually doesn't need to delete the files by itself, so it seemed like a good choice to rely on the postmaster removing them. Now, if get_sock_dir() should only return a single directory instead of many (well, two), that makes the original code from pg_regress more attractive to use. (Possibly it will even be a candidate for moving to pgcommon.a, though the static/global variables might defeat that.) I'll send an updated patch soonish. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers