Re: Libpq support to connect to standby server as priority

On Tue, Nov 13, 2018 at 05:54:17PM +1100, Haribabu Kommi wrote:
> On Tue, Nov 13, 2018 at 7:26 AM Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
> 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