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
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 |
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 |