On Wed, Oct 21, 2020 at 6:13 AM Magnus Hagander <mag...@hagander.net> 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 that backends still are >> running(even after pg_terminate_backend() is called) until the interrupts >> are processed(using ProcessInterrupts()). This could cause problems >> especially in testing, for instance in a sql file right after >> pg_terminate_backend(), if any test case depends on the backend's >> non-existence[1], but the backend is not terminated. As discussed in [1], >> we have wait_pid()(see regress.c and sql/dblink.sql), but it's not usable >> across the system. In [1], we thought it would be better to have functions >> ensuring the backend's exit on the similar lines of pg_terminate_backend(). >> >> I propose to have two functions: >> >> 1. pg_terminate_backend_and_wait() -- which sends SIGTERM to the backend >> and wait's until it's exit. >> > > 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? > Agreed on the overload, and the timeouts make sense too - with the caller deciding whether a timeout results in a failure or a false return value. > >> 2. pg_wait_backend() -- which waits for a given backend process. Note >> that this function has to be used carefully after pg_terminate_backend(), >> if used on a backend that's not ternmited it simply keeps waiting in a loop. >> > > It seems this one also very much would need a timeout value. > > Is there a requirement for waiting to be superuser only? You are not affecting any session but your own during the waiting period. I could imagine, in theory at least, wanting to wait for a backend to go idle as well as for it disappearing. Scope creep in terms of this patch's goal but worth at least considering now. David J.
