Re: Function to kill backend

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, Magnus Hagander <mha(at)sollentuna(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Function to kill backend
Date: 2004-04-06 20:15:23
Message-ID: 200404062015.i36KFNH07858@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > If there is a problem, maybe we can fix it, or perhap have the kill
> > function use SIGINT, then wait for the query to cancel, then SIGTERM.
>
> Well, if someone could prove that the SIGTERM path is equivalent to
> a transaction-aborting error followed by normal client disconnect,
> I'd feel a lot better about its reliability. We know those two code
> paths work well.
>
> Right now I do not think they are equivalent, but maybe they could be
> made so.

I just did a whitespace-ignore diff of the SIGINT (which is cancel and
we know works), and SIGTERM (which is under study). Here is a diff of
SIGINT vs. SIGTERM functions:

1,2c1,2
< static void
< StatementCancelHandler(SIGNAL_ARGS)
---
> void
> die(SIGNAL_ARGS)
6,10c6,7
< /*
< * Don't joggle the elbow of proc_exit, nor an already-in-progress
< * abort
< */
< if (!proc_exit_inprogress && !InError)
---
> /* Don't joggle the elbow of proc_exit */
> if (!proc_exit_inprogress)
13c10
< QueryCancelPending = true;
---
> ProcDiePending = true;
16,18c13,14
< * If it's safe to interrupt, and we're waiting for a lock,
< * service the interrupt immediately. No point in interrupting if
< * we're waiting for input, however.
---
> * If it's safe to interrupt, and we're waiting for input or a
> * lock, service the interrupt immediately
26,33c22,26
< if (LockWaitCancel())
< {
< DisableNotifyInterrupt();
< InterruptHoldoffCount--;
< ProcessInterrupts();
< }
< else
< InterruptHoldoffCount--;
---
> DisableNotifyInterrupt();
> /* Make sure CheckDeadLock won't run while shutting down... */
> LockWaitCancel();
> InterruptHoldoffCount--;
> ProcessInterrupts();

The big difference seems to be the InError test, and the test in SIGINT
whether LockWaitCancel() actually returns true or false. Also,
DisableNotifyInterrupt() is called before LockWaitCancel().

Also, one sets QueryCancelPending and the other ProcDiePending. Those
are handled by:

void
ProcessInterrupts(void)
{
/* OK to accept interrupt now? */
if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
return;
InterruptPending = false;
if (ProcDiePending)
{
ProcDiePending = false;
QueryCancelPending = false; /* ProcDie trumps QueryCancel */
ImmediateInterruptOK = false; /* not idle anymore */
DisableNotifyInterrupt();
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
errmsg("terminating connection due to administrator command")));
}
if (QueryCancelPending)
{
QueryCancelPending = false;
ImmediateInterruptOK = false; /* not idle anymore */
DisableNotifyInterrupt();
ereport(ERROR,
(errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling query due to user request")));
}
/* If we get here, do nothing (probably, QueryCancelPending was reset) */
}

and the only difference here seems to be the elog(SHUTDOWN) call.

On first glance, I don't see anything dangerous about SIGTERM.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2004-04-06 20:24:36 Re: [HACKERS] logging statement levels
Previous Message Bruce Momjian 2004-04-06 20:03:21 Re: Function to kill backend

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Dunstan 2004-04-06 20:24:36 Re: [HACKERS] logging statement levels
Previous Message Bruce Momjian 2004-04-06 20:01:52 Re: [HACKERS] logging statement levels