Re: BackgroundPsql swallowing errors on windows

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Noah Misch <noah(at)leadboat(dot)com>
Subject: Re: BackgroundPsql swallowing errors on windows
Date: 2026-06-15 13:43:35
Message-ID: aa1bfc94-bd50-4b40-aec0-77eb533260a0@dunslane.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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

Attachment Content-Type Size
v16-0001-Add-PostgreSQL-Test-Session-for-libpq-based-TAP-.patch text/x-patch 61.7 KB
v16-0002-Use-PostgreSQL-Test-Session-within-the-TAP-test-.patch text/x-patch 8.0 KB
v16-0003-Convert-TAP-tests-to-PostgreSQL-Test-Session.patch text/x-patch 175.3 KB
v16-0004-ci-make-the-TAP-test-jobs-work-with-PostgreSQL-T.patch text/x-patch 5.4 KB
v16-0005-Check-for-FFI-Platypus-when-TAP-tests-are-enable.patch text/x-patch 6.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2026-06-15 13:45:22 Re: [Patch] Fix pg_upgrade/t/007_multixact_conversion.pl
Previous Message Ayush Tiwari 2026-06-15 13:41:51 Re: pg_recvlogical: send final feedback on SIGINT/SIGTERM exit