On Thu, Dec 16, 2010 at 11:58 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Greg Smith <g...@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.
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
