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.

Reply via email to