Greg Smith <greg@2ndquadrant.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.
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.
regards, tom lane
