Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

18 февраля в 00:31:02


On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
> Hi,
>
> On 2026-02-16 16:38:02 -0500, Andrew Dunstan wrote:
>> Here we are almost exactly a year later. I returned to this work quite
>> recently, and reached a milestone, namely the removal of all calls to
>> BackgroundPsql, with all the TAP tests passing. The XS module still has some
>> problems, and I think I'm inclined not to pursue it, and just rely on the
>> FFI mapping.
>>
>> The current state of this is attached. People who are interested can hit me
>> up for info in Vancouver if not before. I'll work on turning this into a set
>> of commitable patches.
> Nice progress!
>
> I briefly tried this out. The overall resource usage of the test is noticeably
> reduced - and that's on linux with fast fork, so it should be considerably
> better on windows.  However, the tests take a lot longer than before, I think
> mostly due to polling for results rather than waiting for them to be ready
> using PQsocketPoll() or such.
>
> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with the patch. I
> think that's just due to the various usleep(100_000);
>
>
> FWIW, oauth_validator/001_server fails with the patch at the moment.
>


Try this version. On my machine it's now a few percent faster. I fixed 
the polling. I also added pipeline support for large sets of commands, 
to minimize roundtrips.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

18 февраля в 22:41:51


On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
> Hi,
>
> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>> I briefly tried this out. The overall resource usage of the test is noticeably
>>> reduced - and that's on linux with fast fork, so it should be considerably
>>> better on windows.  However, the tests take a lot longer than before, I think
>>> mostly due to polling for results rather than waiting for them to be ready
>>> using PQsocketPoll() or such.
>>>
>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with the patch. I
>>> think that's just due to the various usleep(100_000);
>>>
>>>
>>> FWIW, oauth_validator/001_server fails with the patch at the moment.
>>>
>> Try this version. On my machine it's now a few percent faster. I fixed the
>> polling. I also added pipeline support for large sets of commands, to
>> minimize roundtrips.
> Nice!  Will try it out.
>
>
> Have you tried it on windows already? That's where we pay by far the biggest
> price due to all the unnecessary process creations...
>
> It looks like strawberry perl has FFI::Platypus, but not FFI::C.  There is
> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like it's
> documentation.  There is however FFI::Platypus::Record, which maybe could
> suffice?
>
> Do we actually need FFI::C, or can we work around not having it? Looks like
> it's just used for notify related stuff.
>
> It looks like mingw doesn't have packages for FFI::Platypus, but it'll
> probably be a lot easier to build that than when using msvc.
>
>

I replaced the use of FFI::C with FFI::Platypus::Record. That comes for 
free with FFI::Platypus, so there would be no extra dependency. It means 
a little extra housekeeping so we don't lose track of the pointer for 
later use with PQfreemem, but it's not too bad.

I have tried it out with Windows, seemed to work OK although the 
xid_wraparound tests 2 and 3 timed out.

Latest is attached.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andres Freund <andres@anarazel.de>

Дата:

14 февраля 2025 г. в 21:52:19

Hi,

On 2025-02-14 12:14:55 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2025-02-14 08:14:45 -0500, Andrew Dunstan wrote:
> >> ... If there is interest I will
> >> bring the work up to date, and maybe we can introduce this early in the v19
> >> cycle. It would significantly reduce our dependency on IPC::Run, especially
> >> the pump() stuff.
> 
> > I definitely am interested.
> 
> +1.  Would there be a chance of removing our use of IPC::Run entirely?
> There'd be a lot to like about that.

Unfortunately I doubt it'd get us that close to that.

We do have several tests that intentionally involve psql,
e.g. 010_tab_completion.pl, 001_password.pl. Presumably those would continue
to use something like BackgroundPsql.pm, even if we had a sane way to have
longrunning connections in tap tests.

There also are a bunch of things we use IPC::Run to run synchronously, we
probably could replace most of those without too much pain. The hardest
probably would be timeout handling.

Greetings,

Andres Freund

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

4 июня в 00:23:30


