Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Dec 5, 2016 at 1:59 PM, Mithun Cy wrote: > On Mon, Dec 5, 2016 at 11:23 PM, Robert Haas wrote: >>I think that you need a restoreErrorMessage call here: >>/* Skip any remaining addresses for this host. */ >>

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Dec 5, 2016 at 11:23 PM, Robert Haas wrote: >I think that you need a restoreErrorMessage call here: >/* Skip any remaining addresses for this host. */ >conn->addr_cur = NULL; >if

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 05/12/16 17:00, Mithun Cy wrote: [...] errorMessage even outside PQconnectPoll. But that seems not required. Attacting the new patch which fixes the same. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com [...] Is that meant to be

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Sun, Dec 4, 2016 at 11:00 PM, Mithun Cy wrote: > On Fri, Dec 2, 2016 at 8:54 PM, Robert Haas wrote: >> Couldn't this just be a variable in PQconnectPoll(), instead of adding >> a new structure member? > > I have fixed same with a local

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Dec 2, 2016 at 8:54 PM, Robert Haas wrote: > Couldn't this just be a variable in PQconnectPoll(), instead of adding > a new structure member? I have fixed same with a local variable in PQconnectPoll, Initally I thought savedMessage should have same visiblitly as

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Dec 1, 2016 at 5:00 AM, Mithun Cy wrote: > On Wed, Nov 30, 2016 at 7:14 AM, Mithun Cy > wrote: >> Thanks, send query resets the errorMessage. Will fix same. >> PQsendQuery()->PQsendQueryStart()->resetPQExpBuffer(>errorMessage); > >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 30, 2016 at 7:14 AM, Mithun Cy wrote: > Thanks, send query resets the errorMessage. Will fix same. > *PQsendQuery()->PQsendQueryStart()->resetPQExpBuffer(>errorMessage);* *Please find the patch which fixes this bug. **conn->errorMessage as per definition

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 30, 2016 at 1:44 AM, Robert Haas wrote: >On Tue, Nov 29, 2016 at 2:19 PM, Kuntal Ghosh wrote: >> I was doing some testing with the patch and I found some inconsistency >> in the error message. > Hmm, maybe the query buffer is

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 29, 2016 at 3:14 PM, Robert Haas wrote: > On Tue, Nov 29, 2016 at 2:19 PM, Kuntal Ghosh > wrote: >> On Thu, Nov 24, 2016 at 5:46 PM, Mithun Cy >> wrote: >>> I have taken this suggestion now renamed

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 29, 2016 at 2:19 PM, Kuntal Ghosh wrote: > On Thu, Nov 24, 2016 at 5:46 PM, Mithun Cy wrote: >> I have taken this suggestion now renamed target_server_type to >> target_session_attrs with possible 2 values "read-write", "any".

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 24, 2016 at 5:46 PM, Mithun Cy wrote: > I have taken this suggestion now renamed target_server_type to > target_session_attrs with possible 2 values "read-write", "any". > May be we could expand to "readonly" and "prefer-readonly" in next patch > proposal.

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 24, 2016 at 7:16 AM, Mithun Cy wrote: > On Wed, Nov 23, 2016 at 10:19 PM, Catalin Iacob > wrote: >On Tue, Nov 22, 2016 at 8:38 AM, Tsunakawa, Takayuki > wrote: > >> If you want to connect to a

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 23, 2016 at 10:19 PM, Catalin Iacob wrote: On Tue, Nov 22, 2016 at 8:38 AM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: >> If you want to connect to a server where the transaction is read-only, then shouldn't the connection parameter be

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 22, 2016 at 8:38 AM, Tsunakawa, Takayuki 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 >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Sun, Nov 13, 2016 at 9:02 PM, Tsunakawa, Takayuki wrote: > From: pgsql-hackers-ow...@postgresql.org >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas >> Great, committed. There's still potentially more work to be done here, >> because my

Re: [HACKERS] Patch: Implement failover on libpq connect level.

