Re: Libpq support to connect to standby server as priority

Haribabu Kommi <kommi.haribabu@gmail.com>

From: Haribabu Kommi <kommi.haribabu@gmail.com>
To: laurenz.albe@cybertec.at
Cc: Robert Haas <robertmhaas@gmail.com>, Jing Wang <jingwangian@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
Date: 2018-11-09T11:48:02Z
Lists: pgsql-hackers

Attachments

On Fri, Sep 28, 2018 at 5:31 PM Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

>
> On Thu, Jul 19, 2018 at 10:59 PM Haribabu Kommi <kommi.haribabu@gmail.com>
> wrote:
>
>>
>> On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <robertmhaas@gmail.com>
>> wrote:
>>
>>> On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <laurenz.albe@cybertec.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

Commits

  1. Make test_target_session_attrs more robust against connection failure.

  2. Extend the abilities of libpq's target_session_attrs parameter.

  3. Mark default_transaction_read_only as GUC_REPORT.

  4. Introduce a new GUC_REPORT setting "in_hot_standby".

  5. Avoid spamming the client with multiple ParameterStatus messages.

  6. Avoid harmless Valgrind no-buffer-pin errors.

  7. Restructure libpq code to remove some duplicity

  8. libpq: Add target_session_attrs parameter.