Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-05 Thread Fujii Masao
On 2020/10/05 20:32, Bharath Rupireddy wrote: On Mon, Oct 5, 2020 at 9:45 AM Fujii Masao wrote: I see the errmsg() with plain texts in other places in the code base as well. Is it that we look at the error message and if it is a plain text(without database objects or table data), we

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-05 Thread Bharath Rupireddy
On Mon, Oct 5, 2020 at 9:45 AM Fujii Masao wrote: > > > I see the errmsg() with plain texts in other places in the code base > > as well. Is it that we look at the error message and if it is a plain > > text(without database objects or table data), we decide to have no > > translation? Or is

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-04 Thread Fujii Masao
On 2020/10/03 20:40, Bharath Rupireddy wrote: On Fri, Oct 2, 2020 at 11:30 PM Fujii Masao wrote: Attaching v8 patch, please review it.. Both make check and make check-world passes on v8. Thanks for updating the patch! It basically looks good to me. I tweaked the patch as follows. +

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-03 Thread Bharath Rupireddy
On Fri, Oct 2, 2020 at 11:30 PM Fujii Masao wrote: > > > Attaching v8 patch, please review it.. Both make check and make > > check-world passes on v8. > > Thanks for updating the patch! It basically looks good to me. > I tweaked the patch as follows. > > + if (!entry->conn || > +

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-02 Thread Fujii Masao
On 2020/10/02 0:46, Bharath Rupireddy wrote: On Thu, Oct 1, 2020 at 8:10 PM Fujii Masao wrote: pg_stat_clear_snapshot() can be used to reset the entry. Thanks. I wasn't knowing it. + EXIT WHEN proccnt = 0; +END LOOP; Isn't it better to sleep here, to avoid th busy

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-01 Thread Bharath Rupireddy
On Thu, Oct 1, 2020 at 8:10 PM Fujii Masao wrote: > > pg_stat_clear_snapshot() can be used to reset the entry. > Thanks. I wasn't knowing it. > > + EXIT WHEN proccnt = 0; > +END LOOP; > > Isn't it better to sleep here, to avoid th busy loop? > +1. > > So what I thought was

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-01 Thread Fujii Masao
On 2020/10/01 21:14, Bharath Rupireddy wrote: On Wed, Sep 30, 2020 at 11:32 PM Fujii Masao wrote: And another way, if we don't want to use wait_pid() is to have a plpgsql stored procedure, that in a loop keeps on checking for the backed pid from pg_stat_activity, exit when pid is 0. and

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-10-01 Thread Bharath Rupireddy
On Wed, Sep 30, 2020 at 11:32 PM Fujii Masao wrote: > > > And another way, if we don't want to use wait_pid() is to have a > > plpgsql stored procedure, that in a loop keeps on checking for the > > backed pid from pg_stat_activity, exit when pid is 0. and then proceed > > to issue the next

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-30 Thread Fujii Masao
On 2020/09/30 21:02, Bharath Rupireddy wrote: On Tue, Sep 29, 2020 at 10:01 PM Fujii Masao wrote: I think this is okay, because pg_terminate_backend() sends SIGTERM to the backend, and upon receiving SIGTERM the signal handler die() will be called and since there is no query being

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-30 Thread Bharath Rupireddy
On Tue, Sep 29, 2020 at 10:01 PM Fujii Masao wrote: > > > I think this is okay, because pg_terminate_backend() sends SIGTERM to > > the backend, and upon receiving SIGTERM the signal handler die() will > > be called and since there is no query being executed on the backend by > > the time SIGTERM

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-29 Thread Fujii Masao
On 2020/09/30 0:50, Bharath Rupireddy wrote: Thanks for the comments. On Tue, Sep 29, 2020 at 7:30 PM Fujii Masao wrote: +1 to add debug3 message there. But this message doesn't seem to match with what the error actually happened. What about something like "could not start remote

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-29 Thread Bharath Rupireddy
Thanks for the comments. On Tue, Sep 29, 2020 at 7:30 PM Fujii Masao wrote: > > +1 to add debug3 message there. But this message doesn't seem to > match with what the error actually happened. What about something like > "could not start remote transaction on connection %p", instead? > Looks

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-29 Thread Fujii Masao
On 2020/09/25 21:19, Bharath Rupireddy wrote: On Fri, Sep 25, 2020 at 3:21 PM Fujii Masao mailto:masao.fu...@oss.nttdata.com>> wrote: > > I think that we can simplify the code by merging the connection-retry > code into them, like the attached very WIP patch (based on yours) does. > +1.

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-25 Thread Bharath Rupireddy
On Fri, Sep 25, 2020 at 3:21 PM Fujii Masao wrote: > > I think that we can simplify the code by merging the connection-retry > code into them, like the attached very WIP patch (based on yours) does. > +1. > > + else > + ereport(ERROR, > +

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-25 Thread Fujii Masao
On 2020/09/25 13:56, Bharath Rupireddy wrote: On Wed, Sep 23, 2020 at 8:19 PM Fujii Masao wrote: Please let me know if okay with the above agreed points, I will work on the new patch. Yes, please work on the patch! Thanks! I may revisit the above points later, though ;) Thanks,

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-24 Thread Bharath Rupireddy
On Wed, Sep 23, 2020 at 8:19 PM Fujii Masao wrote: > > > Please let me know if okay with the above agreed points, I will work on the > > new patch. > > Yes, please work on the patch! Thanks! I may revisit the above points later, > though ;) > Thanks, attaching v4 patch. Please consider this

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-23 Thread Fujii Masao
On 2020/09/21 12:44, Bharath Rupireddy wrote: On Thu, Sep 17, 2020 at 10:20 PM Fujii Masao mailto:masao.fu...@oss.nttdata.com>> wrote: > > In 1st way, you may also need to call ReleaseExternalFD() when new connection fails > to be made, as connect_pg_server() does. Also we need to check

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-20 Thread Bharath Rupireddy
On Thu, Sep 17, 2020 at 10:20 PM Fujii Masao wrote: > > In 1st way, you may also need to call ReleaseExternalFD() when new connection fails > to be made, as connect_pg_server() does. Also we need to check that > non-superuser has used password to make new connection, > as connect_pg_server()

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-17 Thread Fujii Masao
On 2020/09/17 15:44, Bharath Rupireddy wrote: Thanks for the review comments. I will post a new patch soon addressing all the comments. Thanks a lot! On Tue, Sep 15, 2020 at 2:49 PM Fujii Masao wrote: + PQreset(entry->conn); Isn't using PQreset() to reconnect

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-17 Thread Bharath Rupireddy
Thanks for the review comments. I will post a new patch soon addressing all the comments. On Tue, Sep 15, 2020 at 2:49 PM Fujii Masao wrote: > > + PQreset(entry->conn); > > Isn't using PQreset() to reconnect to the foreign server unsafe? > When new connection is

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-09-15 Thread Fujii Masao
On 2020/07/17 21:02, Bharath Rupireddy wrote: On Tue, Jul 14, 2020 at 6:13 PM Ashutosh Bapat wrote: Has this been added to CF, possibly next CF? I have not added yet. Request to get it done in this CF, as the final patch for review(v3 patch) is ready and shared. We can target it to the

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-17 Thread Bharath Rupireddy
> > On Tue, Jul 14, 2020 at 6:13 PM Ashutosh Bapat > wrote: > > > > Has this been added to CF, possibly next CF? > > > > I have not added yet. Request to get it done in this CF, as the final > patch for review(v3 patch) is ready and shared. We can target it to > the next CF if there are major

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-14 Thread Bharath Rupireddy
On Tue, Jul 14, 2020 at 6:13 PM Ashutosh Bapat wrote: > > Has this been added to CF, possibly next CF? > I have not added yet. Request to get it done in this CF, as the final patch for review(v3 patch) is ready and shared. We can target it to the next CF if there are major issues with the patch.

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-14 Thread Ashutosh Bapat
Has this been added to CF, possibly next CF? On Tue, Jul 14, 2020 at 7:27 AM Bharath Rupireddy wrote: > > > > > You could get a backend's PID using PQbackendPID and then kill it by > > calling pg_terminate_backend() to kill the remote backend to automate > > scenario of remote backend being

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-13 Thread Bharath Rupireddy
> > You could get a backend's PID using PQbackendPID and then kill it by > calling pg_terminate_backend() to kill the remote backend to automate > scenario of remote backend being killed. > I already added the test case in v2 patch itself(added one more test case in v3 patch), using the similar

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-12 Thread Ashutosh Bapat
On Wed, Jul 8, 2020 at 6:10 PM Bharath Rupireddy wrote: > > I couldn't think of adding a test case to the existing postgres_fdw > regression test suite with an automated scenario of the remote backend > getting killed. You could get a backend's PID using PQbackendPID and then kill it by calling

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-11 Thread Bharath Rupireddy
Thanks for the comments. Attaching the v2 patch. > > > One way, we could solve the above problem is that, upon firing the new > > foreign query from local backend using the cached connection, > > (assuming the remote backend that was cached in the local backed got > > killed for some reasons),

Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-10 Thread Kasahara Tatsuhito
Hi, On Wed, Jul 8, 2020 at 9:40 PM Bharath Rupireddy wrote: > One way, we could solve the above problem is that, upon firing the new > foreign query from local backend using the cached connection, > (assuming the remote backend that was cached in the local backed got > killed for some reasons),

Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

2020-07-08 Thread Bharath Rupireddy
Hi, Currently with the postgres_fdw remote connections cached in the local backend, the queries that use the cached connections from local backend will not check whether the remote backend is killed or gone away, and it goes tries to submit the query and fails if the remote backend is killed.