Re: [PATCH] pgbench: add multiconnect option

Hello David,

> This patch adds the concept of "multiconnect" to pgbench (better
> terminology welcome).

Good. I was thinking of adding such capability, possibly for handling
connection errors and reconnecting…

> The basic idea here is to allow connections made with pgbench to use
> different auth values or connect to multiple databases. We implement
> this using a user-provided PGSERVICEFILE and choosing a PGSERVICE from
> this based on a number of strategies. (Currently the only supported
> strategies are round robin or random.)

I was thinking of providing a allowing a list of conninfo strings with
repeated options, eg --conninfo "foo" --conninfo "bla"…

Your approach using PGSERVICEFILE also make sense!

Maybe it could be simplified, the code base reduced, and provide more
benefits, by mixing both ideas.

In particular, pgbench parses the file but then it will be read also by
libpq, yuk yuk.

Also, I do not like that PGSERVICE is overriden by pgbench, while other
options are passed with the parameters approach in doConnect. It would
make proce sense to add a "service" field to the parameters for
consistency, if this approach was to be pursued.

On reflexion, I'd suggest to use the --conninfo (or some other name)
approach, eg "pgbench --conninfo='service=s1' --conninfo='service=s2'" and
users just have to set PGSERVICEFILE env themselves, which I think is
better than pgbench overriding env variables behind their back.

This allow to have a service file with more connections and just tell
pgbench which ones to use, which is the expected way to use this feature.
This drops file parsing.

I can only see benefit to this simplified approach.
What do you think?

About the patch:

There are warnings about trailing whitespaces when applying the patch, and
there are some tabbing issues in the file.

I would not consume "-g" option unless there is some logical link with the
feature. I'd be okay with "-m" if it is still needed. I would suggest to
use it for the choice strategy?

stringinfo: We already have PQExpBuffer imported, could we use that
instead? Having two set of struct/functions which do the same in the same
source file does not look like a good idea. If we do not parse the file,
nothing is needed, which is a relief.

Attached is my work-in-progress start at adding conninfo, that would need
to be improved with strategies.

--
Fabien.