Re: [PATCH] V3: Idle in transaction cancellation

On Thu, Dec 16, 2010 at 1:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Thu, Dec 16, 2010 at 12:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Another thing I don't quite understand is - at what point does the
>>>> protocol allow us to emit an error?
>
>>> Basically, you can send an error in response to a query.
>
>> What about some other message that's not a query?
>
> There aren't any (I'm using a loose definition of "query" here --- any
> client request counts).

OK.

>>> You can only send one, and in that situation you probably want the
>>> cancellation to be reported.
>
>> What about an elog or ereport with severity < ERROR?  Surely there
>> must at least be provision for multiple non-error messages per
>> transaction.
>
> You can send NOTICEs freely, but downgrading an error to a notice is
> probably not a great solution --- keep in mind that some clients just
> discard those altogether.

Yeah, I wasn't proposing that, just trying to understand the rules.

>>> FWIW, I'm not too worried about preserving the existing
>>> recovery-conflict behavior, as I think the odds are at least ten to one
>>> that that code is broken when you look closely enough.  I do like the
>>> idea that this patch would provide a better-thought-out framework for
>>> handling the conflict case.
>
>> We already have pg_terminate_backend() and pg_cancel_backend().  Are
>> you imagining a general mechanism like pg_rollback_backend()?
>
> No, not really, I'm just concerned about the fact that it's trying to
> send a message while in DoingCommandRead state.  FE/BE protocol
> considerations aside, that's likely to break if using SSL, because who
> knows where we've interrupted openssl.  In fairness, the various
> pre-existing FATAL-interrupt cases have that problem already, but I was
> willing to live with it for things that don't happen during normal
> operation.

Hmm. It's seeming to me that what we want to do is something like this:

1. If an error is thrown while DoingCommandRead, it gets upgraded to
FATAL. I don't think we have much choice about this because, per your
previous comments, we can't longjmp() here without risking protocol
breakage, and we certainly can't return from an elog(ERROR) or
ereport(ERROR).

2. If a recovery conflict arrives while DoingCommandRead(), we
AbortCurrentTransaction(). If this runs into unexpected trouble,
it'll turn into a FATAL per #1. If it completes successfully, then
we'll set a flag indicating that upon emerging from DoingCommandRead
state, we need to signal the recovery conflict to the client.

3. When we clear DoingCommandRead, we'll check whether the flag is set
and if so ereport(ERROR).

Step #2 seems like the dangerous part, but I'm not immediately sure
what hazards may be lurking there.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company