Re: [PATCH] pgbench: add multiconnect option

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: David Christensen <david(dot)christensen(at)crunchydata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] pgbench: add multiconnect option
Date: 2021-07-01 10:22:45
Message-ID: alpine.DEB.2.22.394.2107011052210.2024567@pseudo
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

Attachment Content-Type Size
pgbench-multi-connect-conninfo-1.patch text/x-diff 5.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2021-07-01 10:28:08 Re: Teach pg_receivewal to use lz4 compression
Previous Message Dean Rasheed 2021-07-01 10:02:51 Re: Numeric multiplication overflow errors