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.