An updated patch with some fixes for bugs reported earlier, A. failover_to_new_master_v4.patch Default value "any" is added to target_server_type parameter during its definition. B. libpq-failover-smallbugs_02.patch Fixed the issue raised by [PATCH] pgpassfile connection option

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 21 November 2016 at 00:08, Mithun Cy wrote: >> 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

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > I am very strict about regressing the performance of things that we already > have, but I try not to make a policy that a new feature must be as fast > as it could ever be. That could

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Sun, Nov 20, 2016 at 8:48 PM, Tsunakawa, Takayuki wrote: >> True, but raising the bar for this feature so that it doesn't get done is >> also bad. It can be improved in a later patch. > > I thought you are very strict about performance, so I hesitate to believe

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > On Thu, Nov 17, 2016 at 10:08 PM, Craig Ringer > wrote: > > We can and probably should have both. > > > > If the server tells us on connect whether it's a

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: Mithun Cy [mailto:mithun...@enterprisedb.com] > > + {"target_server_type", "PGTARGETSERVERTYPE", > DefaultTargetServerType, NULL, > > + "Target-Server-Type", "", 6, > > Thanks fixed. + {"target_server_type", "PGTARGETSERVERTYPE", NULL, NULL, The default value

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Nov 18, 2016 at 6:39 AM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: >Typo. , and "observering" -> "observing". Thanks fixed. > + {"target_server_type", "PGTARGETSERVERTYPE", DefaultTargetServerType, NULL, > + "Target-Server-Type", "", 6, Thanks

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 19 November 2016 at 01:29, Robert Haas wrote: >> We can and probably should have both. >> >> If the server tells us on connect whether it's a standby or not, use that. >> >> Otherwise, ask it. >> >> That way we don't pay the round-trip cost and get the log spam when >>

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 17, 2016 at 10:08 PM, Craig Ringer wrote: > On 17 November 2016 at 10:57, Robert Haas wrote: >> On Wed, Nov 16, 2016 at 9:00 PM, Tsunakawa, Takayuki >> wrote: >>> Do we really want to enable libpq failover

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 17 November 2016 at 10:57, Robert Haas wrote: > On Wed, Nov 16, 2016 at 9:00 PM, Tsunakawa, Takayuki > wrote: >> Do we really want to enable libpq failover against pre-V10 servers? I don't >> think so, as libpq is a part of PostgreSQL

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@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. >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 17, 2016 at 8:27 AM, Robert Haas wrote: >but SELECT pg_is_in_recovery() and SHOW transaction_read_only >exist in older versions so if we pick either of those methods then it >will just work. I am adding next version of the patch it have following fixes.

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 16, 2016 at 9:00 PM, Tsunakawa, Takayuki wrote: > Do we really want to enable libpq failover against pre-V10 servers? I don't > think so, as libpq is a part of PostgreSQL and libpq failover is a new > feature in PostgreSQL 10. At least, as one user,

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Hmm, let's go back to the JDBC method, then. "show transaction_read_only" > will return true on a standby, but presumably also on any other non-writable > node. You could even force

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > In my understanding pg_is_in_recovery() returns true if it's a standby node. > However, even if it returns other than true, the server is not necessarily > a primary. Even it's not

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > On Mon, Nov 14, 2016 at 8:09 PM, Tsunakawa, Takayuki > wrote: > > From: pgsql-hackers-ow...@postgresql.org > >>

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 16, 2016 at 12:00 PM, Catalin Iacob wrote: > On Wed, Nov 16, 2016 at 2:50 PM, Robert Haas wrote: >> Hmm, let's go back to the JDBC method, then. "show >> transaction_read_only" will return true on a standby, but presumably >> also on

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 16, 2016 at 2:50 PM, Robert Haas wrote: > Hmm, let's go back to the JDBC method, then. "show > transaction_read_only" will return true on a standby, but presumably > also on any other non-writable node. You could even force it to be > true artificially if you

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[moving this branch of discussion to pgsql-jdbc] On Tue, Nov 15, 2016 at 10:31 PM, Mithun Cy wrote: > JDBC is sending "show transaction_read_only" to find whether it > is master or not. If true, that's insane. That can be different on each connection to the cluster

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 15, 2016 at 11:31 PM, Mithun Cy wrote: >> > So I am tempted to just >> > hold my nose and hard-code the SQL as JDBC is presumably already >> doing. > > JDBC is sending "show transaction_read_only" to find whether it is master or > not. > Victor's patch also

Re: [HACKERS] Patch: Implement failover on libpq connect level.

> JDBC is sending "show transaction_read_only" to find whether it is master > or not. > Victor's patch also started with it, but later it was transformed into > pg_is_in_recovery > by him as it appeared more appropriate to identify the master / slave. In my understanding pg_is_in_recovery()

Re: [HACKERS] Patch: Implement failover on libpq connect level.

> > > So I am tempted to just > > hold my nose and hard-code the SQL as JDBC is presumably already > doing. JDBC is sending "show transaction_read_only" to find whether it is master or not. Victor's patch also started with it, but later it was transformed into pg_is_in_recovery by him as it

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 15, 2016 at 12:53 PM, Catalin Iacob wrote: > On Tue, Nov 15, 2016 at 5:58 PM, Robert Haas wrote: >> On Tue, Nov 15, 2016 at 9:42 AM, Alvaro Herrera >> wrote: >>> I would rather come up with something that works

Re: [HACKERS] Patch: Implement failover on libpq connect level.

Catalin Iacob wrote: > On Tue, Nov 15, 2016 at 3:42 PM, Alvaro Herrera > wrote: > > FWIW I'm not sure "primary" is the right term here either. I think what > > we really want to know is whether the node can accept writes; maybe > > "pg_is_writable_node". > > This made

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 15, 2016 at 3:42 PM, Alvaro Herrera wrote: > FWIW I'm not sure "primary" is the right term here either. I think what > we really want to know is whether the node can accept writes; maybe > "pg_is_writable_node". This made me think of another complication:

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 15, 2016 at 5:58 PM, Robert Haas wrote: > On Tue, Nov 15, 2016 at 9:42 AM, Alvaro Herrera > wrote: >> I would rather come up with something that works in both cases that we >> can extend internally later, say pg_is_primary_node() or

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 15, 2016 at 9:42 AM, Alvaro Herrera wrote: > Robert Haas wrote: >> On Mon, Nov 14, 2016 at 2:38 AM, Mithun Cy >> wrote: >> > I have not tested with logical replication. Currently we identify the >> > primary to connect based on

Re: [HACKERS] Patch: Implement failover on libpq connect level.

Robert Haas wrote: > On Mon, Nov 14, 2016 at 2:38 AM, Mithun Cy wrote: > > I have not tested with logical replication. Currently we identify the > > primary to connect based on result of "SELECT pg_is_in_recovery()". So I > > think it works. Do you want me test a

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Nov 14, 2016 at 8:09 PM, Tsunakawa, Takayuki wrote: > From: pgsql-hackers-ow...@postgresql.org >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy >> Thanks, my concern is suppose you have 3 server in cluster A(new version), >> B(new

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Nov 14, 2016 at 2:38 AM, Mithun Cy wrote: > On Fri, Nov 11, 2016 at 7:33 PM, Peter Eisentraut > wrote: >> We tend to use the term "primary" instead of "master". > > Thanks, I will use primary instead of master in my next

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy > Thanks, my concern is suppose you have 3 server in cluster A(new version), > B(new version), C(old version). If we implement as above only new servers > will send ParameterStatus message

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Nov 14, 2016 at 1:37 PM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: > No, there's no concern about compatibility. Please look at this: > https://www.postgresql.org/docs/devel/static/protocol- flow.html#PROTOCOL-ASYNC Thanks, my concern is suppose you have 3 server in

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy > If you are suggesting me to change in protocol messages, I think that would > not be backward compatible to older version servers. I also think such level > of protocol changes will not

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Nov 14, 2016 at 11:31 AM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: > PGconn->target_server_type is not freed in freePGconn(). Thanks, will fix in new patch. >Could you add PGTARGETSERVERTYPE environment variable? Like other variables, it will ease testing, since

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Nov 11, 2016 at 7:33 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > We tend to use the term "primary" instead of "master". Thanks, I will use primary instead of master in my next patch. >Will this work with logical replication? I have not tested with logical

Re: [HACKERS] Patch: Implement failover on libpq connect level.

Hi, Mithun Before going deeper into the patch, let me give you some findings. (1) PGconn->target_server_type is not freed in freePGconn(). (2) Could you add PGTARGETSERVERTYPE environment variable? Like other variables, it will ease testing, since users can change the behavior without

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Great, committed. There's still potentially more work to be done here, > because my patch omits some features that were present in Victor's original > submission, like setting the

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 11/9/16 10:05 AM, Mithun Cy wrote: > As in Victor's patch we have a new connection parameter > "target_server_type", It can take 2 values 1. "any" 2. "master" with > DEFAULT as "any". If it's has the value "any" we can connect to any of > the host server (both master(primary) and

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Great, committed. There's still potentially more work to be done here, > because my patch omits some features that were present in Victor's original > submission, like setting the

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy > Yes this patch will only address failover to new master, values "master" > and "any" appeared sufficient for that case. Do you mean that unlike pgJDBC "standby" and "prefer_standby" are

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 10, 2016 at 1:11 PM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: > Why don't you add "standby" and "prefer_standby" as the target_server_type value? Are you thinking that those values are useful with load balancing > feature? Yes this patch will only address failover

Re: [HACKERS] Patch: Implement failover on libpq connect level.

From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy > Among the remaining things I have worked on failover to new master idea. > Below patch implement that idea. This is taken from Victors patch but > rewritten by me to do some cleanup. As

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Nov 3, 2016 at 7:16 PM, Robert Haas wrote: > Great, committed. There's still potentially more work to be done > here, because my patch omits some features that were present in > Victor's original submission, like setting the failover timeout, > optionally

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Nov 2, 2016 at 1:59 PM, Mithun Cy wrote: > On Tue, Nov 1, 2016 at 9:42 PM, Robert Haas wrote: >> Ah, nuts. Thanks, good catch. Should be fixed in the attached version. > > I repeated the test on new patch, It works fine now, Also did

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 9:42 PM, Robert Haas wrote: > Ah, nuts. Thanks, good catch. Should be fixed in the attached version. I repeated the test on new patch, It works fine now, Also did some more negative tests forcibly failing some internal calls. All tests have passed.

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 11:21 AM, Mithun Cy wrote: > On Tue, Nov 1, 2016 at 7:44 PM, Robert Haas wrote: >>> Starting program: /home/mithun/libpqbin/bin/./psql >>> postgres://%2home%2mithun:/postgres -U mithun1 >>Can you provide a concrete

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 7:44 PM, Robert Haas wrote: >> Starting program: /home/mithun/libpqbin/bin/./psql >> postgres://%2home%2mithun:/postgres -U mithun1 >Can you provide a concrete test scenario or some test code that fails? >connhost is supposed to be getting set in

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 9:43 AM, Mithun Cy wrote: >> Starting program: /home/mithun/libpqbin/bin/./psql >> postgres://%2fhome%2fmithun:/postgres -U mithun1 >> [Thread debugging using libthread_db enabled] >> Using host libthread_db library

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 6:54 PM, Robert Haas wrote: >That's the wrong syntax. If you look in > https://www.postgresql.org/docs/devel/static/libpq-connect.html under > "32.1.1.2. Connection URIs", it gives an example of how to include a > slash in a pathname. You have to

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Nov 1, 2016 at 2:03 AM, Mithun Cy wrote: > On Thu, Oct 27, 2016 at 11:15 PM, Robert Haas wrote: >> Thanks. Here's a new version with a fix for that issue and also a fix >> for PQconnectionNeedsPassword(), which was busted in v1. > I did

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Oct 27, 2016 at 11:15 PM, Robert Haas wrote: > Thanks. Here's a new version with a fix for that issue and also a fix > for PQconnectionNeedsPassword(), which was busted in v1. I did some more testing of the patch for both URI and (host, port) parameter pairs. I did

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Oct 27, 2016 at 9:46 AM, Mithun Cy wrote: > On Wed, Oct 26, 2016 at 8:49 PM, Robert Haas wrote: >> Let me know your thoughts. > > One small issue. I tried to run make installcheck after applying patch there > seems to be a invalid write

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Oct 26, 2016 at 8:49 PM, Robert Haas wrote: > Let me know your thoughts. One small issue. I tried to run make installcheck after applying patch there seems to be a invalid write access in code (resulting in crash). ==118997== Invalid write of size 1 ==118997==

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Oct 24, 2016 at 11:57 AM, Robert Haas wrote: > So now I think that to make this work correctly, we're going to need > to change both the URL parser and also add parsing for the host and > port. Let's say the user says this: > >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Oct 24, 2016 at 4:15 PM, Peter Eisentraut wrote: > On 10/24/16 11:57 AM, Robert Haas wrote: >> Today, since the host part can't include a >> port specifier, it's regarded as part of the IP address, and I think >> it would probably be a bad idea to change

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Oct 24, 2016 at 4:40 PM, Alvaro Herrera wrote: > Umm, my recollection regarding IPv6 parsing in the URI syntax is that > those must appear inside square brackets -- it's not valid to have the > IPv6 address outside brackets, and the port number is necessarily >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, Oct 25, 2016 at 2:10 AM, Alvaro Herrera wrote: > Robert Haas wrote: > > >> It is not obvious what it means if there are multiple ports but the >> number doesn't equal the number of hosts. > > I think we should reject the case of differing number of elements and >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

Robert Haas wrote: > While I was experimenting with this today, I discovered a problem of > interpretation related to IPv6 addresses. Internally, a postgresql:// > URL and a connection string are converted into the same format, so > postgresql://a,b/ means just the same thing as host=a,b. I

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 10/24/16 11:57 AM, Robert Haas wrote: > Today, since the host part can't include a > port specifier, it's regarded as part of the IP address, and I think > it would probably be a bad idea to change that, as I believe Victor's > patch would. He seems to have it in mind that we could allow

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 10/24/16 6:36 AM, Thom Brown wrote: > So should it be the case that it disallows UNIX socket addresses > entirely? I can't imagine a list of UNIX socket addresses being that > useful. But maybe a mixed list of Unix-domain sockets and TCP connections? The nice thing is that is it currently

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, 19 Oct 2016 12:04:27 -0400 Robert Haas wrote: > On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner > wrote: > > On Thu, 13 Oct 2016 12:30:59 +0530 > > Mithun Cy wrote: > >> On Fri, Sep 30, 2016 at 2:14 PM, Victor

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Oct 19, 2016 at 7:26 PM, Peter van Hardenberg wrote: > Supporting different ports on different servers would be a much appreciated > feature (I can't remember if it was Kafka or Cassandra that didn't do this > and it was very annoying.) > > Remember, as the connection string

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, 19 Oct 2016 20:15:38 -0400 Robert Haas wrote: > > Some more comments: > > - I am pretty doubtful that the changes to connectOptions2() work as > intended. I think that's only going to be called before actualhost > and actualport could possibly get set. I don't

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, 24 Oct 2016 11:36:31 +0100 Thom Brown wrote: > > - It's pretty clear that this isn't going to work if the host list > > includes a mix of hostnames and UNIX socket addresses. The code > > that handles the UNIX socket address case is totally separate from > > the code

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 20 October 2016 at 01:15, Robert Haas wrote: > > On Wed, Oct 19, 2016 at 12:04 PM, Robert Haas wrote: > > On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner wrote: > >> On Thu, 13 Oct 2016 12:30:59 +0530 > >> Mithun Cy

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Oct 19, 2016 at 12:04 PM, Robert Haas wrote: > On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner wrote: >> On Thu, 13 Oct 2016 12:30:59 +0530 >> Mithun Cy wrote: >>> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Oct 19, 2016 at 3:08 PM, Robert Haas wrote: > On Mon, Oct 26, 2015 at 4:25 PM, Peter Eisentraut wrote: > > On 10/14/15 6:41 AM, Victor Wagner wrote: > All in all, I'm still feeling pretty good about trying to support the > same syntax that our

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, Oct 26, 2015 at 4:25 PM, Peter Eisentraut wrote: > On 10/14/15 6:41 AM, Victor Wagner wrote: >> 1. It is allowed to specify several hosts in the connect string, either >> in URL-style (separated by comma) or in param=value form (several host >> parameters). > > I'm not

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On 13 October 2016 at 10:53, Victor Wagner wrote: > On Thu, 13 Oct 2016 12:30:59 +0530 > Mithun Cy wrote: > >> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner >> wrote: > >> Okay but for me consistency is also important. Since

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner wrote: > On Thu, 13 Oct 2016 12:30:59 +0530 > Mithun Cy wrote: >> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner >> wrote: >> Okay but for me consistency is also important. Since

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, 13 Oct 2016 12:30:59 +0530 Mithun Cy wrote: > On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner > wrote: > Okay but for me consistency is also important. Since we agree to > disagree on some of the comments and others have not expressed any

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner wrote: >Ok, some trailing whitespace and mixing of tabs and spaces >which git apply doesn't like really present in the patch. >I'm attached hear version with these issues resolved. There were some more trailing spaces and spaces

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Sep 30, 2016 at 5:44 PM, Victor Wagner wrote: > But backward compatibility is more > important thing, so I now assume, that user tries to connect just one > node, and this node is read only, user knows what he is doing. Moved this patch to next CF. (Note that I am in

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Thu, 29 Sep 2016 23:45:52 +0530 Mithun Cy wrote: > This patch do not apply on latest code. it fails as follows It's strange. I have no problems applying it to commit fd321a1dfd64d30 Ok, some trailing whitespace and mixing of tabs and spaces which git apply

Re: [HACKERS] Patch: Implement failover on libpq connect level.

This patch do not apply on latest code. it fails as follows libpq-failover-9.patch:176: trailing whitespace. thread.o pgsleep.o libpq-failover-9.patch:184: trailing whitespace. check: libpq-failover-9.patch:185: trailing whitespace. $(prove_check) libpq-failover-9.patch:186: trailing whitespace.

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Sun, 25 Sep 2016 17:31:53 +0530 Mithun Cy wrote: > I have some more comments on libpq-failover-8.patch > > + /* FIXME need to check that port is numeric */ > > Is this still applicable?. > Unfortunately, it was. I've fixed this problem in 9-th version of patch

Re: [HACKERS] Patch: Implement failover on libpq connect level.

I have some more comments on libpq-failover-8.patch + /* FIXME need to check that port is numeric */ Is this still applicable?. 1) + /* + * if there is more than one host in the connect string and + * target_server_type is not specified explicitely, set + * target_server_type to "master" +

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, Sep 9, 2016 at 4:49 AM, Victor Wagner wrote: > Random permutation is much more computationally complex than random > picking. It really isn't. The pseudocode given at https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle is all of 4 lines long, and one of those

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Fri, 9 Sep 2016 11:20:59 +0530 Mithun Cy wrote: > If concern is only about excluding address which was tried > previously. Then I think we can try this way, randomly permute given > address list (for example fisher-yates shuffle) before trying any of > those

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Wed, Sep 7, 2016 at 7:26 PM, Victor Wagner wrote: > No, algorithm here is more complicated. It must ensure that there would > not be second attempt to connect to host, for which unsuccessful > connection attempt was done. So, there is list rearrangement. >Algorithm for pick

Re: [HACKERS] Patch: Implement failover on libpq connect level.

> Hello, Victor. > > > I'm sending new version of patch. > > > > I've replaced readonly option with target_server_type (with JDBC > > compatibility alias targetServerType), > > > > use logic of setting defaults based on number of hosts in the connect > > string instead of complicated condition

Re: [HACKERS] Patch: Implement failover on libpq connect level.

Hello, Victor. > I'm sending new version of patch. > > I've replaced readonly option with target_server_type (with JDBC > compatibility alias targetServerType), > > use logic of setting defaults based on number of hosts in the connect > string instead of complicated condition in the state

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Tue, 6 Sep 2016 07:58:28 +0530 Mithun Cy wrote: > On Mon, Sep 5, 2016 at 4:33 PM, Aleksander Alekseev < > a.aleks...@postgrespro.ru> wrote: > >After a brief examination I've found following ways to improve the > >patch. > Adding to above comments. > I'm sending

Re: [HACKERS] Patch: Implement failover on libpq connect level.

> > 8) get_next_element procedure implementation is way too smart (read > > "complicated"). You could probably just store current list length and > > generate a random number between 0 and length-1. > > No, algorithm here is more complicated. It must ensure that there would > not be second

Re: [HACKERS] Patch: Implement failover on libpq connect level.

On Mon, 5 Sep 2016 14:03:11 +0300 Aleksander Alekseev wrote: > Hello, Victor. > > > 1) It looks like code is not properly formatted. > Thanks for pointing to the documentation and formatting problems. I'll fix them > > static int > > connectDBStart(PGconn

  1   2   >