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. */ >>conn->addr_cur = NULL; >>

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 (conn->whichhost + 1 < conn->nconnhost)

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 'at

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 variable in PQconnectPoll, Initally I thought > savedMessa

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 errorMessage so we can res

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(&conn->errorMessage); > > Please find the patch which fixes this bug. conn->e

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(&conn->errorMessage);* *Please find the patch which fixes this bug. **conn->errorMessage as per definition only stores current erro

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 getting cleared someplace in there. We > might need

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 target_server_type to >>> target_session_attrs with possible 2 values "read-write", "

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". >> May be we could expand to "readonly" and "prefer-readon

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. Attaching the patch for same.

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 server where the transaction is read-only, > then shouldn't the connection parameter b

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 something like >>"target_se

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 > the ideal solution. I agree with

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 patch omits some features that wer

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 primary based >> on the server ve

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 you > forgive the additional ro

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 standby or not, use that. > >

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 fiel

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 fi

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 >> talking to newer servers

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 against pre-V10 servers? I >>> don't think so, as libpq is a part of Postgre

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 and libpq failover is a new >> feature in PostgreSQL 10.

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. Tsunakawa's comments 1. PGconn

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, I don't want PostgreSQL to > sa

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 i

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 con

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 > >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Mithun Cy > >>

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 any other non-writable node. You could even force

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 wanted to force traffic

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 and can change tens of thous

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 started with it, but later i

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() return

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 appea

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 in both cases that we >>> can extend internally later, say pg_is_primary_

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 me think of another complic

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: what about cascading replica

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 something like >> that instead; and we implement it

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 result of "SELECT pg_is_in_recovery()". So I >> > think

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 particular setup? > > If logical

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 version), C(old version). If we implem

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 patch. > >>Will this work with logical replication? > > I have not t

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 cl

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 b

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 users

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 replicatio

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 alterin

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 fail

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 slave(standby))

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 fail

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 t

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 i

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 randomizing the order of the host

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 some more > negative tests forcibly failing some inte

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. This patch works as des

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 test scenario or some test code that fails? >>connhost

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 connectOptions2(), which

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 "/lib64/libthread_db.so.1". >> psql: could not connect

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 use %2F, or else the URL p

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 some more testing of the patch for both URI and (ho

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 test for ipv4, ipv6, UD

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 access in code (resulting in crash). > > ==118997==

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==at 0x4E3DDF1: connectOpt

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: > > postgresql://[1::2]:3,[4::5],[6::7]::8/ > > What I think we n

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 that, as I believe Victor's >> patc

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 > outside the brackets. So th

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 > neither host nor port is a

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 thou

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 things

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 tra

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 Wagner > >> wrote: > >> Okay but for me consistency is also important. Sinc

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 gets more comp

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 think we keep > reinvoki

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 that handles the mu

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 wrote: > >>> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner > >>> wrote: > >

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 >>> wrote: >>> Okay but for me consistency is also important. Since we agree

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 JDBC driver already does. It's certainly no

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 fond of having URLs

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 we agree to >> disagree on some of the comments and others have not ex

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 we agree to >> disagree on some of the comments and others have not ex

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 > problem I am moving it to committer. Thank yo

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 used for tabs in patc

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 CF-vacuuming mode th

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 doesn't like really present in the

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. l

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 (attached) > > I think we ne

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 lines is a comment.

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 address in it. After that you can t

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 random list element

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 machi

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 new version of patch. I've r

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 attempt

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 *conn) > > { > > + struc

  1   2   >