Обсуждение: Unix-domain socket support on Windows

It works!

(apparently as of Windows 10 version 1803)

Here are some patches to get a discussion rolling.

Basically, it just works, but you need to define your own struct
sockaddr_un.  (This is what configure currently uses as a proxy for
HAVE_UNIX_SOCKETS, so (a) that needs a bit of tweaking, and (b) that is
the reason why builds haven't blown up already.)

But we'll now need to make things work so that binaries with Unix-domain
socket support work on systems without run-time support.  We already did
that exercise with IPv6 support, so some of the framework is already in
place.

Depending on your Windows environment, there might not be a suitable
/tmp directory, so you'll need to specify a directory explicitly using
postgres -k or similar.  This leads to the question what the default for
DEFAULT_PGSOCKET_DIR should be on Windows.  I think it's probably best,
at least for now, to set it so that by default, neither server nor libpq
use Unix sockets unless explicitly selected.  This can be done easily on
the server side by defining DEFAULT_PGSOCKET_DIR as "".  But in libpq, I
don't think the code would handle that correctly everywhere, so it would
need some more analysis and restructuring possibly.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 07/08/2019 16:56, Peter Eisentraut wrote:
> It works!

Cool!

Am I reading the patches correctly, that getpeereid() still doesn't work 
on Windows? That means that peer authentication doesn't work, right? 
That's a bit sad. One of the big advantages of unix domain sockets over 
TCP is peer authentication.

- Heikki

On 2019-08-07 16:06, Heikki Linnakangas wrote:
> Am I reading the patches correctly, that getpeereid() still doesn't work 
> on Windows? That means that peer authentication doesn't work, right? 
> That's a bit sad. One of the big advantages of unix domain sockets over 
> TCP is peer authentication.

Correct, it's not supported.  I think it's plausible that they will add
this in the future.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-08-07 15:56, Peter Eisentraut wrote:
> Depending on your Windows environment, there might not be a suitable
> /tmp directory, so you'll need to specify a directory explicitly using
> postgres -k or similar.  This leads to the question what the default for
> DEFAULT_PGSOCKET_DIR should be on Windows.  I think it's probably best,
> at least for now, to set it so that by default, neither server nor libpq
> use Unix sockets unless explicitly selected.  This can be done easily on
> the server side by defining DEFAULT_PGSOCKET_DIR as "".  But in libpq, I
> don't think the code would handle that correctly everywhere, so it would
> need some more analysis and restructuring possibly.

Updated patches, which now also address that issue: There is no default
socket dir on Windows and it's disabled by default on both client and
server.

Some comments on the patches:

v2-0001-Enable-Unix-domain-sockets-support-on-Windows.patch

This is pretty straightforward, apart from maybe some comments, but it
would need to be committed last, because it would enable all the Unix
socket related code on Windows, which needs to be fixed up by the
subsequent patches first.

v2-0002-Sort-out-getpeereid-and-struct-passwd-handling-on.patch

Maybe a more elegant way with fewer #ifdef WIN32 can be found?

v2-0003-psql-Remove-one-use-of-HAVE_UNIX_SOCKETS.patch

This could be committed independently.

v2-0004-libpq-Remove-unnecessary-uses-of-HAVE_UNIX_SOCKET.patch

This one as well.

v2-0005-initdb-Detect-Unix-domain-socket-support-dynamica.patch

I think this patch contains some nice improvements in general.  How much
of that ends up being useful depends on how the subsequent patches (esp.
0007) end up, since with Unix-domain sockets disabled by default on
Windows, we won't need initdb doing any detection.

v2-0006-Fix-handling-of-Unix-domain-sockets-on-Windows-in.patch

This is a fairly independent and isolated change.

v2-0007-Disable-Unix-sockets-by-default-on-Windows.patch

This one is a bit complicated.  Since there is no good default location
for Unix sockets on Windows, and many systems won't support them for
some time, the default implemented here is to not use them by default on
the server or client.  This needs a fair amount of restructuring in the
to support the case of "supports Unix sockets but don't use them by
default", while maintaining the existing cases of "doesn't support Unix
sockets" and "use Unix sockets by default".  There is some room for
discussion here.


This patch set needs testers with various Windows versions to test
different configurations, combinations, and versions.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Aug 14, 2019 at 6:27 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> This patch set needs testers with various Windows versions to test
> different configurations, combinations, and versions.

It's failing to build on cfbot's AppVeyor setup[1].  That's currently
using Windows SDK 7.1, so too old for the new AF_UNIX sockets, but
presumably something is wrong because it shouldn't fail to compile and
link.

src/interfaces/libpq/fe-connect.c(2682): warning C4101: 'pwdbuf' :
unreferenced local variable [C:\projects\postgresql\libpq.vcxproj]
src/interfaces/libpq/fe-connect.c(2687): warning C4101: 'passerr' :
unreferenced local variable [C:\projects\postgresql\libpq.vcxproj]

fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
referenced in function PQconnectPoll
[C:\projects\postgresql\libpq.vcxproj]

[1] https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.55034?fullLog=true

-- 
Thomas Munro
https://enterprisedb.com

On 2019-09-03 01:45, Thomas Munro wrote:
> fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
> referenced in function PQconnectPoll
> [C:\projects\postgresql\libpq.vcxproj]

This should be fixed in the attached patches.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-Sep-03, Peter Eisentraut wrote:

> On 2019-09-03 01:45, Thomas Munro wrote:
> > fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
> > referenced in function PQconnectPoll
> > [C:\projects\postgresql\libpq.vcxproj]
> 
> This should be fixed in the attached patches.

Minor bitrot in MSVC script; probably trivial to resolve.

I think you should get 0001 (+0002?) pushed and see what the buildfarm
thinks; move forward from there.  0003+0004 sound like they should just
be pushed shortly afterwards, while the three remaining ones might need
some more careful review.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-Sep-06, Alvaro Herrera from 2ndQuadrant wrote:

> I think you should get 0001 (+0002?) pushed and see what the buildfarm
> thinks; move forward from there.

... but of course this goes counter to what you said earlier about 0001
needing to be pushed last.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

To move this topic a long, I'll submit some preparatory patches in a 
committable order.

First is the patch to deal with getpeereid() that was already included 
in the previous patch series.  This is just some refactoring that 
reduces the difference between Windows and other platforms and prepares 
the Unix-domain socket specific code to compile cleanly on Windows.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Oct 30, 2019 at 10:32 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> To move this topic a long, I'll submit some preparatory patches in a
> committable order.
>
> First is the patch to deal with getpeereid() that was already included
> in the previous patch series.  This is just some refactoring that
> reduces the difference between Windows and other platforms and prepares
> the Unix-domain socket specific code to compile cleanly on Windows.
>


This looks fairly sane and straightforward. Let's give it an outing on
the buildfarm ASAP so we can keep moving forward on this.

cheers

andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-12-16 05:39, Andrew Dunstan wrote:
> On Wed, Oct 30, 2019 at 10:32 PM Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>>
>> To move this topic a long, I'll submit some preparatory patches in a
>> committable order.
>>
>> First is the patch to deal with getpeereid() that was already included
>> in the previous patch series.  This is just some refactoring that
>> reduces the difference between Windows and other platforms and prepares
>> the Unix-domain socket specific code to compile cleanly on Windows.
>>
> 
> 
> This looks fairly sane and straightforward. Let's give it an outing on
> the buildfarm ASAP so we can keep moving forward on this.

pushed

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Next patch:  This allows building *with* Unix-domain socket support but 
*without* a default Unix socket path.  This is needed because on Windows 
we don't have a good default location like "/tmp" and we probably don't 
want Unix sockets by default at run time so that older Windows versions 
continue to work out of the box with the same binaries.

We have code paths for Unix socket support and no Unix socket support. 
Now add a third variant: Unix socket support but do not use a Unix 
socket by default in the client or the server, only if you explicitly
specify one.

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect.  This mostly already worked like that;
only a few places needed to be adjusted.  Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Dec 18, 2019 at 02:52:15PM +0100, Peter Eisentraut wrote:
> To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
> to "" has the desired effect.  This mostly already worked like that;
> only a few places needed to be adjusted.  Notably, the reference to
> DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
> callers already resolve an empty socket directory setting with a
> default if appropriate.

Would it make sense to support abstract sockets in PostgreSQL?

I know it's bit unrelated.  I haven't read all the code here I just was
thinking about it because of the code checking the leading \0 byte of the dir.

Garick

On 2019-12-18 15:24, Hamlin, Garick L wrote:
> On Wed, Dec 18, 2019 at 02:52:15PM +0100, Peter Eisentraut wrote:
>> To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
>> to "" has the desired effect.  This mostly already worked like that;
>> only a few places needed to be adjusted.  Notably, the reference to
>> DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
>> callers already resolve an empty socket directory setting with a
>> default if appropriate.
> 
> Would it make sense to support abstract sockets in PostgreSQL?

Maybe, I'm not sure.

> I know it's bit unrelated.  I haven't read all the code here I just was
> thinking about it because of the code checking the leading \0 byte of the dir.

We would probably represent abstract sockets with a leading '@' in the 
user-facing components and only translate it to the internal format at 
the last moment, probably in that very same UNIXSOCK_PATH() function. 
So I think that wouldn't be a problem.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-12-18 14:52, Peter Eisentraut wrote:
> Next patch:  This allows building *with* Unix-domain socket support but
> *without* a default Unix socket path.  This is needed because on Windows
> we don't have a good default location like "/tmp" and we probably don't
> want Unix sockets by default at run time so that older Windows versions
> continue to work out of the box with the same binaries.
> 
> We have code paths for Unix socket support and no Unix socket support.
> Now add a third variant: Unix socket support but do not use a Unix
> socket by default in the client or the server, only if you explicitly
> specify one.
> 
> To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
> to "" has the desired effect.  This mostly already worked like that;
> only a few places needed to be adjusted.  Notably, the reference to
> DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
> callers already resolve an empty socket directory setting with a
> default if appropriate.

Perhaps this patch is too boring to be reviewed.  If there are no 
objections, I'll commit it soon and then submit the final patches with 
the real functionality for the next commit fest.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 2019-12-18 14:52, Peter Eisentraut wrote:
>> We have code paths for Unix socket support and no Unix socket support.
>> Now add a third variant: Unix socket support but do not use a Unix
>> socket by default in the client or the server, only if you explicitly
>> specify one.
>> 
>> To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
>> to "" has the desired effect.  This mostly already worked like that;
>> only a few places needed to be adjusted.  Notably, the reference to
>> DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
>> callers already resolve an empty socket directory setting with a
>> default if appropriate.

> Perhaps this patch is too boring to be reviewed.  If there are no 
> objections, I'll commit it soon and then submit the final patches with 
> the real functionality for the next commit fest.

Sorry, I'd paid no particular attention to this thread because
I figured it'd take a Windows-competent person to review.  But
the patch as it stands isn't that.

The code looks fine (and a big +1 for not having knowledge of
DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH).  I wonder though
whether any user-facing documentation needs to be adjusted.

            regards, tom lane

On Fri, 31 Jan 2020 at 02:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > On 2019-12-18 14:52, Peter Eisentraut wrote:
> >> We have code paths for Unix socket support and no Unix socket support.
> >> Now add a third variant: Unix socket support but do not use a Unix
> >> socket by default in the client or the server, only if you explicitly
> >> specify one.
> >>
> >> To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
> >> to "" has the desired effect.  This mostly already worked like that;
> >> only a few places needed to be adjusted.  Notably, the reference to
> >> DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
> >> callers already resolve an empty socket directory setting with a
> >> default if appropriate.
>
> > Perhaps this patch is too boring to be reviewed.  If there are no
> > objections, I'll commit it soon and then submit the final patches with
> > the real functionality for the next commit fest.
>
> Sorry, I'd paid no particular attention to this thread because
> I figured it'd take a Windows-competent person to review.  But
> the patch as it stands isn't that.
>
> The code looks fine (and a big +1 for not having knowledge of
> DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH).  I wonder though
> whether any user-facing documentation needs to be adjusted.

Probably, since it won't work with 'peer' auth from what was said upthread.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

On 2020-01-30 19:41, Tom Lane wrote:
> The code looks fine (and a big +1 for not having knowledge of
> DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH).  I wonder though
> whether any user-facing documentation needs to be adjusted.

There are no user-facing changes in this patch yet.  That will come with 
subsequent patches.

This patch has now been committed.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Here is another patch set to enable this functionality.

0001 enables Unix-domain sockets on Windows, but leaves them turned off 
by default at run time, using the mechanism introduced by a9cff89f7e. 
This is relatively straightforward, except perhaps some aesthetic 
questions about how these different configuration bits are distributed 
around the various files.

0002 deals with pg_upgrade.  It preserves the existing behavior of not 
using Unix-domain sockets on Windows.  This could perhaps be enhanced 
later by either adding a command-line option or a run-time test.  It's 
too complicated right now.

0003 deals with how initdb should initialize postgresql.conf and 
pg_hba.conf.  It introduces a run-time test similar to how we detect 
presence of IPv6.  After I wrote this patch, I have come to think that 
this is overkill and we should just always leave the "local" line in 
pg_hba.conf even if there is no run-time support in the OS.  (I think 
the reason we do the run-time test for IPv6 is that we need it to parse 
the IPv6 addresses in pg_hba.conf, but there is no analogous requirement 
for Unix-domain sockets.)  This patch is optional in any case.

0004 fixes a bug in the pg_upgrade test.sh script that was exposed by 
these changes.

0005 fixes up some issues in the test suites.  Right now, the TAP tests 
are hardcoded to not use Unix-domain sockets on Windows, where as 
pg_regress keys off HAVE_UNIX_SOCKETS, which is no longer a useful 
distinguisher.  The change is to not use Unix-domain sockets for all the 
tests by default on Windows (the previous behavior) but give an option 
to use them.  At the moment, I would consider running the test suites 
with Unix-domain sockets enabled as experimental, but that's only 
because of various issues in the test setups.  For instance, there is an 
issue in the comment of pg_regress.c remove_temp() that I'm not sure how 
to address.  Also, the TAP tests don't seem to work because of some path 
issues.  I figured I'd call time on fiddling with this for now and ship 
the patches.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/12/20 3:32 AM, Peter Eisentraut wrote:
> Here is another patch set to enable this functionality.
>
> 0001 enables Unix-domain sockets on Windows, but leaves them turned
> off by default at run time, using the mechanism introduced by
> a9cff89f7e. This is relatively straightforward, except perhaps some
> aesthetic questions about how these different configuration bits are
> distributed around the various files.
>
> 0002 deals with pg_upgrade.  It preserves the existing behavior of not
> using Unix-domain sockets on Windows.  This could perhaps be enhanced
> later by either adding a command-line option or a run-time test.  It's
> too complicated right now.
>
> 0003 deals with how initdb should initialize postgresql.conf and
> pg_hba.conf.  It introduces a run-time test similar to how we detect
> presence of IPv6.  After I wrote this patch, I have come to think that
> this is overkill and we should just always leave the "local" line in
> pg_hba.conf even if there is no run-time support in the OS.  (I think
> the reason we do the run-time test for IPv6 is that we need it to
> parse the IPv6 addresses in pg_hba.conf, but there is no analogous
> requirement for Unix-domain sockets.)  This patch is optional in any
> case.
>
> 0004 fixes a bug in the pg_upgrade test.sh script that was exposed by
> these changes.
>
> 0005 fixes up some issues in the test suites.  Right now, the TAP
> tests are hardcoded to not use Unix-domain sockets on Windows, where
> as pg_regress keys off HAVE_UNIX_SOCKETS, which is no longer a useful
> distinguisher.  The change is to not use Unix-domain sockets for all
> the tests by default on Windows (the previous behavior) but give an
> option to use them.  At the moment, I would consider running the test
> suites with Unix-domain sockets enabled as experimental, but that's
> only because of various issues in the test setups.  For instance,
> there is an issue in the comment of pg_regress.c remove_temp() that
> I'm not sure how to address.  Also, the TAP tests don't seem to work
> because of some path issues.  I figured I'd call time on fiddling with
> this for now and ship the patches.
>

I have tested this on drongo/fairywren and it works fine. The patches
apply cleanly (with a bit of fuzz) and a full buildfarm run is happy in
both cases.

Unfortunately I don't have a Windows machine that's young enough to
support git master and old enough not to support Unix Domain sockets, so
i can't test that with socket-enabled binaries.

On inspection the patches seem fine.

Let's commit this and keep working on the pg_upgrade and test issues.


cheers


andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-03-27 18:52, Andrew Dunstan wrote:
> I have tested this on drongo/fairywren and it works fine. The patches
> apply cleanly (with a bit of fuzz) and a full buildfarm run is happy in
> both cases.
> 
> Unfortunately I don't have a Windows machine that's young enough to
> support git master and old enough not to support Unix Domain sockets, so
> i can't test that with socket-enabled binaries.
> 
> On inspection the patches seem fine.
> 
> Let's commit this and keep working on the pg_upgrade and test issues.

I have committed this in chunks over the last couple of days.  It's done 
now.

I didn't commit the initdb auto-detection patch.  As I mentioned 
earlier, that one is probably not necessary.

Btw., the default AppVeyor images are too old to support this.  You need 
something like 'image: Visual Studio 2019' to get a new enough image. 
So that's one way to test what happens when it's not supported at run 
time.  (I did test it and you get a sensible error message.)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services