> On 23 Aug 2023, at 21:22, Andres Freund <andres@anarazel.de> wrote:
> On 2023-08-23 14:48:26 +0200, Daniel Gustafsson wrote:
>> I'll do another pass, but below are a few small comments so far.
>>
>> I don't know Windows to know the implications, but should the below file have
>> some sort of warning about not doing that for production/shared systems, only
>> for dedicated test instances?
>
> Ah, I should have explained that: I'm not planning to apply
> - regress: Check for postgres startup completion more often
> - ci: windows: Disabling write cache flushing during test
> right now. Compared to the other patches the wins are much smaller and/or more
> work is needed to make them good.
>
> I think it might be worth going for
> - ci: switch tasks to debugoptimized build
> because that provides a fair bit of gain. But it might be more hurtful than
> helpful due to costing more when ccache doesn't work...
Gotcha.
>> +++ b/src/tools/ci/windows_write_cache.ps1
>> @@ -0,0 +1,20 @@
>> +# Define the write cache to be power protected. This reduces the rate of cache
>> +# flushes, which seems to help metadata heavy workloads on NTFS. We're just
>> +# testing here anyway, so ...
>> +#
>> +# Let's do so for all disks, this could be useful beyond cirrus-ci.
>>
>> One thing in 0010 caught my eye, and while not introduced in this patchset it
>> might be of interest here. In the below hunks we loop X ticks around
>> system(psql), with the loop assuming the server can come up really quickly and
>> sleeping if it doesn't. On my systems I always reach the pg_usleep after
>> failing the check, but if I reverse the check such it first sleeps and then
>> checks I only need to check once instead of twice.
>
> I think there's more effective ways to make this cheaper. The basic thing
> would be to use libpq instead of forking of psql to make a connection
> check.
I had it in my head that not using libpq in pg_regress was a deliberate choice,
but I fail to find a reference to it in the archives.
>> It's a micro-optimization, but if we're changing things here to chase cycles it
>> might perhaps be worth doing?
>
> I wouldn't quite call not waiting for 1s for the server to start, when it does
> so within a few ms, chasing cycles ;). For short tests it's a substantial
> fraction of the overall runtime...
Absolutely, I was referring to shifting the sleep before the test to avoid the
extra test, not the reduction of the pg_usleep. Reducing the sleep is a clear
win.
--
Daniel Gustafsson