From: Andrew Dunstan <andrew@dunslane.net>
To: Fabien COELHO <coelho@cri.ensmp.fr>
To: Gregory Smith <gregsmithpgsql@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: david.christensen@crunchydata.com
Subject: Re: pgbench logging broken by time logic changes
Date: Wed, 16 Jun 2021 15:13:30 -0400
Message-ID: <e05abaab-7bd2-93ed-eeca-3f3ceb542506@dunslane.net> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2106162032110.2407635@pseudo>
References: <CAHLJuCW_8Vpcr0=t6O_gozrg3wXXWXZXDioYJd3NhvKriqgpfQ@mail.gmail.com>
<alpine.DEB.2.22.394.2106162032110.2407635@pseudo>
On 6/16/21 2:59 PM, Fabien COELHO wrote:
>
> Hello Greg,
>
>> I have a lot of community oriented work backed up behind this right
>> now, so
>> I'm gonna be really honest. This time rework commit in its current form
>> makes me uncomfortable at this point in the release schedule. The
>> commit
>> has already fought through two rounds of platform specific bug
>> fixes. But
>> since the buildfarm doesn't test the logging feature, that whole
>> process is
>> suspect.
>
> Logging is/should going to be fixed.
>
>> My take on the PostgreSQL way to proceed: this bug exposes that pgbench
>> logging is a feature we finally need to design testing for.
>
> Sure.
>
> The key feedback for me is the usual one: what is not tested does not
> work. Wow:-)
Agreed.
>
> I'm unhappy because I already added tap tests for time-sensitive
> features (-T and others, maybe logging aggregates, cannot remember),
> which have been removed because they could fail under some
> circonstances (eg very very very very slow hosts), or required some
> special handling (a few lines of code) in pgbench, and the net result
> of this is there is not a single test in place for some features:-(
I'm not familiar with exactly what happened in this case, but tests need
to be resilient over a wide range of performance characteristics. One
way around this issue might be to have a way of detecting that it's on a
slow platform and if so either skipping tests (Test::More provides
plenty of support for this) or expecting different results.
>
> There is no problem with proposing tests, the problem is that they are
> accepted, or if they are accepted then that they are not removed at
> the first small issue but rather fixed, or their limitations accepted,
> because testing time-sensitive features is not as simple as testing
> functional features.
>
> Note that currently there is not a single test of psql with autocommit
> off or with "on error rollback". Last time a submitted tap tests to
> raise psql test coverage from 50% to 90%, it was rejected. I'll admit
> that I'm tired arguing that more testing is required, and I'm very
> happy if someone else is ready to try again. Good luck! :-)
:-(
>
>> We need a new buildfarm test and then a march through a full release
>> phase to see how it goes.
>
>> Only then should we start messing with the time logic. Even if we
>> fixed the source today on both my test platforms, I'd still be
>> nervous that beta 2 could ship and more performance testing could
>> fall over from this modification. And that's cutting things a little
>> close.
>
> Well, the point beta is to discover bugs not caught by reviews and dev
> tests, fix them, and possibly add tests which would have caught them.
>
> If you revert all features on the first issue in a corner case and put
> it back to the queue, then I do not see why the review and dev tests
> will be much better on the next round, so it does not really help
> moving things forward.
>
> IMHO, the pragmatic approach is to look at fixing first, and maybe
> revert if the problems are deep. I'm not sure this is obviously the
> case here.
It does look like the submitted fix basically reverts the changes w.r.t.
this timestamp logging.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andrew@dunslane.net, coelho@cri.ensmp.fr, gregsmithpgsql@gmail.com, david.christensen@crunchydata.com
Subject: Re: pgbench logging broken by time logic changes
In-Reply-To: <e05abaab-7bd2-93ed-eeca-3f3ceb542506@dunslane.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox