Re: A new function to wait for the backend exit after termination

2021-06-14 Thread Noah Misch
On Mon, Jun 14, 2021 at 07:34:59PM +0530, Bharath Rupireddy wrote: > Thanks. +1 to remove the pg_wait_for_backend_termination function. The > patch basically looks good to me. I'm attaching an updated patch. I > corrected a minor typo in the commit message, took docs and code > comment changes sugg

Re: A new function to wait for the backend exit after termination

2021-06-14 Thread Justin Pryzby
On Mon, Jun 14, 2021 at 09:40:27AM -0700, Noah Misch wrote: > > > Agreed, these strings generally give less detail. I can revert that to > > > the > > > v13 wording, 'terminate a server process'. ... > > Maybe you'd also update the release notes. > > What part of the notes did you expect to ch

Re: A new function to wait for the backend exit after termination

2021-06-14 Thread Noah Misch
On Sat, Jun 12, 2021 at 01:27:43PM -0500, Justin Pryzby wrote: > On Sat, Jun 12, 2021 at 08:21:39AM -0700, Noah Misch wrote: > > On Sat, Jun 12, 2021 at 12:12:12AM -0500, Justin Pryzby wrote: > > > Even if it's not removed, the descriptions should be cleaned up. > > > > > > | src/include/catalog/p

Re: A new function to wait for the backend exit after termination

2021-06-14 Thread Bharath Rupireddy
On Sat, Jun 12, 2021 at 10:07 AM Noah Misch wrote: > > On Fri, Jun 11, 2021 at 08:54:08PM -0500, Justin Pryzby wrote: > > On Sat, Jun 05, 2021 at 12:08:01PM -0700, Noah Misch wrote: > > > > > My preference is to remove pg_wait_for_backend_termination(). The > > > > > use case > > > > > that prom

Re: A new function to wait for the backend exit after termination

2021-06-12 Thread Justin Pryzby
On Sat, Jun 12, 2021 at 08:21:39AM -0700, Noah Misch wrote: > On Sat, Jun 12, 2021 at 12:12:12AM -0500, Justin Pryzby wrote: > > Even if it's not removed, the descriptions should be cleaned up. > > > > | src/include/catalog/pg_proc.dat- descr => 'terminate a backend process > > and if timeout is

Re: A new function to wait for the backend exit after termination

2021-06-12 Thread Noah Misch
On Sat, Jun 12, 2021 at 12:12:12AM -0500, Justin Pryzby wrote: > Even if it's not removed, the descriptions should be cleaned up. > > | src/include/catalog/pg_proc.dat- descr => 'terminate a backend process and > if timeout is specified, wait for its exit or until timeout occurs', > => I think d

Re: A new function to wait for the backend exit after termination

2021-06-11 Thread Justin Pryzby
On Fri, Jun 11, 2021 at 09:37:50PM -0700, Noah Misch wrote: > On Fri, Jun 11, 2021 at 08:54:08PM -0500, Justin Pryzby wrote: > > On Sat, Jun 05, 2021 at 12:08:01PM -0700, Noah Misch wrote: > > > > > My preference is to remove pg_wait_for_backend_termination(). The > > > > > use case > > > > > tha

Re: A new function to wait for the backend exit after termination

2021-06-11 Thread Noah Misch
On Fri, Jun 11, 2021 at 08:54:08PM -0500, Justin Pryzby wrote: > On Sat, Jun 05, 2021 at 12:08:01PM -0700, Noah Misch wrote: > > > > My preference is to remove pg_wait_for_backend_termination(). The use > > > > case > > > > that prompted this thread used pg_terminate_backend(pid, 18); it > >

Re: A new function to wait for the backend exit after termination

2021-06-11 Thread Justin Pryzby
On Sat, Jun 05, 2021 at 12:08:01PM -0700, Noah Misch wrote: > > > My preference is to remove pg_wait_for_backend_termination(). The use > > > case > > > that prompted this thread used pg_terminate_backend(pid, 18); it > > > doesn't > > > need pg_wait_for_backend_termination(). Is this an Op

Re: A new function to wait for the backend exit after termination

2021-06-05 Thread Noah Misch
On Sat, Jun 05, 2021 at 12:06:46PM +0530, Bharath Rupireddy wrote: > On Sat, Jun 5, 2021 at 7:02 AM Noah Misch wrote: > > On Tue, Jun 01, 2021 at 01:25:24PM +0530, Bharath Rupireddy wrote: > > > On Tue, Jun 1, 2021 at 9:19 AM Noah Misch wrote: > > > > Given that limitation, is pg_wait_for_backend

Re: A new function to wait for the backend exit after termination

2021-06-04 Thread Bharath Rupireddy
On Sat, Jun 5, 2021 at 7:02 AM Noah Misch wrote: > > On Tue, Jun 01, 2021 at 01:25:24PM +0530, Bharath Rupireddy wrote: > > On Tue, Jun 1, 2021 at 9:19 AM Noah Misch wrote: > > > Given that limitation, is pg_wait_for_backend_termination() useful > > > enough? If > > > waiting for procarray depa

Re: A new function to wait for the backend exit after termination

2021-06-04 Thread Noah Misch
On Tue, Jun 01, 2021 at 01:25:24PM +0530, Bharath Rupireddy wrote: > On Tue, Jun 1, 2021 at 9:19 AM Noah Misch wrote: > > Given that limitation, is pg_wait_for_backend_termination() useful enough? > > If > > waiting for procarray departure is enough, should > > pg_wait_until_termination() > > c

Re: A new function to wait for the backend exit after termination

2021-06-01 Thread Bharath Rupireddy
On Tue, Jun 1, 2021 at 9:19 AM Noah Misch wrote: > > On Thu, Apr 08, 2021 at 11:41:17AM +0200, Magnus Hagander wrote: > > I've applied this patch with some minor changes. Thanks for taking a look at this function. > I wondered if the new pg_wait_for_backend_termination() could replace > regress.

Re: A new function to wait for the backend exit after termination

2021-05-31 Thread Noah Misch
On Thu, Apr 08, 2021 at 11:41:17AM +0200, Magnus Hagander wrote: > I've applied this patch with some minor changes. I wondered if the new pg_wait_for_backend_termination() could replace regress.c:wait_pid(). I think it can't, because the new function requires the backend to still be present in th

Re: A new function to wait for the backend exit after termination

2021-04-08 Thread Magnus Hagander
On Mon, Apr 5, 2021 at 5:21 AM Bharath Rupireddy wrote: > > On Fri, Mar 19, 2021 at 11:37 AM Bharath Rupireddy > wrote: > > Attaching v11 patch that removed the wait boolean flag in the > > pg_terminate_backend and timeout 0 indicates "no wait", negative value > > "errors out", positive value "wa

Re: A new function to wait for the backend exit after termination

2021-04-04 Thread Bharath Rupireddy
On Fri, Mar 19, 2021 at 11:37 AM Bharath Rupireddy wrote: > Attaching v11 patch that removed the wait boolean flag in the > pg_terminate_backend and timeout 0 indicates "no wait", negative value > "errors out", positive value "waits for those many milliseconds". Also > addressed other review comme

Re: A new function to wait for the backend exit after termination

2021-03-18 Thread Bharath Rupireddy
On Thu, Mar 18, 2021 at 1:11 PM Bharath Rupireddy wrote: > > On Thu, Mar 18, 2021 at 12:46 PM Fujii Masao > wrote: > > On 2021/03/17 11:58, Kyotaro Horiguchi wrote: > > > The first suggested signature for pg_terminate_backend() with timeout > > > was pg_terminate_backend(pid, timeout). The curre

Re: A new function to wait for the backend exit after termination

2021-03-18 Thread Bharath Rupireddy
On Thu, Mar 18, 2021 at 12:46 PM Fujii Masao wrote: > On 2021/03/17 11:58, Kyotaro Horiguchi wrote: > > The first suggested signature for pg_terminate_backend() with timeout > > was pg_terminate_backend(pid, timeout). The current signature (pid, > > wait?, timeout) looks redundant. Maybe the rea

Re: A new function to wait for the backend exit after termination

2021-03-18 Thread Fujii Masao
On 2021/03/17 11:58, Kyotaro Horiguchi wrote: The first suggested signature for pg_terminate_backend() with timeout was pg_terminate_backend(pid, timeout). The current signature (pid, wait?, timeout) looks redundant. Maybe the reason for rejecting 0 astimeout is pg_terminate_backend(pid, tru

Re: A new function to wait for the backend exit after termination

2021-03-17 Thread Zhihong Yu
Hi, w.r.t. WaitLatch(), if its return value is WL_TIMEOUT, we know the specified timeout has elapsed. It seems WaitLatch() can be enhanced to also return the actual duration of the wait. This way, the caller can utilize the duration directly. As for other places where WaitLatch() is called, simila

Re: A new function to wait for the backend exit after termination

2021-03-17 Thread Bharath Rupireddy
On Wed, Mar 17, 2021 at 8:28 AM Kyotaro Horiguchi wrote: > > At Wed, 17 Mar 2021 07:01:39 +0530, Bharath Rupireddy > wrote in > > Attaching v10 patch for further review. > > The time-out mechanism doesn't count remainingtime as expected, > concretely it does the following. > > do { > kill(); >

Re: A new function to wait for the backend exit after termination

2021-03-16 Thread Kyotaro Horiguchi
At Wed, 17 Mar 2021 07:01:39 +0530, Bharath Rupireddy wrote in > Attaching v10 patch for further review. The time-out mechanism doesn't count remainingtime as expected, concretely it does the following. do { kill(); WaitLatch(WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, waittime); Re

Re: A new function to wait for the backend exit after termination

2021-03-16 Thread Bharath Rupireddy
On Tue, Mar 16, 2021 at 9:48 PM Magnus Hagander wrote: > Does it really make sense that pg_wait_for_backend_termination() > defaults to waiting *100 milliseconds*, and then logs a warning? That > seems extremely short if I'm explicitly asking it to wait. I increased the default wait timeout to 5s

Re: A new function to wait for the backend exit after termination

2021-03-16 Thread Magnus Hagander
On Tue, Mar 16, 2021 at 10:38 AM Bharath Rupireddy wrote: > > On Mon, Mar 15, 2021 at 10:38 AM Fujii Masao > wrote: > > On 2021/03/15 12:27, Bharath Rupireddy wrote: > > > On Sun, Mar 7, 2021 at 2:39 PM Bharath Rupireddy > > > wrote: > > >> Attaching v7 patch for further review. > > > > > > Atta

Re: A new function to wait for the backend exit after termination

2021-03-16 Thread Bharath Rupireddy
On Mon, Mar 15, 2021 at 10:38 AM Fujii Masao wrote: > On 2021/03/15 12:27, Bharath Rupireddy wrote: > > On Sun, Mar 7, 2021 at 2:39 PM Bharath Rupireddy > > wrote: > >> Attaching v7 patch for further review. > > > > Attaching v8 patch after rebasing on to the latest master. > > Thanks for rebasin

Re: A new function to wait for the backend exit after termination

2021-03-14 Thread Fujii Masao
On 2021/03/15 12:27, Bharath Rupireddy wrote: On Sun, Mar 7, 2021 at 2:39 PM Bharath Rupireddy wrote: Attaching v7 patch for further review. Attaching v8 patch after rebasing on to the latest master. Thanks for rebasing the patch! - WAIT_EVENT_XACT_GROUP_UPDATE + WAIT_EVENT_

Re: A new function to wait for the backend exit after termination

2021-03-14 Thread Bharath Rupireddy
On Sun, Mar 7, 2021 at 2:39 PM Bharath Rupireddy wrote: > Attaching v7 patch for further review. Attaching v8 patch after rebasing on to the latest master. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com v8-0001-pg_terminate_backend-with-wait-and-timeout.patch Descri

Re: A new function to wait for the backend exit after termination

2021-03-07 Thread Bharath Rupireddy
On Sat, Mar 6, 2021 at 10:36 PM Magnus Hagander wrote: > > > Attaching v6 patch. Please have a look. > > Taking another look at this patch. Here are a few more comments: Thanks for the review comments. > For pg_terminate_backend, wouldn't it be easier to just create one > function that has a def

Re: A new function to wait for the backend exit after termination

2021-03-06 Thread Magnus Hagander
On Fri, Dec 4, 2020 at 10:13 AM Bharath Rupireddy wrote: > > On Fri, Dec 4, 2020 at 2:02 PM Hou, Zhijie wrote: > > > > Hi, > > > > When test pg_terminate_backend_and_wait with parallel query. > > I noticed that the function is not defined as parallel safe. > > > > I am not very familiar with the

Re: A new function to wait for the backend exit after termination

2020-12-04 Thread hou zhijie
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested Thanks for the new patch, the patch LGTM and works as expected T

Re: A new function to wait for the backend exit after termination

2020-12-04 Thread Bharath Rupireddy
On Fri, Dec 4, 2020 at 2:02 PM Hou, Zhijie wrote: > > Hi, > > When test pg_terminate_backend_and_wait with parallel query. > I noticed that the function is not defined as parallel safe. > > I am not very familiar with the standard about whether a function should be > parallel safe. > But I found

RE: A new function to wait for the backend exit after termination

2020-12-04 Thread Hou, Zhijie
Hi, When test pg_terminate_backend_and_wait with parallel query. I noticed that the function is not defined as parallel safe. I am not very familiar with the standard about whether a function should be parallel safe. But I found the following function are all defined as parallel safe: pg_promot

Re: A new function to wait for the backend exit after termination

2020-12-03 Thread Bharath Rupireddy
On Fri, Dec 4, 2020 at 8:44 AM Hou, Zhijie wrote: > > -however only superusers can terminate superuser backends. > +however only superusers can terminate superuser backends. When no > +wait and timeout are > +provided, only SIGTERM is sent to the backend with the gi

RE: A new function to wait for the backend exit after termination

2020-12-03 Thread Hou, Zhijie
Hi, -however only superusers can terminate superuser backends. +however only superusers can terminate superuser backends. When no +wait and timeout are +provided, only SIGTERM is sent to the backend with the given process +ID and false is returned immediatel

Re: A new function to wait for the backend exit after termination

2020-12-02 Thread Bharath Rupireddy
Thanks for the review. On Thu, Dec 3, 2020 at 7:24 AM Hou, Zhijie wrote: > > 1. > + > + ereport(WARNING, > + (errmsg("could not wait for the termination of the > backend with PID %d within %ld milliseconds", > + pid, timeout))); >

RE: A new function to wait for the backend exit after termination

2020-12-02 Thread Hou, Zhijie
> > I changed the status to 'wait on anthor'. > The others of the patch LGTM, > I think it can be changed to Ready for Committer again, when this comment > is confirmed. > I am Sorry I forgot a possible typo comment. +{ oid => '16386', descr => 'terminate a backend process and wait for it\'s e

RE: A new function to wait for the backend exit after termination

2020-12-02 Thread Hou, Zhijie
> > + ereport(WARNING, > > + (errmsg("could not wait for the termination of the > backend with PID %d within %ld milliseconds", > > + pid, timeout))); > > > The code use %ld to print int64 type. > > How about use INT64_FORMAT, which looks more

Re: A new function to wait for the backend exit after termination

2020-12-02 Thread Tom Lane
"Hou, Zhijie" writes: > + ereport(WARNING, > + (errmsg("could not wait for the termination of the > backend with PID %d within %ld milliseconds", > + pid, timeout))); > The code use %ld to print int64 type. > How about use INT64_FORMAT,

RE: A new function to wait for the backend exit after termination

2020-12-02 Thread Hou, Zhijie
Hi I take a look into the patch, and here some comments. 1. + + ereport(WARNING, + (errmsg("could not wait for the termination of the backend with PID %d within %ld milliseconds", + pid, timeout))); + The code use %ld to print in

Re: A new function to wait for the backend exit after termination

2020-12-02 Thread Muhammad Usama
On Wed, Dec 2, 2020 at 1:30 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Mon, Nov 30, 2020 at 8:10 PM Muhammad Usama wrote: > > > > The following review has been posted through the commitfest application: > > make installcheck-world: tested, passed > > Implements fe

Re: A new function to wait for the backend exit after termination

2020-12-02 Thread Bharath Rupireddy
On Mon, Nov 30, 2020 at 8:10 PM Muhammad Usama wrote: > > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: tested, passed > Documentation:not tested

Re: A new function to wait for the backend exit after termination

2020-11-30 Thread Muhammad Usama
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested I have tested the patch against current master branch (commit:67

Re: A new function to wait for the backend exit after termination

2020-10-31 Thread Bharath Rupireddy
On Wed, Oct 28, 2020 at 6:41 PM Fujii Masao wrote: > > I prefer that false is returned when the timeout happens, > like pg_promote() does. > Done. > > When the specified timeout is negative, the following error is thrown *after* > SIGTERM is signaled to the target backend. This seems strange to

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread David G. Johnston
On Wed, Oct 28, 2020 at 10:14 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Wed, Oct 28, 2020 at 7:51 PM David G. Johnston > wrote: > > > I was thinking this would be useful for orchestration. However, as you > say, its a pretty fragile method. I withdraw the sugges

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread Bharath Rupireddy
On Wed, Oct 28, 2020 at 7:51 PM David G. Johnston wrote: > >> And also in case if the given pid is not a >> backend pid, we are throwing a warning and returning false but not >> error. >> >> Similarly we can return false on timeout, if required a >> warning. Thoughts? > > IMO, if there are multipl

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread David G. Johnston
On Wed, Oct 28, 2020 at 6:50 AM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > Thanks for the comments. > > On Wed, Oct 28, 2020 at 6:41 PM Fujii Masao > wrote: > > > > I prefer that false is returned when the timeout happens, > > like pg_promote() does. > > > > Earlier it w

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread Bharath Rupireddy
Thanks for the comments. On Wed, Oct 28, 2020 at 6:41 PM Fujii Masao wrote: > > I prefer that false is returned when the timeout happens, > like pg_promote() does. > Earlier it was suggested to error out on timeout. Since users can not guess on time it takes to terminate or become idle, throwing

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread Fujii Masao
On 2020/10/28 20:50, Bharath Rupireddy wrote: On Wed, Oct 21, 2020 at 6:43 PM Magnus Hagander wrote: I think it would be nicer to have a pg_terminate_backend(pid, wait=false), so a function with a second parameter which defaults to the current behaviour of not waiting. And it might be a g

Re: A new function to wait for the backend exit after termination

2020-10-28 Thread Bharath Rupireddy
On Wed, Oct 21, 2020 at 6:43 PM Magnus Hagander wrote: > > I think it would be nicer to have a pg_terminate_backend(pid, wait=false), so > a function with a second parameter which defaults to the current behaviour of > not waiting. And it might be a good idea to also give it a timeout parameter?

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread Bharath Rupireddy
On Thu, Oct 22, 2020 at 8:39 AM David G. Johnston wrote: > >> If the backend is terminated within the user specified timeout then >> the function returns true, otherwise false. > > I’m suggesting an option for the second case to fail instead of returning > false. > That seems fine. > >> > >> >

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread David G. Johnston
On Wednesday, October 21, 2020, Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > Thanks for the feedback. > > On Wed, Oct 21, 2020 at 8:01 PM David G. Johnston > wrote: > > > > On Wed, Oct 21, 2020 at 6:13 AM Magnus Hagander > wrote: > >> > >> I think it would be nicer to hav

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread Bharath Rupireddy
Thanks for the feedback. On Wed, Oct 21, 2020 at 8:01 PM David G. Johnston wrote: > > On Wed, Oct 21, 2020 at 6:13 AM Magnus Hagander wrote: >> >> I think it would be nicer to have a pg_terminate_backend(pid, wait=false), >> so a function with a second parameter which defaults to the current >

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread Andres Freund
On 2020-10-21 15:13:36 +0200, Magnus Hagander wrote: > It seems this one also very much would need a timeout value. I'm not really against that, but I wonder if we just end up reimplementing statement timeout...

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread Bharath Rupireddy
Thanks for the feedback. On Wed, Oct 21, 2020 at 6:43 PM Magnus Hagander wrote: > >> Currently pg_terminate_backend(), sends SIGTERM to the backend process but >> doesn't ensure it's exit. There are chances that backends still are >> running(even after pg_terminate_backend() is called) until th

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread David G. Johnston
On Wed, Oct 21, 2020 at 6:13 AM Magnus Hagander wrote: > On Wed, Oct 21, 2020 at 3:02 PM Bharath Rupireddy < > bharath.rupireddyforpostg...@gmail.com> wrote: > >> Hi, >> >> Currently pg_terminate_backend(), sends SIGTERM to the backend process >> but doesn't ensure it's exit. There are chances th

Re: A new function to wait for the backend exit after termination

2020-10-21 Thread Magnus Hagander
On Wed, Oct 21, 2020 at 3:02 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > Hi, > > Currently pg_terminate_backend(), sends SIGTERM to the backend process but > doesn't ensure it's exit. There are chances that backends still are > running(even after pg_terminate_backend()

A new function to wait for the backend exit after termination

2020-10-21 Thread Bharath Rupireddy
Hi, Currently pg_terminate_backend(), sends SIGTERM to the backend process but doesn't ensure it's exit. There are chances that backends still are running(even after pg_terminate_backend() is called) until the interrupts are processed(using ProcessInterrupts()). This could cause problems especiall