On 2026-06-02 Tu 3:03 PM, Andrew Dunstan wrote:
>
> On 2026-02-18 We 2:41 PM, Andrew Dunstan wrote:
>>
>> On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
>>> Hi,
>>>
>>> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>>>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>>>> I briefly tried this out. The overall resource usage of the test 
>>>>> is noticeably
>>>>> reduced - and that's on linux with fast fork, so it should be 
>>>>> considerably
>>>>> better on windows.  However, the tests take a lot longer than 
>>>>> before, I think
>>>>> mostly due to polling for results rather than waiting for them to 
>>>>> be ready
>>>>> using PQsocketPoll() or such.
>>>>>
>>>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with 
>>>>> the patch. I
>>>>> think that's just due to the various usleep(100_000);
>>>>>
>>>>>
>>>>> FWIW, oauth_validator/001_server fails with the patch at the moment.
>>>>>
>>>> Try this version. On my machine it's now a few percent faster. I 
>>>> fixed the
>>>> polling. I also added pipeline support for large sets of commands, to
>>>> minimize roundtrips.
>>> Nice!  Will try it out.
>>>
>>>
>>> Have you tried it on windows already? That's where we pay by far the 
>>> biggest
>>> price due to all the unnecessary process creations...
>>>
>>> It looks like strawberry perl has FFI::Platypus, but not FFI::C.  
>>> There is
>>> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like it's
>>> documentation.  There is however FFI::Platypus::Record, which maybe 
>>> could
>>> suffice?
>>>
>>> Do we actually need FFI::C, or can we work around not having it? 
>>> Looks like
>>> it's just used for notify related stuff.
>>>
>>> It looks like mingw doesn't have packages for FFI::Platypus, but it'll
>>> probably be a lot easier to build that than when using msvc.
>>>
>>>
>>
>> I replaced the use of FFI::C with FFI::Platypus::Record. That comes 
>> for free with FFI::Platypus, so there would be no extra dependency. 
>> It means a little extra housekeeping so we don't lose track of the 
>> pointer for later use with PQfreemem, but it's not too bad.
>>
>> I have tried it out with Windows, seemed to work OK although the 
>> xid_wraparound tests 2 and 3 timed out.
>>
>> Latest is attached.
>>
>>
>>
>
> Here is v12. I removed the XS variant in this version, which makes 
> things simpler. We can restore it if necessary.
>
> Patch 1 adds the new framework
>
> Patch 2 adapts Cluster.pm to it, as well as handling some instability 
> at global destruction time that was exacerbated by using FFI::Platypus.
>
> Patch 3 makes improvements in the individual TAP tests using the 
> framework, including removing every one of the calls to 
> background_psql().
>
>
> I'm going to add this to the CF and will start testing (again) on 
> Windows.
>
>
>

v13 attached now passes all tests on my Windows machine.


cheers


andrew




--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Tom Lane <tgl@sss.pgh.pa.us>

Дата:

14 февраля 2025 г. в 20:14:55

Andres Freund  writes:
> On 2025-02-14 08:14:45 -0500, Andrew Dunstan wrote:
>> ... If there is interest I will
>> bring the work up to date, and maybe we can introduce this early in the v19
>> cycle. It would significantly reduce our dependency on IPC::Run, especially
>> the pump() stuff.

> I definitely am interested.

+1.  Would there be a chance of removing our use of IPC::Run entirely?
There'd be a lot to like about that.

			regards, tom lane

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

13 июня в 01:39:38


