Re: Patch: Implement failover on libpq connect level.

Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>

From: "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
To: 'Mithun Cy' <mithun.cy@enterprisedb.com>, Robert Haas <robertmhaas@gmail.com>
Cc: Peter van Hardenberg <pvh@pvh.ca>, Peter Eisentraut <peter_e@gmx.net>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2016-11-18T01:09:46Z
Lists: pgsql-hackers
From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Mithun Cy
> I am adding next version of the patch it have following fixes.
> Tsunakawa's comments
> 
> 1.  PGconn->target_server_type is now freed in freePGconn() 2.  Added
> PGTARGETSERVERTYPE.
> 
> 
> Additional comments from others
> 3.  Moved from SELECT pg_is_in_recovery() to SHOW transaction_read_only
> now should handle different kind of replication, as we recognise server
> to which writable connection can be made as primary. Very exactly like JDBC
> driver. Also documented about it.
> 4. renamed words from master to primary.

Thank you.  The following items need addressing.  Some of them require some more discussion to reach consensus, and I hope they will settle down soon.  After checking the progress for a week or so, I'll mark the CommitFest entry as "ready for committer" or "waiting on author".

(1)
+        server. Set this to <literaL>any</literal>, if you want to connect to
+        A server is recognized as a primary/standby by observering whether it

Typo.  <literaL. -> <literal>, and "observering" -> "observing".


(2)
+	{"target_server_type", "PGTARGETSERVERTYPE", NULL, NULL,
+		"Target server type", "", 6,

Looking at existing parameters, the default value is defined as a macro, and the display label is a sequence of words separated by "-".  i.e.

+	{"target_server_type", "PGTARGETSERVERTYPE", DefaultTargetServerType, NULL,
+		"Target-Server-Type", "", 6,


(3)
Please avoid adding another round trip by using a GUC_REPORTed variable (ParameterStatus entry).  If you want to support this libpq failover with pre-10 servers, you can switch the method of determining the primary based on the server version.  But I don't think it's worth supporting older servers at the price of libpq code complexity.


(4)
Please consider supporting "standby" and "prefer_standby" like PgJDBC.  They are useful without load balancing when multiple standbys are used for HA.


(5)
I haven't tracked the progress of logical replication, but will target_server_type be likely to be usable with it?  How will target_server_type fit logical replication?

Regards
Takayuki Tsunakawa



Commits

  1. libpq: Add target_session_attrs parameter.

  2. Remove superuser checks in pgstattuple

  3. Fix unwanted flushing of libpq's input buffer when socket EOF is seen.