Hello andres & Tom,

 A bit concerned that we're turning pgbench into a kitchen sink.

I do not understand "kitchen sink" expression in this context, and your
general concerns about pgbench in various comments in your message.

We're adding a lot of stuff to pgbench that only a few people
use. There's a lot of duplication with similar parts of code in other
parts of the codebase. pgbench in my opinion is a tool to facilitate
postgres development, not a goal in itself.

I disagree.

I think that pgbench should *also* allow to test postgres performance in realistic scenarii that allow to communicate about performance, and reassure users about their use case, not just a simplified tpc-b.

Even if you would want to restrict it to internal postgres development, which I would see as a shame, recently added features are still useful.

For instance, I used extensively tps throttling, latencies and timeouts measures when developping and testing the checkpointer sorting & throttling patch.

Some people are just proposing a new storage engine which changes the cost of basic operations (improves committed transaction, makes rolling-back more expensive). What is the actual impact depending on the rollback rate? How do you plan to measure that? Pgbench needs capacities to be useful there, and the good news is that some recently added ones would come handy.

It's a bad measure, but the code growth shows my concerns somewhat:
master:        5660 +817
REL_10_STABLE: 4843 +266
REL9_6_STABLE: 4577 +424
REL9_5_STABLE: 4153 +464
REL9_4_STABLE: 3689 +562
REL9_3_STABLE: 3127 +338
REL9_2_STABLE: 2789 +96
REL9_1_STABLE: 2693

A significant part of this growth is the expression engine, which is mostly trivial code, although alas not necessarily devout of bugs. If moved to fe-utils, pgbench code footprint will be reduced by about 2000 lines.

Also, code has been removed (eg the fork-based implementation) and significant restructuring which has greatly improved code maintenance, even if the number of lines has possibly increased in passing.

So this setting-variable-from-query patch goes with having boolean
expressions (already committed), having conditions (\if in the queue),
improving the available functions (eg hashes, in the queue)... so that
existing, data-dependent, realistic benchmarks can be implemented, and
benefit for the great performance data collection provided by the tool.

I agree that they're useful in a few cases, but they have to consider
that they need to be reviewed and maintained an the project is quite
resource constrained in that regard.

Currently I do most of the reviewing & maintenance of pgbench, apart from the patch I submit.

I can stop doing both if the project decides that improving pgbench capabilities is against its interest.

Tom said:

FWIW, I share Andres' concern that pgbench is being extended far past what anyone has shown a need for. If we had infinite resources
this wouldn't be a big problem, but it's eating into limited
committer hours and I'm not really convinced that we're getting adequate return.

As pgbench patches can stay ready-to-committers for half a dozen CF, I'm not sure the strain on the committer time is that heavy:-) There are not so many of them, most of them are trivial. If you drop them on the ground that the you do not want them, it will not change anything to the lack of reviewing resources and incapacity of the project to process the submitted patches, which in my opinion is a wider issue, not related to the few pgbench-related submissions.

On the "adequate return" point, my opinion is that currently pgbench is just below the feature set needed to be generally usable, so not improving it is a self-fullfilling ensurance that it will not be used further. Once the "right" feature set is reached (for me, at least extracting query output into variables, having conditionals, possibly a few more functions if some benches use them), whether it would be actually more widely used by both developers and users is an open question.

Now, as I said, if pgbench improvements are not seen as desirable, I can mark submissions as "rejected" and do other things with my little available time than try to contribute to postgres.

- pgbench - test whether a variable exists

As already said, the motivation is that it is a preparation for a (much)
larger patch which would move pgbench expressions to fe utils and use them
in "psql".

You could submit it together with that.

Sure, I could. My previous experience is that maintaining a set of dependent patches is tiresome, and it does not help much with testing and reviewing either. So I'm doing things one (slow) step at a time, especially as each time I've submitted patches which were doing more than one thing I was asked to disentangle features and restructuring.

But I don't see in the first place why we need to add the feature with duplicate code, just so we can unify.

It is not duplicate code. In psql the variable-exists-test is currently performed on the fly by the lexer. With the expression engine, it needs to be lexed, parsed and finally evaluated, so this is necessarily new code.

We can gain it via the unification, no?

Well, this would be a re-implementation anyway. I'm not sure the old one would disappear completly, because it depends on backslash commands which have different lexing assumptions (eg currently the variable-exists-test is performed from both "psqlscan.l" and "psqlscanslash.l" independently).


Reply via email to