On 2026-06-10 We 4:23 PM, Andrew Dunstan wrote:
>
> On 2026-06-03 We 5:23 PM, Andrew Dunstan wrote:
>>
>> On 2026-06-02 Tu 3:03 PM, Andrew Dunstan wrote:
>>>
>>> On 2026-02-18 We 2:41 PM, Andrew Dunstan wrote:
>>>>
>>>> On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
>>>>> Hi,
>>>>>
>>>>> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>>>>>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>>>>>> I briefly tried this out. The overall resource usage of the test 
>>>>>>> is noticeably
>>>>>>> reduced - and that's on linux with fast fork, so it should be 
>>>>>>> considerably
>>>>>>> better on windows.  However, the tests take a lot longer than 
>>>>>>> before, I think
>>>>>>> mostly due to polling for results rather than waiting for them 
>>>>>>> to be ready
>>>>>>> using PQsocketPoll() or such.
>>>>>>>
>>>>>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with 
>>>>>>> the patch. I
>>>>>>> think that's just due to the various usleep(100_000);
>>>>>>>
>>>>>>>
>>>>>>> FWIW, oauth_validator/001_server fails with the patch at the 
>>>>>>> moment.
>>>>>>>
>>>>>> Try this version. On my machine it's now a few percent faster. I 
>>>>>> fixed the
>>>>>> polling. I also added pipeline support for large sets of 
>>>>>> commands, to
>>>>>> minimize roundtrips.
>>>>> Nice!  Will try it out.
>>>>>
>>>>>
>>>>> Have you tried it on windows already? That's where we pay by far 
>>>>> the biggest
>>>>> price due to all the unnecessary process creations...
>>>>>
>>>>> It looks like strawberry perl has FFI::Platypus, but not FFI::C.  
>>>>> There is
>>>>> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like it's
>>>>> documentation.  There is however FFI::Platypus::Record, which 
>>>>> maybe could
>>>>> suffice?
>>>>>
>>>>> Do we actually need FFI::C, or can we work around not having it? 
>>>>> Looks like
>>>>> it's just used for notify related stuff.
>>>>>
>>>>> It looks like mingw doesn't have packages for FFI::Platypus, but 
>>>>> it'll
>>>>> probably be a lot easier to build that than when using msvc.
>>>>>
>>>>>
>>>>
>>>> I replaced the use of FFI::C with FFI::Platypus::Record. That comes 
>>>> for free with FFI::Platypus, so there would be no extra dependency. 
>>>> It means a little extra housekeeping so we don't lose track of the 
>>>> pointer for later use with PQfreemem, but it's not too bad.
>>>>
>>>> I have tried it out with Windows, seemed to work OK although the 
>>>> xid_wraparound tests 2 and 3 timed out.
>>>>
>>>> Latest is attached.
>>>>
>>>>
>>>>
>>>
>>> Here is v12. I removed the XS variant in this version, which makes 
>>> things simpler. We can restore it if necessary.
>>>
>>> Patch 1 adds the new framework
>>>
>>> Patch 2 adapts Cluster.pm to it, as well as handling some 
>>> instability at global destruction time that was exacerbated by using 
>>> FFI::Platypus.
>>>
>>> Patch 3 makes improvements in the individual TAP tests using the 
>>> framework, including removing every one of the calls to 
>>> background_psql().
>>>
>>>
>>> I'm going to add this to the CF and will start testing (again) on 
>>> Windows.
>>>
>>>
>>>
>>
>> v13 attached now passes all tests on my Windows machine.
>>
>>
>>
>
> rebased, including porting a new use of background_psql.
>
>
>

v15 including a check for FFI::Platypus at setup time, and CI 
modifications to allow tests to pass.


cheers


andrew





--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andres Freund <andres@anarazel.de>

Дата:

17 февраля в 03:17:48

Hi,

On 2026-02-16 16:38:02 -0500, Andrew Dunstan wrote:
> Here we are almost exactly a year later. I returned to this work quite
> recently, and reached a milestone, namely the removal of all calls to
> BackgroundPsql, with all the TAP tests passing. The XS module still has some
> problems, and I think I'm inclined not to pursue it, and just rely on the
> FFI mapping.
> 
> The current state of this is attached. People who are interested can hit me
> up for info in Vancouver if not before. I'll work on turning this into a set
> of commitable patches.

Nice progress!

