Robert Haas <robertmh...@gmail.com> wrote: > Tom Lane <t...@sss.pgh.pa.us> wrote: >> Robert Haas <robertmh...@gmail.com> writes:
>>> I thought the reason why this hasn't been implemented before >>> now is that sending an ErrorResponse to the client will result >>> in a loss of protocol sync. >> >> Hmm ... you are right that this isn't as simple as an >> ereport(ERROR), but I'm not sure it's impossible. We could for >> instance put the backend into skip-till-Sync state so that it >> effectively ignored the next command message. Causing that to >> happen might be impracticably messy, though. > > Another thing we could maybe do is AbortCurrentTransaction() and > send the client a NoticeResponse saying "hey, expect all of your > future commands to fail with complaints about the transaction > being aborted". Wow, until I read through the thread on this patch and the old thread that Robert linked to I had forgotten the attempt by Andres to deal with this three and a half years ago. That patch died because of how complicated it was to do the right thing if the connection was left open. Personally, where I have seen a use for this, treating it as FATAL would be better anyway; but was OK with treating it as an ERROR if that didn't sink the patch. I guess that puts me in the same camp as Josh and Robert. Vik has submitted v1 and v2 of this patch; the only difference between them is that v1 makes a timeout FATAL and v2 makes it an ERROR (with appropriate corresponding changes to code comments, documentation, and message text). It's not hard to show the problems with an ERROR under v2, confirming that the simple approach of just changing the ereport severity is not enough: test=# set idle_in_transaction_timeout = '3s'; SET test=# begin; BEGIN test=# select 1; ERROR: current transaction is aborted due to idle-in-transaction timeout test=# commit; ERROR: current transaction is aborted, commands ignored until end of transaction block test=# commit; ROLLBACK The first thing is that I don't think a delay between the BEGIN and the SELECT should cause a timeout to trigger, but more importantly there should not be two ERROR responses to one SELECT statement. I'm inclined to abandon the ERROR approach as not worth the effort and fragility, and focus on v1 of the patch. If we can't get to consensus on that, I think that this patch should be flagged "Returned with Feedback", noting that any follow-up version requires some way to deal with the issues raised regarding multiple ERROR messages. Abridged for space. hopefully without losing the main points of the authors, so far: Josh Berkus: > My $0.019999999999998: terminating the connection is the > preferred behavior. Tom Lane: > FWIW, I think aborting the transaction is probably better, > especially if the patch is designed to do nothing to > already-aborted transactions. If the client is still there, it > will see the abort as a failure in its next query, which is less > likely to confuse it completely than a connection loss. (I > think, anyway.) Álvaro Herrera: > I think if we want this at all it should abort the xact, not drop > the connection. Andrew Dunstan: > [quotes Tom's argument] > Yes, I had the same thought. David G Johnston: > Default to dropping the connection but give the > usersadministrators the capability to decide for themselves? Robert Haas: > Assuming that the state of play hasn't changed in some way I'm > unaware of since 2010, I think the best argument for FATAL here > is that it's what we can implement. I happen to think it's > better anyway, because the cases I've seen where this would > actually be useful involve badly-written applications that are > not under the same administrative control as the database and > supposedly have built-in connection poolers, except sometimes > they seem to forget about some of the connections they themselves > opened. The DBAs can't make the app developers fix the app; they > just have to try to survive. Aborting the transaction is a step > in the right direction but since the guy at the other end of the > connection is actually or in effect dead, that just leaves you > with an eternally idle connection. Tom Lane (reprise): > I'm not sure whether cancel-transaction behavior is enough better > than cancel-session to warrant extra effort here. Andres Freund: > [quotes Tom's latest (above)] > I don't think idle_in_transaction_timeout is worth this, but if > we could implement it we could finally have recovery conflicts > against idle in txn sessions not use FATAL... Kevin Grittner: > Personally, where I have seen a use for this, treating it as > FATAL would be better anyway A few ideas were put forward on how a much more complicated patch could allow transaction rollback with an ERROR work acceptably, but I think that would probably need to be a new patch in a later CF, so that is omitted here. So, can we agree to use FATAL to terminate the connection? -- Kevin Grittner EDB: 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
