Re: Libpq support to connect to standby server as priority

From: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
To: laurenz(dot)albe(at)cybertec(dot)at
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, 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-11-09 11:48:02
Message-ID: CAJrrPGcwt0LLLzDF9c3-zWrESJCJko-HovDOrMobkaWQaJ5YUw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Fri, Sep 28, 2018 at 5:31 PM Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
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.
>

[some how i didn't receive this mail, copy pasted from mailing list ]

>The code is much more readable now, thanks.

Thanks for the review.

>--- 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.

Added comments along the lines that you mentioned. And also try
to update some more comments.

>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.

Yes, I didn't add that logic of keeping the first connection open, Currently
I feel that adds more complexity in supporting the same. If everyone feels
that is required, I will add that logic.

Updated patch attached.

Regards,
Haribabu Kommi
Fujitsu Australia

Attachment Content-Type Size
0001-Allow-taget-session-attrs-to-accept-prefer-read-opti_v4.patch application/octet-stream 14.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2018-11-09 11:53:01 Re: Libpq support to connect to standby server as priority
Previous Message Daniel Verite 2018-11-09 11:18:51 Alternative to \copy in psql modelled after \g