Re: Libpq support to connect to standby server as priority

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Jing Wang <jingwangian(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Subject: Re: Libpq support to connect to standby server as priority
Date: 2018-10-05 14:34:34
Message-ID: 360dcd4eff43a302e04148abca0c94f56a233df9.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Haribabu Kommi wrote:
> On Thu, Jul 19, 2018 at 10:59 PM Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> wrote:
> > On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > > On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> wrote:
> > > > What about keeping the first successful connection open and storing it in a
> > > > variable if we are in "prefer-read" mode.
> > > > If we get the read-only connection we desire, close that cached connection,
> > > > otherwise use it.
> > >
> > > I like this idea. If I recall correctly, the logic in this area is
> > > getting pretty complex, so we might need to refactor it for better
> > > readability and maintainability.
> >
> > OK. I will work on the code refactoring first and then provide the
> > prefer-read option on top it.
>
> commits d1c6a14bacf and 5ca00774194 have refactored the logic
> of handling the different connection states.
>
> Attached is a rebased patch after further refactoring the new option
> code for easier maintenance.

The code is much more readable now, thanks.

--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -397,6 +397,7 @@ struct pg_conn
int nconnhost; /* # of hosts named in conn string */
int whichhost; /* host we're currently trying/connected to */
pg_conn_host *connhost; /* details about each named host */
+ int read_write_host_index; /* index for first read-write host in connhost */

/* Connection data */
pgsocket sock; /* FD for socket, PGINVALID_SOCKET if

I think the comment could use more love.

This would be the place to document the logic:
Initial value is -1, then then index of the first working server
we found, and -2 for the second attempt to connect to that server.

I notice that you don't keep the first connection open, but close
and reopen it. I guess that is a matter of taste, but it would be
easier on resources (and reduce connection time) if the connection
were kept open.
Admittedly, it would be more difficult and might further complicate
code that is not very clear as it is.

If you work on some more on the comment above, I will mark it as
ready for committer.

Yours,
Laurenz Albe

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-10-05 14:53:34 Re: SCRAM with channel binding downgrade attack
Previous Message Tom Lane 2018-10-05 13:56:35 Re: now() vs transaction_timestamp()