Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" testpending solution of its timing is (fwd) - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" testpending solution of its timing is (fwd)
Date
Msg-id alpine.DEB.2.21.1902191232190.7308@lancre
Whole thread Raw
In response to Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" testpending solution of its timing is (fwd)
List pgsql-hackers
Hello Tom,

>>> Unfortunately, there was no activity over the last few commitfests and the
>>> proposed patch pgbench-tap-progress-6 can't be applied anymore without
>>> conflicts. Fabien, what are your plans about it, could you please post a
>>> rebased version?
>
>> Here it is.
>
> I'm confused about the intended scope of this patch.  The thread title
> refers only to adding a regression test, but the actual patch includes
> nontrivial C-code changes, and a skim of the recent discussion suggests
> that there are some bug fixes involved.  Please clarify.

The C changes ensures a minimal deterministic behavior under time 
uncertainties in a test, so that there is always at least one output. 
Since a piece of code is needed twice, a function is created, which also 
improves readability.

> As I think I made clear already, I am not in favor of adding more
> timing-sensitive regression tests here.

Currently there are none in pgbench: they were removed because of very 
slow beasts and intermittent failures that you rightfully dislike.

> I do not think there is value commensurate with the risk of intermittent
> test failures.

The main point of these tests is to provide coverage, which is currently 
pretty abysmal all over postgres. The time aspect is *very* loose: if the 
time target is not met the test is ignored, so there should not be 
intermittent test failures, unless there is a bug, obviously.

> However, if we're fixing bugs or poor behavior, that's certainly worth 
> doing.

Hmmm. The underlying "bug", which can be categorized as a "feature", is 
that when the process cannot run in time then some output values may be 
skipped (eg if you run with a progress report every second for two seconds 
you may not have any progress report, and the process may run for more 
than two seconds because the process does not have enough cpu power to 
notice that it can stop). The code change does not fix these per se, but 
ensure that whatever there is one progress report, so that it can be 
tested in the TAP.

-- 
Fabien.


pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: [Bug Fix] ECPG: could not use set xxx to default statement
Next
From: Peter Eisentraut
Date:
Subject: Re: unconstify equivalent for volatile