Re: [PATCH] V3: Idle in transaction cancellation

On Thu, Dec 16, 2010 at 11:58 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Greg Smith <greg(at)2ndquadrant(dot)com> writes:
>> I count four issues of various sizes left with this patch right now:
>
>> 1) This levels bit
>> 2) Can the approach used be simplified or the code made cleaner?
>> 3) What is the interaction with Hot Standby error handling?
>> 4) The usual code formatting nitpicking, Kevin mentioned braces being an
>> issue
>
> You forgot (5) it doesn't work and (6) it's impossibly ugly :-(.
>
> The reason it doesn't work is you can *not* throw a longjmp while in
> DoingCommandRead state.  This isn't just a matter of not breaking the
> protocol at our level; it risks leaving openssl in a confused state,
> either violating the ssl protocol or simply with internal state trashed
> because it got interrupted in the middle of changing it.

Ah! An excellent point. Thanks for weighing in on this.

> It's possible we could refactor things so we abort the open transaction
> while inside the interrupt handler, then queue up an error to be
> reported whenever we next get a command (as envisioned in part 0003),
> then just return control back to the input stack.  But things could get
> messy if we get another error to be reported while trying to abort.
>
> In any case I really do not care for flag bits in the elog level field.
> That's horrid, and I don't think it's even well matched to the problem.
> What we need is for elog.c to understand that we're in a *state* that
> forbids transmission to the client; it's not a property of specific
> error messages, much less a property that the code reporting the error
> should be required to be aware of.
>
> I think a more realistic approach to the problem might be to expose the
> DoingCommandRead flag to elog.c, and have it take responsibility for
> queuing messages that can't be sent to the client right away.  Plus the
> above-mentioned refactoring of where transaction abort happens.

This is along the lines of what Andres and I have already been groping
towards upthread. But the question of what to do if another error is
encountered while trying to abort the transaction is one that I also
thought about, and I don't see an easy solution. I suppose we could
upgrade such an error to FATAL, given that right now we throw an
*unconditional* FATAL here when DoingCommandRead. That's not
super-appealing, but it might be the most practical solution.

Another thing I don't quite understand is - at what point does the
protocol allow us to emit an error? Suppose that the transaction gets
cancelled due to a conflict with recovery while we're
DoingCommandRead, and then the user now sends us "SELCT 2+2". Are we
going to send them back both errors now, or just one of them? Which
one?

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