Re: Cleaning up historical portability baggage - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Cleaning up historical portability baggage
Date
Msg-id 20220811171412.7sgz2mpro5fpko3t@awork3.anarazel.de
Whole thread Raw
In response to Re: Cleaning up historical portability baggage  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Cleaning up historical portability baggage
Re: Cleaning up historical portability baggage
List pgsql-hackers
Hi,

On 2022-08-11 10:52:51 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > The most interesting things to say about these ones are:
> >  * The concept of a no-Unix-socket build is removed.  We should be
> > able to do that now, right?  Peter E seemed to say approximately that
> > in the commit message for 797129e5.  Or is there a thought that a new
> > operating system might show up that doesn't have 'em and we'd wish
> > we'd kept this stuff well marked out?
> 
> I'm kind of down on removing that.  I certainly think it's premature
> to do so today, when we haven't even yet shipped a release that
> assumes we can always define it on Windows

I think what might be good next step is to have tests default to using unix
sockets on windows, rather than requiring PG_TEST_USE_UNIX_SOCKETS. The
pg_regress.c and Utils.pm hunks disabling it by default on windows don't
really make sense anymore.

#if !defined(HAVE_UNIX_SOCKETS)
    use_unix_sockets = false;
#elif defined(WIN32)

    /*
     * We don't use Unix-domain sockets on Windows by default, even if the
     * build supports them.  (See comment at remove_temp() for a reason.)
     * Override at your own risk.
     */
    use_unix_sockets = getenv("PG_TEST_USE_UNIX_SOCKETS") ? true : false;
#else
    use_unix_sockets = true;
#endif

and

    # Specifies whether to use Unix sockets for test setups.  On
    # Windows we don't use them by default since it's not universally
    # supported, but it can be overridden if desired.
    $use_unix_sockets =
      (!$windows_os || defined $ENV{PG_TEST_USE_UNIX_SOCKETS});


Tests don't reliably run on windows without PG_TEST_USE_UNIX_SOCKETS, due to
the port conflict detection being incredibly racy. I see occasional failures
even without test concurrency, and with test concurrency it reliably fails.


I don't really know what to do about the warnings around remove_temp() and
trapsig(). I think we actually may be overreading the restrictions. To me the
documented restrictions read more like a high-level-ish explanation of what's
safe in a signal handler and what not. And it seems to not have caused a
problem on windows on several thousand CI cycles, including plenty failures.


Alternatively we could just default to putting the socketdir inside the data
directory on windows - I *think* windows doesn't have strict path length
limits for the socket location. If the socket dir isn't in some global temp
directory, we don't really need signal_remove_temp, given that we're ok with
leaving the much bigger data directory around.  The socket directory
determination doesn't really work on windows right now anyway, one manually
has to set a temp directory as TMPDIR isn't normally set on windows, and /tmp
doesn't exist.

    char       *template = psprintf("%s/pg_regress-XXXXXX",
                                    getenv("TMPDIR") ? getenv("TMPDIR") : "/tmp");

both TEMP and TMP would exist though...



> -- I won't be too surprised if we get pushback on that after 15.0 is out.

From what angle? I think our default behaviour doesn't change because
DEFAULT_PGSOCKET_DIR is "". And OS compatibility wise we apparently are good
as well?


> But in general, Unix sockets seem like kind of an obvious thing that might
> not be there on some future platform.

Maybe not with the precise API, but I can't imagine a new platform not having
something very similar. It serves a pretty obvious need, and I think the
security benefits have become more important over time...

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: "Jonathan S. Katz"
Date:
Subject: Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size
Next
From: "Euler Taveira"
Date:
Subject: Re: Allow logical replication to copy tables in binary format