I briefly tried this out. The overall resource usage of the test is noticeably
reduced - and that's on linux with fast fork, so it should be considerably
better on windows.  However, the tests take a lot longer than before, I think
mostly due to polling for results rather than waiting for them to be ready
using PQsocketPoll() or such.

E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with the patch. I
think that's just due to the various usleep(100_000);

FWIW, oauth_validator/001_server fails with the patch at the moment.

Greetings,

Andres Freund

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

18 июня в 15:17:47


On 2026-06-15 Mo 9:43 AM, Andrew Dunstan wrote:
>
> On 2026-06-12 Fr 6:39 PM, Andrew Dunstan wrote:
>>
>> On 2026-06-10 We 4:23 PM, Andrew Dunstan wrote:
>>>
>>> On 2026-06-03 We 5:23 PM, Andrew Dunstan wrote:
>>>>
>>>> On 2026-06-02 Tu 3:03 PM, Andrew Dunstan wrote:
>>>>>
>>>>> On 2026-02-18 We 2:41 PM, Andrew Dunstan wrote:
>>>>>>
>>>>>> On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>>>>>>>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>>>>>>>> I briefly tried this out. The overall resource usage of the 
>>>>>>>>> test is noticeably
>>>>>>>>> reduced - and that's on linux with fast fork, so it should be 
>>>>>>>>> considerably
>>>>>>>>> better on windows.  However, the tests take a lot longer than 
>>>>>>>>> before, I think
>>>>>>>>> mostly due to polling for results rather than waiting for them 
>>>>>>>>> to be ready
>>>>>>>>> using PQsocketPoll() or such.
>>>>>>>>>
>>>>>>>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s 
>>>>>>>>> with the patch. I
>>>>>>>>> think that's just due to the various usleep(100_000);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> FWIW, oauth_validator/001_server fails with the patch at the 
>>>>>>>>> moment.
>>>>>>>>>
>>>>>>>> Try this version. On my machine it's now a few percent faster. 
>>>>>>>> I fixed the
>>>>>>>> polling. I also added pipeline support for large sets of 
>>>>>>>> commands, to
>>>>>>>> minimize roundtrips.
>>>>>>> Nice!  Will try it out.
>>>>>>>
>>>>>>>
>>>>>>> Have you tried it on windows already? That's where we pay by far 
>>>>>>> the biggest
>>>>>>> price due to all the unnecessary process creations...
>>>>>>>
>>>>>>> It looks like strawberry perl has FFI::Platypus, but not 
>>>>>>> FFI::C.  There is
>>>>>>> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like 
>>>>>>> it's
>>>>>>> documentation.  There is however FFI::Platypus::Record, which 
>>>>>>> maybe could
>>>>>>> suffice?
>>>>>>>
>>>>>>> Do we actually need FFI::C, or can we work around not having it? 
>>>>>>> Looks like
>>>>>>> it's just used for notify related stuff.
>>>>>>>
>>>>>>> It looks like mingw doesn't have packages for FFI::Platypus, but 
>>>>>>> it'll
>>>>>>> probably be a lot easier to build that than when using msvc.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> I replaced the use of FFI::C with FFI::Platypus::Record. That 
>>>>>> comes for free with FFI::Platypus, so there would be no extra 
>>>>>> dependency. It means a little extra housekeeping so we don't lose 
>>>>>> track of the pointer for later use with PQfreemem, but it's not 
>>>>>> too bad.
>>>>>>
>>>>>> I have tried it out with Windows, seemed to work OK although the 
>>>>>> xid_wraparound tests 2 and 3 timed out.
>>>>>>
>>>>>> Latest is attached.
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Here is v12. I removed the XS variant in this version, which makes 
>>>>> things simpler. We can restore it if necessary.
>>>>>
>>>>> Patch 1 adds the new framework
>>>>>
>>>>> Patch 2 adapts Cluster.pm to it, as well as handling some 
>>>>> instability at global destruction time that was exacerbated by 
>>>>> using FFI::Platypus.
>>>>>
>>>>> Patch 3 makes improvements in the individual TAP tests using the 
>>>>> framework, including removing every one of the calls to 
>>>>> background_psql().
>>>>>
>>>>>
>>>>> I'm going to add this to the CF and will start testing (again) on 
>>>>> Windows.
>>>>>
>>>>>
>>>>>
>>>>
>>>> v13 attached now passes all tests on my Windows machine.
>>>>
>>>>
>>>>
>>>
>>> rebased, including porting a new use of background_psql.
>>>
>>>
>>>
>>
>> v15 including a check for FFI::Platypus at setup time, and CI 
>> modifications to allow tests to pass.
>>
>>
>>
>
>
> v16 follows several rounds of review, and tightens up a lot of things, 
> so now errors don't silently disappear, or failing tests hang where 
> previously they would time out. There has also been some code cleanup, 
> removal of magic numbers (you can now check for CONNECTION_OK for 
> example), removal of some dead code.
>


