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
- 0001-Allow-taget-session-attrs-to-accept-prefer-read-opti_v4.patch (application/octet-stream) patch v4-0001
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
-
Make test_target_session_attrs more robust against connection failure.
- 3769e11a3183 14.0 landed
-
Extend the abilities of libpq's target_session_attrs parameter.
- ee28cacf619f 14.0 landed
-
Mark default_transaction_read_only as GUC_REPORT.
- d16f8c8e416d 14.0 landed
-
Introduce a new GUC_REPORT setting "in_hot_standby".
- bf8a662c9afa 14.0 landed
-
Avoid spamming the client with multiple ParameterStatus messages.
- 2432b1a04087 14.0 landed
-
Avoid harmless Valgrind no-buffer-pin errors.
- a766d6ca22ac 14.0 cited
-
Restructure libpq code to remove some duplicity
- b438e7e7a1c5 13.0 landed
-
libpq: Add target_session_attrs parameter.
- 721f7bd3cbcc 10.0 cited