CHECK_FOR_INTERRUPTS in spgdoinsert() isn't helpful

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: CHECK_FOR_INTERRUPTS in spgdoinsert() isn't helpful
Date: 2014-05-29 20:25:52
Message-ID: 16519.1401395152@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

One of the annoying things about the SPGiST bug Teodor pointed out is
that once you're in the infinite loop, query cancel doesn't work to get
you out of it. There are a couple of CHECK_FOR_INTERRUPTS() calls in
spgdoinsert() that are meant to get you out of exactly this sort of
buggy-opclass situation --- but they don't help. The reason is that
when we reach that point we're holding one or more buffer locks, which
means the HOLD_INTERRUPTS count is positive, so ProcessInterrupts is
afraid to do anything.

In point of fact, we'd be happy to give up the locks and then throw
the error. So I was thinking about inventing some macro or out-of-line
function that went about like this:

if (InterruptPending && (QueryCancelPending || ProcDiePending))
{
LWLockReleaseAll();
ProcessInterrupts();
elog(ERROR, "ProcessInterrupts failed to throw an error");
}

where we don't expect to reach the final elog; it's just there to make
sure we don't return control after yanking locks out from under the
caller.

You might object that this is risky since it might release locks other
than the buffer locks spgdoinsert() is specifically concerned with.
However, if spgdoinsert() were to throw an error for a reason other than
query cancel, any such locks would get released anyway as the first step
during error cleanup, so I think this is not a real concern.

There may be other places in the index AMs or other low-level code where
this'd be appropriate; I've not really looked.

Thoughts?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2014-05-29 20:31:22 Re: Re: Logical slots not mentioned in CREATE_REPLICATION_SLOT for replication protocol docs
Previous Message Heikki Linnakangas 2014-05-29 20:23:19 Re: Extended Prefetching using Asynchronous IO - proposal and patch