In view of discussions on 
https://www.postgresql.org/message-id/cdaaf722-4529-435b-9340-cedf1a3a277f%40dunslane.net 
I have withdrawn the CF item.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

17 февраля в 00:38:02


On 2025-02-14 Fr 11:54 AM, Andres Freund wrote:
> Hi,
>
> On 2025-02-14 08:14:45 -0500, Andrew Dunstan wrote:
>> It's been discussed before, but I'd really really like to get rid of
>> BackgroundPsql. It's ugly, non-intuitive and fragile.
> I agree, unfortunately we're stuck with this until we have a better
> alternative in tree :(
>
>
>> Last year I did some work on this. I was going to present it at Athens but
>> illness prevented me, and then other life events managed to get in my way.
>> But the basic work is around. See 
>> This introduces a libpq session object (PostgreSQL::Test::Session) which can
>> be backed either by FFI or a small XS wrapper - the commit has recipes for
>> both. Missing is a meson.build file for the XS wrapper. There are
>> significant performance gains to be had too (poll_query_until is much nicer,
>> for example, as are most uses of safe_psql). If there is  interest I will
>> bring the work up to date, and maybe we can introduce this early in the v19
>> cycle. It would significantly reduce our dependency on IPC::Run, especially
>> the pump() stuff.
> I definitely am interested.


Here we are almost exactly a year later. I returned to this work quite 
recently, and reached a milestone, namely the removal of all calls to 
BackgroundPsql, with all the TAP tests passing. The XS module still has 
some problems, and I think I'm inclined not to pursue it, and just rely 
on the FFI mapping.

The current state of this is attached. People who are interested can hit 
me up for info in Vancouver if not before. I'll work on turning this 
into a set of commitable patches.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

15 июня в 16:43:35


On 2026-06-12 Fr 6:39 PM, Andrew Dunstan wrote:
>
> On 2026-06-10 We 4:23 PM, Andrew Dunstan wrote:
>>
>> On 2026-06-03 We 5:23 PM, Andrew Dunstan wrote:
>>>
>>> On 2026-06-02 Tu 3:03 PM, Andrew Dunstan wrote:
>>>>
>>>> On 2026-02-18 We 2:41 PM, Andrew Dunstan wrote:
>>>>>
>>>>> On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>>>>>>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>>>>>>> I briefly tried this out. The overall resource usage of the 
>>>>>>>> test is noticeably
>>>>>>>> reduced - and that's on linux with fast fork, so it should be 
>>>>>>>> considerably
>>>>>>>> better on windows.  However, the tests take a lot longer than 
>>>>>>>> before, I think
>>>>>>>> mostly due to polling for results rather than waiting for them 
>>>>>>>> to be ready
>>>>>>>> using PQsocketPoll() or such.
>>>>>>>>
>>>>>>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s 
>>>>>>>> with the patch. I
>>>>>>>> think that's just due to the various usleep(100_000);
>>>>>>>>
>>>>>>>>
>>>>>>>> FWIW, oauth_validator/001_server fails with the patch at the 
>>>>>>>> moment.
>>>>>>>>
>>>>>>> Try this version. On my machine it's now a few percent faster. I 
>>>>>>> fixed the
>>>>>>> polling. I also added pipeline support for large sets of 
>>>>>>> commands, to
>>>>>>> minimize roundtrips.
>>>>>> Nice!  Will try it out.
>>>>>>
>>>>>>
>>>>>> Have you tried it on windows already? That's where we pay by far 
>>>>>> the biggest
>>>>>> price due to all the unnecessary process creations...
>>>>>>
>>>>>> It looks like strawberry perl has FFI::Platypus, but not FFI::C.  
>>>>>> There is
>>>>>> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like 
>>>>>> it's
>>>>>> documentation.  There is however FFI::Platypus::Record, which 
>>>>>> maybe could
>>>>>> suffice?
>>>>>>
>>>>>> Do we actually need FFI::C, or can we work around not having it? 
>>>>>> Looks like
>>>>>> it's just used for notify related stuff.
>>>>>>
>>>>>> It looks like mingw doesn't have packages for FFI::Platypus, but 
>>>>>> it'll
>>>>>> probably be a lot easier to build that than when using msvc.
>>>>>>
>>>>>>
>>>>>
>>>>> I replaced the use of FFI::C with FFI::Platypus::Record. That 
>>>>> comes for free with FFI::Platypus, so there would be no extra 
>>>>> dependency. It means a little extra housekeeping so we don't lose 
>>>>> track of the pointer for later use with PQfreemem, but it's not 
>>>>> too bad.
>>>>>
>>>>> I have tried it out with Windows, seemed to work OK although the 
>>>>> xid_wraparound tests 2 and 3 timed out.
>>>>>
>>>>> Latest is attached.
>>>>>
>>>>>
>>>>>
>>>>
>>>> Here is v12. I removed the XS variant in this version, which makes 
>>>> things simpler. We can restore it if necessary.
>>>>
>>>> Patch 1 adds the new framework
>>>>
>>>> Patch 2 adapts Cluster.pm to it, as well as handling some 
>>>> instability at global destruction time that was exacerbated by 
>>>> using FFI::Platypus.
>>>>
>>>> Patch 3 makes improvements in the individual TAP tests using the 
>>>> framework, including removing every one of the calls to 
>>>> background_psql().
>>>>
>>>>
>>>> I'm going to add this to the CF and will start testing (again) on 
>>>> Windows.
>>>>
>>>>
>>>>
>>>
>>> v13 attached now passes all tests on my Windows machine.
>>>
>>>
>>>
>>
>> rebased, including porting a new use of background_psql.
>>
>>
>>
>
> v15 including a check for FFI::Platypus at setup time, and CI 
> modifications to allow tests to pass.
>
>
>


v16 follows several rounds of review, and tightens up a lot of things, 
so now errors don't silently disappear, or failing tests hang where 
previously they would time out. There has also been some code cleanup, 
removal of magic numbers (you can now check for CONNECTION_OK for 
example), removal of some dead code.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

2 июня в 22:03:57


On 2026-02-18 We 2:41 PM, Andrew Dunstan wrote:
>
> On 2026-02-17 Tu 4:56 PM, Andres Freund wrote:
>> Hi,
>>
>> On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
>>> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
>>>> I briefly tried this out. The overall resource usage of the test is 
>>>> noticeably
>>>> reduced - and that's on linux with fast fork, so it should be 
>>>> considerably
>>>> better on windows.  However, the tests take a lot longer than 
>>>> before, I think
>>>> mostly due to polling for results rather than waiting for them to 
>>>> be ready
>>>> using PQsocketPoll() or such.
>>>>
>>>> E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with 
>>>> the patch. I
>>>> think that's just due to the various usleep(100_000);
>>>>
>>>>
>>>> FWIW, oauth_validator/001_server fails with the patch at the moment.
>>>>
>>> Try this version. On my machine it's now a few percent faster. I 
>>> fixed the
>>> polling. I also added pipeline support for large sets of commands, to
>>> minimize roundtrips.
>> Nice!  Will try it out.
>>
>>
>> Have you tried it on windows already? That's where we pay by far the 
>> biggest
>> price due to all the unnecessary process creations...
>>
>> It looks like strawberry perl has FFI::Platypus, but not FFI::C.  
>> There is
>> perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like it's
>> documentation.  There is however FFI::Platypus::Record, which maybe 
>> could
>> suffice?
>>
>> Do we actually need FFI::C, or can we work around not having it? 
>> Looks like
>> it's just used for notify related stuff.
>>
>> It looks like mingw doesn't have packages for FFI::Platypus, but it'll
>> probably be a lot easier to build that than when using msvc.
>>
>>
>
> I replaced the use of FFI::C with FFI::Platypus::Record. That comes 
> for free with FFI::Platypus, so there would be no extra dependency. It 
> means a little extra housekeeping so we don't lose track of the 
> pointer for later use with PQfreemem, but it's not too bad.
>
> I have tried it out with Windows, seemed to work OK although the 
> xid_wraparound tests 2 and 3 timed out.
>
> Latest is attached.
>
>
>

Here is v12. I removed the XS variant in this version, which makes 
things simpler. We can restore it if necessary.

Patch 1 adds the new framework

Patch 2 adapts Cluster.pm to it, as well as handling some instability at 
global destruction time that was exacerbated by using FFI::Platypus.

Patch 3 makes improvements in the individual TAP tests using the 
framework, including removing every one of the calls to background_psql().


I'm going to add this to the CF and will start testing (again) on Windows.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andrew Dunstan <andrew@dunslane.net>

Дата:

14 февраля 2025 г. в 16:14:45


On 2025-02-13 Th 12:39 PM, Andres Freund wrote:
> Hi,
>
> One of the remaining tasks for AIO was to convert the tests to be proper tap
> tests.  I did that and was thanked by the tests fairly randomly failing on
> windows. Which test fails changes from run to run.
>
> The symptom is that BackgroundPsql->query() sometimes would simply swallow
> errors that were clearly generated by the backend. Which then would cause
> tests to fail, because testing for errors was the whole point of $test.
>
>
> At first I thought the issue was that psql didn't actually flush stderr after
> displaying errors. And while that may be an issue, it doesn't seem to be the
> one causing problems for me.
>
> Lots of hair-pulling later, I have a somewhat confirmed theory for what's
> happening:
>
> BackgroundPsql::query() tries to detect if the passed in query has executed by
> adding a "banner" after the query and using pump_until() to wait until that
> banner has been "reached".  That seems to work reasonably well on !windows.
>
> On windows however, it looks like there's no guarantee that if stdout has been
> received by IPC::Run, stderr also has been received, even if the stderr
> content has been generated first. I tried to add an extra ->pump_nb() call to
> query(), thinking that maybe IPC::Run just didn't get input that had actually
> arrived, due to waiting on just one pipe. But no success.
>
> My understanding is that IPC::Run uses a proxy process on windows to execute
> subprocesses and then communicates with that over TCP (or something along
> those lines).  I suspect what's happening is that the communication with the
> external process allows for reordering between stdout/stderr.
>
> And indeed, changing BackgroundPsql::query() to emit the banner on both stdout
> and stderr and waiting on both seems to fix the issue.
>
>
> One complication is that I found that just waiting for the banner, without
> also its newline, sometimes lead to unexpected newlines causing later queries
> to fail. I think that happens if the trailing newline is read separately from
> the rest of the string.
>
> However, matching the newlines caused tests to fail on some machines. After a
> lot of cursing I figured out that for interactive psql we output \r\n, causing
> my regex match to fail. I.e. tests failed whenever IO::PTY was availble...
>
> It's also not correct, as we did before, to just look for the banner, it has
> to be anchored to either the start of the output or a newline, otherwise the
> \echo (or \warn) command itself will be matched by pump_until() (but then the
> replacing the command would fail). Not sure that could cause active problems
> without the addition of \warn (which is also echoed on stdout), but it
> certainly could after.
>
>
> The banner being the same between queries made it hard to understand if a
> banner that appeared in the output was from the current query or a past
> query. Therefore I added a counter to it.
>
>
> For debugging I added a "note" that shows stdout/stderr after executing the
> query, I think it may be worth keeping that, but I'm not sure.
>
>
> This was a rather painful exercise.
>
>


It's been discussed before, but I'd really really like to get rid of 
BackgroundPsql. It's ugly, non-intuitive and fragile.

Last year I did some work on this. I was going to present it at Athens 
but illness prevented me, and then other life events managed to get in 
my way. But the basic work is around. See 
 
This introduces a libpq session object (PostgreSQL::Test::Session) which 
can be backed either by FFI or a small XS wrapper - the commit has 
recipes for both. Missing is a meson.build file for the XS wrapper. 
There are significant performance gains to be had too (poll_query_until 
is much nicer, for example, as are most uses of safe_psql). If there is  
interest I will bring the work up to date, and maybe we can introduce 
this early in the v19 cycle. It would significantly reduce our 
dependency on IPC::Run, especially the pump() stuff.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: BackgroundPsql swallowing errors on windows

От:

Andres Freund <andres@anarazel.de>

Дата:

14 февраля 2025 г. в 19:54:20

Hi,

On 2025-02-14 08:14:45 -0500, Andrew Dunstan wrote:
> It's been discussed before, but I'd really really like to get rid of
> BackgroundPsql. It's ugly, non-intuitive and fragile.

I agree, unfortunately we're stuck with this until we have a better
alternative in tree :(


> Last year I did some work on this. I was going to present it at Athens but
> illness prevented me, and then other life events managed to get in my way.
> But the basic work is around. See 
> This introduces a libpq session object (PostgreSQL::Test::Session) which can
> be backed either by FFI or a small XS wrapper - the commit has recipes for
> both. Missing is a meson.build file for the XS wrapper. There are
> significant performance gains to be had too (poll_query_until is much nicer,
> for example, as are most uses of safe_psql). If there is  interest I will
> bring the work up to date, and maybe we can introduce this early in the v19
> cycle. It would significantly reduce our dependency on IPC::Run, especially
> the pump() stuff.

I definitely am interested.

Greetings,

Andres Freund

Re: BackgroundPsql swallowing errors on windows

От:

Andres Freund <andres@anarazel.de>

Дата:

18 февраля в 00:56:11

Hi,

On 2026-02-17 16:31:02 -0500, Andrew Dunstan wrote:
> On 2026-02-16 Mo 7:17 PM, Andres Freund wrote:
> > I briefly tried this out. The overall resource usage of the test is noticeably
> > reduced - and that's on linux with fast fork, so it should be considerably
> > better on windows.  However, the tests take a lot longer than before, I think
> > mostly due to polling for results rather than waiting for them to be ready
> > using PQsocketPoll() or such.
> >
> > E.g. bloom/001_wal takes about 15s on HEAD for me, but 138s with the patch. I
> > think that's just due to the various usleep(100_000);
> >
> >
> > FWIW, oauth_validator/001_server fails with the patch at the moment.
> >

> Try this version. On my machine it's now a few percent faster. I fixed the
> polling. I also added pipeline support for large sets of commands, to
> minimize roundtrips.

Nice!  Will try it out.

Have you tried it on windows already? That's where we pay by far the biggest
price due to all the unnecessary process creations...

It looks like strawberry perl has FFI::Platypus, but not FFI::C.  There is
perl/vendor/lib/FFI/Platypus/Lang/C.pm, but that just seems like it's
documentation.  There is however FFI::Platypus::Record, which maybe could
suffice?

Do we actually need FFI::C, or can we work around not having it? Looks like
it's just used for notify related stuff.

It looks like mingw doesn't have packages for FFI::Platypus, but it'll
probably be a lot easier to build that than when using msvc.

Greetings,

Andres Freund