Re: Patch: Implement failover on libpq connect level.

From: Catalin Iacob <iacobcatalin(at)gmail(dot)com>
To: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, Peter van Hardenberg <pvh(at)pvh(dot)ca>, Peter Eisentraut <peter_e(at)gmx(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Implement failover on libpq connect level.
Date: 2016-11-23 16:49:29
Message-ID: CAHg_5grVKbO73CqKNYsCYsX5aJ=deDSAyW44wjmwt1uqngScdQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-jdbc

On Tue, Nov 22, 2016 at 8:38 AM, Tsunakawa, Takayuki
<tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
> transaction_read_only=on does not mean the standby. As the manual article on hot standby says, they are different.

> I'm afraid that if the current patch is committed, you will lose interest in the ideal solution.

I agree with Robert that lacking the ideal solution shouldn't block a
good enough solution today. But I find the current patch as it stands
too confusing to pass the "good enough" bar.

> Then if the current patch is out as v10, there would be a concern about incompatibility when we pursue the ideal solution in a later release. That is, "should we continue to report > that this server is standby even if it's actually a primary with transaction_read_only is on, to maintain compatibility with the older release."

Agreed. I am worried that this will end up being a wart:
target_server_type=primary doesn't really mean primary it means that
by default the session is not read only which by the way is usually
the case for primaries but that can be changed.

> If you want to connect to a server where the transaction is read-only, then shouldn't the connection parameter be something like "target_session_attrs=readonly"? That represents exactly what the code does.

FWIW I find this to be a reasonable compromise. To keep the analogy
with the current patch it would be more something like
"target_session_attrs=read_write|any" and the docs could point out "by
the way, if you didn't tweak default_transaction_read_only read_write
will only pick a primary so this can be used for failover".
The functionality is the same but changing the name removes the
ambiguity in the semantics and leaves the door open for a future real
primary/replica functionality which includes logical replication with
whatever semantics are deemed appropriate then.

We do lose pgjdbc compatibility but the current patch doesn't have
that 100% anyway: in pgjdbc it's called targetServerType and it
accepts master instead of primary and it also accepts slave and
preferSlave
I notice at https://github.com/pgjdbc/www/blob/master/documentation/head/connect.md
that pgjdbc says "The master/slave distinction is currently done by
observing if the server allows writes" which, due to the "currently"
part seems to acknowledge this is not ideal and leaves the door open
for changes in semantics but I don't think changing semantics is going
to fly for Postgres itself.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-11-23 16:54:48 Re: patch: function xmltable
Previous Message Tom Lane 2016-11-23 16:33:38 Re: Typo in comment in file analyze.c [master branch]

Browse pgsql-jdbc by date

  From Date Subject
Next Message Mithun Cy 2016-11-24 12:16:48 Re: Patch: Implement failover on libpq connect level.
Previous Message Robert Haas 2016-11-22 20:52:38 Re: Patch: Implement failover on libpq connect level.