Re: Libpq support to connect to standby server as priority

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
93 messages Options
12345
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Jing Wang
Hi All,

Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case.

Some people may have noticed there is already another patch (https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal. 

It is better separate these 2 patches to consider.

Regards,
Jing Wang
Fujitsu Australia


libpq_support_perfer-read_002.patch (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2

On Wed, Jan 24, 2018 at 9:01 AM Jing Wang <[hidden email]> wrote:
Hi All,

Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case.

Some people may have noticed there is already another patch (https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal. 

It is better separate these 2 patches to consider.

I also feel prefer-read and read-only options needs to take as two different options.
prefer-read is simple to support than read-only.

Here I attached an updated patch that is rebased to the latest master and also
fixed some of the corner scenarios.

Regards,
Haribabu Kommi
Fujitsu Australia

0001-Allow-target-session-attrs-to-accept-prefer-read-opti.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Haribabu Kommi wrote:

> On Wed, Jan 24, 2018 at 9:01 AM Jing Wang <[hidden email]> wrote:
> > Hi All,
> >
> > Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case.
> >
> > Some people may have noticed there is already another patch (https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal.
> >
> > It is better separate these 2 patches to consider.
>
> I also feel prefer-read and read-only options needs to take as two different options.
> prefer-read is simple to support than read-only.
>
> Here I attached an updated patch that is rebased to the latest master and also
> fixed some of the corner scenarios.

The patch applies, builds and passes "make check-world".

I think the "prefer-read" functionality is desirable: It is exactly what you need
if you want to use replication for load balancing, and your application supports
different database connections for reading and writing queries.

"read-only" does not have a clear use case in my opinion.

With the patch, PostgreSQL behaves as expected if I have a primary and a standby and run:

  psql "host=/tmp,/tmp port=5433,5434 target_session_attrs=prefer-read"

But if I stop the standby (port 5434), libpq goes into an endless loop.

Concerning the code:

- The documentation needs some attention. Suggestion:

   If this parameter is set to <literal>prefer-read</literal>, connections
   where <literal>SHOW transaction_read_only</literal> returns off are preferred.
   If no such connection can be found, a connection that allows read-write
   transactions will be accepted.

- I think the construction with "read_write_host_index" makes the code even more
  complicated than it already is.

  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.

Yours,
Laurenz Albe

Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2


On Wed, Jul 4, 2018 at 11:14 PM Laurenz Albe <[hidden email]> wrote:
Haribabu Kommi wrote:
> On Wed, Jan 24, 2018 at 9:01 AM Jing Wang <[hidden email]> wrote:
> > Hi All,
> >
> > Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case.
> >
> > Some people may have noticed there is already another patch (https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal.
> >
> > It is better separate these 2 patches to consider.
>
> I also feel prefer-read and read-only options needs to take as two different options.
> prefer-read is simple to support than read-only.
>
> Here I attached an updated patch that is rebased to the latest master and also
> fixed some of the corner scenarios.

Thanks for the review.
 
The patch applies, builds and passes "make check-world".

I think the "prefer-read" functionality is desirable: It is exactly what you need
if you want to use replication for load balancing, and your application supports
different database connections for reading and writing queries.

"read-only" does not have a clear use case in my opinion.

With the patch, PostgreSQL behaves as expected if I have a primary and a standby and run:

  psql "host=/tmp,/tmp port=5433,5434 target_session_attrs=prefer-read"

But if I stop the standby (port 5434), libpq goes into an endless loop.

There was a problem in reusing the primary host index and it leads to loop.
Attached patch fixed the issue.
 
Concerning the code:

- The documentation needs some attention. Suggestion:

   If this parameter is set to <literal>prefer-read</literal>, connections
   where <literal>SHOW transaction_read_only</literal> returns off are preferred.
   If no such connection can be found, a connection that allows read-write
   transactions will be accepted.

updated as per you comment.
 
- I think the construction with "read_write_host_index" makes the code even more
  complicated than it already is.

  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.

Even if we add a variable to cache the connection, I don't think the logic of checking
the next host for the read-only host logic may not change, but the extra connection
request to the read-write host again will be removed.

Regards,
Haribabu Kommi
Fujitsu Australia

0001-Allow-taget-session-attrs-to-accept-prefer-read-opti_v2.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2

On Wed, Jul 11, 2018 at 6:00 PM Haribabu Kommi <[hidden email]> wrote:


On Wed, Jul 4, 2018 at 11:14 PM Laurenz Albe <[hidden email]> wrote:
Haribabu Kommi wrote:
 
- I think the construction with "read_write_host_index" makes the code even more
  complicated than it already is.

  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.

Even if we add a variable to cache the connection, I don't think the logic of checking
the next host for the read-only host logic may not change, but the extra connection
request to the read-write host again will be removed.

I evaluated your suggestion of caching the connection and reuse it when there is no
read only server doesn't find, but I am thinking that it will add more complexity and also
the connection to the other servers delays, the cached connection may be closed by
the server also because of timeout.

I feel the extra time during connection may be fine, if user is preferring the prefer-read
mode, instead of adding more complexity in handling the cached connection? 

comments?

Regards,
Haribabu Kommi
Fujitsu Australia
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Haribabu Kommi wrote:

> > On Wed, Jul 4, 2018 at 11:14 PM Laurenz Albe <[hidden email]> wrote:
> > > Haribabu Kommi wrote:
> > >  
> > > - I think the construction with "read_write_host_index" makes the code even more
> > >   complicated than it already is.
> > >
> > >   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.
> >
> > Even if we add a variable to cache the connection, I don't think the logic of checking
> > the next host for the read-only host logic may not change, but the extra connection
> > request to the read-write host again will be removed.
>
> I evaluated your suggestion of caching the connection and reuse it when there is no
> read only server doesn't find, but I am thinking that it will add more complexity and also
> the connection to the other servers delays, the cached connection may be closed by
> the server also because of timeout.
>
> I feel the extra time during connection may be fine, if user is preferring the prefer-read
> mode, instead of adding more complexity in handling the cached connection?
>
> comments?

I tested the new patch, and it works as expected.

I don't think that time-out of the cached session is a valid concern, because that
would have to be a really short timeout.
On the other hand, establishing the connection twice (first to check if it is read-only,
then again because no read-only connection is found) can be quite costly.

But that is a matter of debate, as is the readability of the code.

Since I don't think I can contribute more to this patch, I'll mark it as
ready for committer.

Yours,
Laurenz Albe

Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2
On Tue, Jul 17, 2018 at 12:42 AM Laurenz Albe <[hidden email]> wrote:
Haribabu Kommi wrote:
> > On Wed, Jul 4, 2018 at 11:14 PM Laurenz Albe <[hidden email]> wrote:
> > > Haribabu Kommi wrote:
> > > 
> > > - I think the construction with "read_write_host_index" makes the code even more
> > >   complicated than it already is.
> > >
> > >   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.
> >
> > Even if we add a variable to cache the connection, I don't think the logic of checking
> > the next host for the read-only host logic may not change, but the extra connection
> > request to the read-write host again will be removed.
>
> I evaluated your suggestion of caching the connection and reuse it when there is no
> read only server doesn't find, but I am thinking that it will add more complexity and also
> the connection to the other servers delays, the cached connection may be closed by
> the server also because of timeout.
>
> I feel the extra time during connection may be fine, if user is preferring the prefer-read
> mode, instead of adding more complexity in handling the cached connection?
>
> comments?

I tested the new patch, and it works as expected.

Thanks for the confirmation.
 
I don't think that time-out of the cached session is a valid concern, because that
would have to be a really short timeout.
On the other hand, establishing the connection twice (first to check if it is read-only,
then again because no read-only connection is found) can be quite costly.

But that is a matter of debate, as is the readability of the code.

Thanks for your opinion, let's wait for opinion from others also.
I can go for the modification, if others also find it useful.
  
Regards,
Haribabu Kommi
Fujitsu Australia
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Robert Haas
In reply to this post by Laurenz Albe
On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <[hidden email]> 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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2

On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <[hidden email]> wrote:
On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <[hidden email]> 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.

Regards,
Haribabu Kommi
Fujitsu Australia
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2

On Thu, Jul 19, 2018 at 10:59 PM Haribabu Kommi <[hidden email]> wrote:

On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <[hidden email]> wrote:
On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <[hidden email]> 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.

Regards,
Haribabu Kommi
Fujitsu Australia

0001-Allow-taget-session-attrs-to-accept-prefer-read-opti_v3.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Haribabu Kommi wrote:

> On Thu, Jul 19, 2018 at 10:59 PM Haribabu Kommi <[hidden email]> wrote:
> > On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <[hidden email]> wrote:
> > > On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2
In reply to this post by Haribabu Kommi-2

On Fri, Sep 28, 2018 at 5:31 PM Haribabu Kommi <[hidden email]> wrote:

On Thu, Jul 19, 2018 at 10:59 PM Haribabu Kommi <[hidden email]> wrote:

On Wed, Jul 18, 2018 at 10:53 PM Robert Haas <[hidden email]> wrote:
On Wed, Jul 4, 2018 at 9:14 AM, Laurenz Albe <[hidden email]> 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

0001-Allow-taget-session-attrs-to-accept-prefer-read-opti_v4.patch (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Haribabu Kommi wrote:
> Added comments along the lines that you mentioned. And also try
> to update some more comments.

Looks ok to me, I'll mark it as "ready for committer".

Yours,
Laurenz Albe


Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Tom Lane-2
Laurenz Albe <[hidden email]> writes:
> Haribabu Kommi wrote:
>> Added comments along the lines that you mentioned. And also try
>> to update some more comments.

> Looks ok to me, I'll mark it as "ready for committer".

I don't like this patch at all: the business with keeping two connections
open seems impossibly fragile and full of race conditions.  (For instance,
by the time you return the read-only session to the application, it might
not be read-only any more.  I also wonder what inquiry functions like
PQsocket ought to return while in this state.)  I think the feature
definition needs to be re-thought to make that unnecessary.

Also, we really need to consider the interaction between this and the
feature(s) being discussed in the thread at

https://www.postgresql.org/message-id/flat/1700970.cRWpxnom9y%40hammer.magicstack.net

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Tom Lane wrote:

> Laurenz Albe <[hidden email]> writes:
> > Haribabu Kommi wrote:
> > > Added comments along the lines that you mentioned. And also try
> > > to update some more comments.
> > Looks ok to me, I'll mark it as "ready for committer".
>
> I don't like this patch at all: the business with keeping two connections
> open seems impossibly fragile and full of race conditions.  (For instance,
> by the time you return the read-only session to the application, it might
> not be read-only any more.  I also wonder what inquiry functions like
> PQsocket ought to return while in this state.)  I think the feature
> definition needs to be re-thought to make that unnecessary.

As it is now, the patch doesn't keep two connections open.  It remembers
the index of the host of the first successful writable connection, but
closes the connection, and opens another one to that host if no read-only
host can be found.

If the read-only connection turns writable after it has been tested,
but before it is returned, that can hardly be avoided.
I don't think that's so bad - after all, you asked for a read-only
connection *if possible*.
If you demand that the server be not promoted until the connection has
been returned to the client, you'd somehow have to block the server
from being promoted, right?

> Also, we really need to consider the interaction between this and the
> feature(s) being discussed in the thread at
>
> https://www.postgresql.org/message-id/flat/1700970.cRWpxnom9y%40hammer.magicstack.net

That's a good point.

Yours,
Laurenz Albe


Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Haribabu Kommi-2

On Tue, Nov 13, 2018 at 7:26 AM Laurenz Albe <[hidden email]> wrote:
Tom Lane wrote:
> Laurenz Albe <[hidden email]> writes:
> > Haribabu Kommi wrote:
> > > Added comments along the lines that you mentioned. And also try
> > > to update some more comments.
> > Looks ok to me, I'll mark it as "ready for committer".
>
> I don't like this patch at all: the business with keeping two connections
> open seems impossibly fragile and full of race conditions.  (For instance,
> by the time you return the read-only session to the application, it might
> not be read-only any more.  I also wonder what inquiry functions like
> PQsocket ought to return while in this state.)  I think the feature
> definition needs to be re-thought to make that unnecessary.

As it is now, the patch doesn't keep two connections open.  It remembers
the index of the host of the first successful writable connection, but
closes the connection, and opens another one to that host if no read-only
host can be found.

If the read-only connection turns writable after it has been tested,
but before it is returned, that can hardly be avoided.
I don't think that's so bad - after all, you asked for a read-only
connection *if possible*.
If you demand that the server be not promoted until the connection has
been returned to the client, you'd somehow have to block the server
from being promoted, right?

I also have the same opinion of Laurenz, that this option is letting the
application to connect to read-only server if possible, otherwise let it
connect to read-write server.

I feel that any of the state changes during the connection and after connection,
needs not to be reflected on the existing connection for these type of
connections.
 
> Also, we really need to consider the interaction between this and the
> feature(s) being discussed in the thread at
>
> https://www.postgresql.org/message-id/flat/1700970.cRWpxnom9y%40hammer.magicstack.net

That's a good point.

Thanks for the link. Based on the conclusion on the other thread of GUC_REPORT,
this patch also can use that logic, but that is limited only till the connection establishment
for these connection types.

Regards,
Haribabu Kommi
Fujitsu Australia
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Michael Paquier-2
On Tue, Nov 13, 2018 at 05:54:17PM +1100, Haribabu Kommi wrote:
> On Tue, Nov 13, 2018 at 7:26 AM Laurenz Albe <[hidden email]>
> wrote:
>> As it is now, the patch doesn't keep two connections open.  It remembers
>> the index of the host of the first successful writable connection, but
>> closes the connection, and opens another one to that host if no read-only
>> host can be found.

That's commented in the patch as follows:
+    /*
+     * Requested type is prefer-read, then record this host index
+     * and try the other before considering it later
+     */
+    if ((conn->target_session_attrs != NULL) &&
+        (strcmp(conn->target_session_attrs, "prefer-read") == 0) &&
+        (conn->read_write_host_index != -2))

>> If the read-only connection turns writable after it has been tested,
>> but before it is returned, that can hardly be avoided.
>> I don't think that's so bad - after all, you asked for a read-only
>> connection *if possible*.

Yeah, it is difficult to guarantee that except that checking from time
to time that the connection is still read-only after establishing it.
It is in my opinion mostly a matter of documentation, meaning that the
selection is done when the connection is attempted from the defined
set.

> I also have the same opinion of Laurenz, that this option is letting the
> application to connect to read-only server if possible, otherwise let it
> connect to read-write server.
>
> I feel that any of the state changes during the connection and after
> connection, needs not to be reflected on the existing connection for
> these type of connections.

+      /*
+       * Reset the host index value to avoid recursion during the
+       * second connection attempt.
+       */
+      conn->read_write_host_index = -2;

Okay, this gets ugly.  I am pretty sure that we should use instead a
status flag and actually avoid any kind of recursion risk in the logic.
Or else it would get hard to track to which value what needs to be set
where in the code.

From purely the code point of view, it seems to me that it is actually
more simple to implement a "read-only" mode as this way there is no need
to mix between CONNECTION_CHECK_READONLY and CONNECTION_CHECK_WRITABLE,
remembering the past index of a connection which may be needed later on
if the next ones don't meet with the wanted conditions.

Each time I have heard about load balancing, applications did not really
care about whether only standbys were used for a set of queries and
accepted that the primary also shared some of the read-only load, be it
for analytics or OLTP, in which case "any" covers already everything
needed.  And if you really want a standby, "read-only" would also be
useful so as an application layer can properly fail if there is only a
primary available.

JDBC has its own set of options with targetServerType: master, slave,
secondary, preferSlave and preferSecondary.  What's proposed here is
preferSlave and what we would miss is slave at the end.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Michael Paquier-2
On Thu, Nov 15, 2018 at 05:14:33PM +0900, Michael Paquier wrote:
> JDBC has its own set of options with targetServerType: master, slave,
> secondary, preferSlave and preferSecondary.  What's proposed here is
> preferSlave and what we would miss is slave at the end.

So thinking a couple of extra minutes on this one, I am wondering if it
would be better to close completely the gap with two patches:
1) Get "read-only" done first, which uses most of the existing
infrastructure.  That seems simple enough.
2) Get "prefer-read" and "prefer-write", which need some new
infrastructure to track the last preferred connection depending on what
the client wants.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Tom Lane-2
In reply to this post by Laurenz Albe
Laurenz Albe <[hidden email]> writes:
> Tom Lane wrote:
>> I don't like this patch at all: the business with keeping two connections
>> open seems impossibly fragile and full of race conditions.

> As it is now, the patch doesn't keep two connections open.  It remembers
> the index of the host of the first successful writable connection, but
> closes the connection, and opens another one to that host if no read-only
> host can be found.

Oh!  The reason I assumed it wasn't doing that is that such a behavior
seems completely insane.  If the point is to keep down the load on your
master server, then connecting only to immediately disconnect is not
a friendly way to do that --- even without counting the fact that you
might later come back and connect again.

If that's the best we can do, we should forget the whole feature and
just recommend putting slave servers first in your hosts list when
you want prefer-slave.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Libpq support to connect to standby server as priority

Laurenz Albe
Tom Lane wrote:

> > As it is now, the patch doesn't keep two connections open.  It remembers
> > the index of the host of the first successful writable connection, but
> > closes the connection, and opens another one to that host if no read-only
> > host can be found.
>
> Oh!  The reason I assumed it wasn't doing that is that such a behavior
> seems completely insane.  If the point is to keep down the load on your
> master server, then connecting only to immediately disconnect is not
> a friendly way to do that --- even without counting the fact that you
> might later come back and connect again.

That's why I had argued initially to keep the session open, but you
seem to dislike that idea as well.

> If that's the best we can do, we should forget the whole feature and
> just recommend putting slave servers first in your hosts list when
> you want prefer-slave.

If you know which is which, certainly.
But in a setup with automated failover you cannot be certain which is which.
That's what the proposed feature targets.

Yours,
Laurenz Albe


12345