Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)
Date: 2019-02-19 14:56:58
Message-ID: alpine.DEB.2.21.1902191232190.7308@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2019-02-19 15:00:58 Re: unconstify equivalent for volatile
Previous Message Andrew Dunstan 2019-02-19 14:47:31 Re: [Bug Fix] ECPG: could not use set xxx to default statement