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:-)
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:-(
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.
--
Fabien.
