Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-06-07 Thread Robert Haas
On Wed, Jun 7, 2017 at 11:20 AM, Amit Kapila wrote: > On Sat, Jun 3, 2017 at 1:03 AM, Robert Haas wrote: >> On Fri, Jun 2, 2017 at 3:48 AM, Rafia Sabih >> wrote: >> I don't see how to do that. It could possibly be done with the TAP >> framework, but that exceeds my abilities. >> >> Here's an up

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-06-07 Thread Amit Kapila
On Sat, Jun 3, 2017 at 1:03 AM, Robert Haas wrote: > On Fri, Jun 2, 2017 at 3:48 AM, Rafia Sabih > wrote: > > I don't see how to do that. It could possibly be done with the TAP > framework, but that exceeds my abilities. > > Here's an updated patch with a bunch of cosmetic fixes, plus I > adjust

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-06-02 Thread Robert Haas
On Fri, Jun 2, 2017 at 3:48 AM, Rafia Sabih wrote: > I tried following the sketch stated above, and here's what I added to > v2 of the patch. In begin_remote_xact when savepoint is send to the > remote server through do_sql_command I set the changing_xact_state > flag and when it successfully retu

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-06-02 Thread Rafia Sabih
On Tue, May 16, 2017 at 9:39 PM, Robert Haas wrote: > On Sun, May 7, 2017 at 11:54 AM, Robert Haas wrote: >> I'm having second thoughts based on some more experimentation I did >> this morning. I'll update again once I've had a bit more time to poke >> at it. > > So the issue that I noticed here

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-20 Thread Amit Kapila
On Thu, May 18, 2017 at 7:56 PM, Robert Haas wrote: > On Wed, May 17, 2017 at 6:57 AM, Amit Kapila wrote: >> +1. Why not similar behavior for any other statements executed in >> this module by do_sql_command? > > The other cases are not quite the same situation. It would be good to > accept int

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-18 Thread Robert Haas
On Wed, May 17, 2017 at 6:57 AM, Amit Kapila wrote: > +1. Why not similar behavior for any other statements executed in > this module by do_sql_command? The other cases are not quite the same situation. It would be good to accept interrupts in all cases, but there's no problem with a session co

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-17 Thread Amit Kapila
On Tue, May 16, 2017 at 9:39 PM, Robert Haas wrote: > On Sun, May 7, 2017 at 11:54 AM, Robert Haas wrote: >> I'm having second thoughts based on some more experimentation I did >> this morning. I'll update again once I've had a bit more time to poke >> at it. > > So the issue that I noticed here

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-16 Thread Ashutosh Bapat
On Wed, May 17, 2017 at 7:24 AM, Kyotaro HORIGUCHI wrote: > Hello, > > At Tue, 16 May 2017 12:45:39 -0400, Tom Lane wrote in > <22556.1494953...@sss.pgh.pa.us> >> Robert Haas writes: >> > Concretely, I think we should replace the abort_cleanup_incomplete >> > flag from my previous patch with a

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-16 Thread Kyotaro HORIGUCHI
Hello, At Tue, 16 May 2017 12:45:39 -0400, Tom Lane wrote in <22556.1494953...@sss.pgh.pa.us> > Robert Haas writes: > > Concretely, I think we should replace the abort_cleanup_incomplete > > flag from my previous patch with a changing_xact_state flag and set > > that flag around all transaction

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-16 Thread Tom Lane
Robert Haas writes: > Concretely, I think we should replace the abort_cleanup_incomplete > flag from my previous patch with a changing_xact_state flag and set > that flag around all transaction state changes, clearing it when such > changes have succeeded. On error, the flag remains set, so we kn

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-16 Thread Robert Haas
On Sun, May 7, 2017 at 11:54 AM, Robert Haas wrote: > I'm having second thoughts based on some more experimentation I did > this morning. I'll update again once I've had a bit more time to poke > at it. So the issue that I noticed here is that this problem really isn't confined to abort processi

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-07 Thread Robert Haas
On Sun, May 7, 2017 at 12:42 AM, Amit Kapila wrote: > On Sat, May 6, 2017 at 10:41 PM, Robert Haas wrote: >> On Sat, May 6, 2017 at 12:55 PM, Robert Haas wrote: >>> Oh! Good catch. Given that the behavior in question is intentional >>> there and intended to provide backward compatibility, chan

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-06 Thread Amit Kapila
On Sat, May 6, 2017 at 10:41 PM, Robert Haas wrote: > On Sat, May 6, 2017 at 12:55 PM, Robert Haas wrote: >> Oh! Good catch. Given that the behavior in question is intentional >> there and intended to provide backward compatibility, changing it in >> back-branches doesn't seem like a good plan.

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-06 Thread Robert Haas
On Sat, May 6, 2017 at 12:55 PM, Robert Haas wrote: > Oh! Good catch. Given that the behavior in question is intentional > there and intended to provide backward compatibility, changing it in > back-branches doesn't seem like a good plan. I'll adjust the patch so > that it continues to ignore e

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-06 Thread Robert Haas
On Fri, May 5, 2017 at 7:11 PM, Amit Kapila wrote: > There is a comment in the code which explains why currently we ignore > errors from DEALLOCATE ALL. > > * DEALLOCATE ALL only exists in 8.3 and later, so this > * constrains how old a server postgres_fdw can > * communicate with. We intentional

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-05 Thread Amit Kapila
On Sat, May 6, 2017 at 4:41 AM, Amit Kapila wrote: > On Fri, May 5, 2017 at 9:32 PM, Robert Haas wrote: >> On Fri, May 5, 2017 at 11:15 AM, Amit Kapila wrote: >>> I am not saying to rip those changes. Your most of the mail stresses >>> about the 30-second timeout which you have added in the pat

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-05 Thread Amit Kapila
On Fri, May 5, 2017 at 9:32 PM, Robert Haas wrote: > On Fri, May 5, 2017 at 11:15 AM, Amit Kapila wrote: >> I am not saying to rip those changes. Your most of the mail stresses >> about the 30-second timeout which you have added in the patch to >> detect timeouts during failures (rollback proces

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-05 Thread Robert Haas
On Fri, May 5, 2017 at 11:15 AM, Amit Kapila wrote: > I am not saying to rip those changes. Your most of the mail stresses > about the 30-second timeout which you have added in the patch to > detect timeouts during failures (rollback processing). I have no > objection to that part of the patch e

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-05 Thread Amit Kapila
On Fri, May 5, 2017 at 7:44 PM, Robert Haas wrote: > On Fri, May 5, 2017 at 1:01 AM, Amit Kapila wrote: >> I have tried to verify above point and it seems to me in such a >> situation the transaction status will be either PQTRANS_INTRANS or >> PQTRANS_INERROR. I have tried to mimic "out of memor

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-05 Thread Robert Haas
On Fri, May 5, 2017 at 1:01 AM, Amit Kapila wrote: > I have tried to verify above point and it seems to me in such a > situation the transaction status will be either PQTRANS_INTRANS or > PQTRANS_INERROR. I have tried to mimic "out of memory" failure by > making PQmakeEmptyPGresult return NULL (m

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Amit Kapila
On Thu, May 4, 2017 at 10:40 PM, Robert Haas wrote: > On Thu, May 4, 2017 at 1:04 PM, Amit Kapila wrote: >> On Thu, May 4, 2017 at 10:18 PM, Robert Haas wrote: >>> On Thu, May 4, 2017 at 12:18 PM, Amit Kapila >>> wrote: As soon as the first command fails due to timeout, we will set '

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Amit Kapila
On Thu, May 4, 2017 at 8:08 PM, Robert Haas wrote: > On Thu, May 4, 2017 at 7:13 AM, Amit Kapila wrote: >> In pgfdw_xact_callback, if the execution of ABORT TRANSACTION fails >> due to any reason then I think it will close the connection. The >> relavant code is: >> if (PQstatus(entry->conn) !=

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Robert Haas
On Thu, May 4, 2017 at 1:04 PM, Amit Kapila wrote: > On Thu, May 4, 2017 at 10:18 PM, Robert Haas wrote: >> On Thu, May 4, 2017 at 12:18 PM, Amit Kapila wrote: >>> As soon as the first command fails due to timeout, we will set >>> 'abort_cleanup_failure' which will make a toplevel transaction to

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Amit Kapila
On Thu, May 4, 2017 at 10:18 PM, Robert Haas wrote: > On Thu, May 4, 2017 at 12:18 PM, Amit Kapila wrote: >> As soon as the first command fails due to timeout, we will set >> 'abort_cleanup_failure' which will make a toplevel transaction to >> abort and also won't allow other statements to execut

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Robert Haas
On Thu, May 4, 2017 at 12:18 PM, Amit Kapila wrote: > As soon as the first command fails due to timeout, we will set > 'abort_cleanup_failure' which will make a toplevel transaction to > abort and also won't allow other statements to execute. The patch is > trying to enforce a 30-second timeout a

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Amit Kapila
On Thu, May 4, 2017 at 8:08 PM, Robert Haas wrote: > On Thu, May 4, 2017 at 7:13 AM, Amit Kapila wrote: > >>> - For bonus points, give pgfdw_exec_query() an optional timeout >>> argument, and set it to 30 seconds or so when we're doing abort >>> cleanup. If the timeout succeeds, it errors out (w

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Robert Haas
On Thu, May 4, 2017 at 7:13 AM, Amit Kapila wrote: > In pgfdw_xact_callback, if the execution of ABORT TRANSACTION fails > due to any reason then I think it will close the connection. The > relavant code is: > if (PQstatus(entry->conn) != CONNECTION_OK || > PQtransactionStatus(entry->conn) != PQT

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Amit Kapila
On Thu, May 4, 2017 at 1:19 AM, Robert Haas wrote: > On Thu, Apr 20, 2017 at 10:27 AM, Ashutosh Bapat > wrote: >> The logs above show that 34 seconds elapsed between starting to abort >> the transaction and knowing that the foreign server is unreachable. It >> looks like it took that much time fo

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread Robert Haas
On Thu, May 4, 2017 at 6:23 AM, tushar wrote: > We can see statement_timeout is working but it is taking some extra time,not > sure this is an expected behavior in above case or not. Yeah, that's expected. To fix that, we'd need libpq to have an async equivalent of PQcancel(), which doesn't curr

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread tushar
On 05/04/2017 03:53 PM, tushar wrote: We can see statement_timeout is working but it is taking some extra time,not sure this is an expected behavior in above case or not. This is only when remote server is involved . in case when both the servers are on the same machine , then this is working a

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-04 Thread tushar
On 05/04/2017 08:01 AM, Robert Haas wrote: Patch attached. I tried at my end after applying the patch against PG HEAD, Case 1 - without setting statement_timeout i.e default X machine - create table test1(a int); Y machine - CREATE SERVER myserver_ppas FOREIGN DATA WRAPPER postgres_fdw OPTIO

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-03 Thread Robert Haas
On Wed, May 3, 2017 at 3:49 PM, Robert Haas wrote: > It seems pretty clear to me that we are not going to fix all of these > issues in one patch. Here's a sketch of an idea for how to start > making things better: Patch attached. > - Add an in_abort_cleanup flag to ConnCacheEntry. Ended up ren

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-05-03 Thread Robert Haas
On Thu, Apr 20, 2017 at 10:27 AM, Ashutosh Bapat wrote: > The logs above show that 34 seconds elapsed between starting to abort > the transaction and knowing that the foreign server is unreachable. It > looks like it took that much time for the local server to realise that > the foreign server is

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-04-26 Thread Kyotaro HORIGUCHI
At Tue, 25 Apr 2017 15:43:59 +0530, Ashutosh Bapat wrote in > On Tue, Apr 25, 2017 at 1:31 PM, Kyotaro HORIGUCHI > wrote: > >> > >> The logs above show that 34 seconds elapsed between starting to abort > >> the transaction and knowing that the foreign server is unreachable. It > >> looks like

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-04-25 Thread Ashutosh Bapat
On Tue, Apr 25, 2017 at 1:31 PM, Kyotaro HORIGUCHI wrote: >> >> The logs above show that 34 seconds elapsed between starting to abort >> the transaction and knowing that the foreign server is unreachable. It >> looks like it took that much time for the local server to realise that >> the foreign s

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-04-25 Thread Kyotaro HORIGUCHI
Hello, At Thu, 20 Apr 2017 19:57:30 +0530, Ashutosh Bapat wrote in > On Thu, Apr 20, 2017 at 4:08 PM, Suraj Kharage > wrote: > > Hello All, > > > > The query on foreign table hangs due to network down of remote server for > > near about 16 minutes before exiting. > > statement_timeout is expe

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-04-20 Thread Ashutosh Bapat
On Thu, Apr 20, 2017 at 4:08 PM, Suraj Kharage wrote: > Hello All, > > The query on foreign table hangs due to network down of remote server for > near about 16 minutes before exiting. > statement_timeout is expected to work in this case as well but when i tried > statement_timeout, it is not work

[HACKERS] statement_timeout is not working as expected with postgres_fdw

2017-04-20 Thread Suraj Kharage
Hello All, The query on foreign table hangs due to network down of remote server for near about 16 minutes before exiting. statement_timeout is expected to work in this case as well but when i tried statement_timeout, it is not working as expected. Below is test case to reproduce the issue: [I am