Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Excerpts from Andres Freund's message of lun ene 03 12:03:58 -0300 2011: > On Monday, January 03, 2011 03:38:56 PM Alvaro Herrera wrote: > > Is anybody working on this patch? > I am. Wont be finished in the next two days though (breakin last night...) Okay ... I will be moving to a new house this week anyway :-P > PS: Alvarro: commandprompt.com doesn't resolv anymore, so I can't send you > the > email directly... You gotta be kidding --- hmm, oops! Let me find the flamethrower ... (I guess it's a good thing that my suscription to the list uses a different email address) -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Monday, January 03, 2011 03:38:56 PM Alvaro Herrera wrote: > Is anybody working on this patch? I am. Wont be finished in the next two days though (breakin last night...) Andres PS: Alvarro: commandprompt.com doesn't resolv anymore, so I can't send you the email directly... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Alvaro Herrera wrote: > Is anybody working on this patch? I'm not, but I sure hope someone is -- we could *really* use this in the SSI patch. With something providing the equivalent functionality to Andres's previous patch, and about one day's work in the SSI patch, SSI could guarantee that an immediate retry of a transaction rolled back with a serialization failure would not fail again on conflicts with the same transaction(s). This would be a very nice guarantee to be able to make. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Is anybody working on this patch? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund wrote: > I will try to read the thread and make a proposal for a more > carefull implementation - just not today... I think the results > would be interesting... FWIW, the SSI patch that Dan and I are working on can't have a guarantee that it is immediately safe to retry a transaction which rolls back with a serialization failure (without potentially failing again on exactly the same set of transactions) unless there is a capability such as this "Idle in transaction cancellation" patch would provide. Safe immediate retry would be a nice guarantee for SSI to provide. That being the case, we may not be able to generate the final form of the SSI patch until a patch for this issue is applied. Obviously I know that nobody is in a position to make any promises on this, but I just thought I'd let people know that this issue could possibly be on the critical path to a timely release -- at least if that release will include SSI with the safe retry guarantee. (At least when I'm planning for a release, I like to know such things) -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thursday 16 December 2010 21:41:10 Tom Lane wrote: > Robert Haas writes: > > On Thu, Dec 16, 2010 at 3:18 PM, Tom Lane wrote: > >> I guess you misunderstood what I said. �What I meant was that we cannot > >> longjmp *out to the outer level*, ie we cannot take control away from > >> the input stack. �We could however have a TRY block inside the interrupt > >> handler that catches and handles (queues) any errors occurring during > >> transaction abort. �As long as we eventually return control to openssl > >> I think it should work. > > > > Is there any real advantage to that? > > Not crashing when something funny happens seems like a real advantage to > me. (And an unexpected elog(FATAL) will look like a crash to most > users, even if you want to try to define it as not a crash.) > > > How often do we hit an error > > trying to abort a transaction? And how will we report the error > > anyway? > > Queue it up and report it at the next opportunity, as per upthread. > > > I thought the next thing we'd report would be the recovery > > conflict, not any bizarre can't-abort-the-transaction scenario. > > Well, if we discard it because we're too lazy to implement error message > merging, that's OK. Presumably it'll still get into the postmaster log. > > >> (Hm, but I wonder whether there are any hard > >> timing constraints in the ssl protocol ... although hopefully xact abort > >> won't ever take long enough that that's a real problem.) > > > > That would be incredibly broken. > > Think "authentication timeout". I wouldn't be a bit surprised if the > remote end would drop the connection if certain events didn't come back > reasonably promptly. There might even be security reasons for that, > ie, somebody could brute-force a key if you give them long enough. > (But this is all speculation; I don't actually know SSL innards.) I will try to read the thread and make a proposal for a more carefull implementation - just not today... I think the results would be interesting... Thanks for the input, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Excerpts from Tom Lane's message of jue dic 16 17:54:51 -0300 2010: > Robert Haas writes: > > On Thu, Dec 16, 2010 at 3:41 PM, Tom Lane wrote: > >> (But this is all speculation; I don't actually know SSL innards.) > > > I would be really surprised if aborting a transaction takes long > > enough to mess up SSL. I mean, there could be a network delay at any > > time, too. > > Yeah, that's what I think too. I was just wondering if there could be > any situation where xact abort takes minutes. Maybe if it's transaction commit there could be a long queue of pending deferred triggers, but on abort, those things are discarded. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Robert Haas writes: > On Thu, Dec 16, 2010 at 3:41 PM, Tom Lane wrote: >> (But this is all speculation; I don't actually know SSL innards.) > I would be really surprised if aborting a transaction takes long > enough to mess up SSL. I mean, there could be a network delay at any > time, too. Yeah, that's what I think too. I was just wondering if there could be any situation where xact abort takes minutes. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thu, Dec 16, 2010 at 3:41 PM, Tom Lane wrote: >> I thought the next thing we'd report would be the recovery >> conflict, not any bizarre can't-abort-the-transaction scenario. > > Well, if we discard it because we're too lazy to implement error message > merging, that's OK. Presumably it'll still get into the postmaster log. OK, that's reasonable. >>> (Hm, but I wonder whether there are any hard >>> timing constraints in the ssl protocol ... although hopefully xact abort >>> won't ever take long enough that that's a real problem.) > >> That would be incredibly broken. > > Think "authentication timeout". I wouldn't be a bit surprised if the > remote end would drop the connection if certain events didn't come back > reasonably promptly. There might even be security reasons for that, > ie, somebody could brute-force a key if you give them long enough. > (But this is all speculation; I don't actually know SSL innards.) I would be really surprised if aborting a transaction takes long enough to mess up SSL. I mean, there could be a network delay at any time, too. -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Robert Haas writes: > On Thu, Dec 16, 2010 at 3:18 PM, Tom Lane wrote: >> I guess you misunderstood what I said. What I meant was that we cannot >> longjmp *out to the outer level*, ie we cannot take control away from >> the input stack. We could however have a TRY block inside the interrupt >> handler that catches and handles (queues) any errors occurring during >> transaction abort. As long as we eventually return control to openssl >> I think it should work. > Is there any real advantage to that? Not crashing when something funny happens seems like a real advantage to me. (And an unexpected elog(FATAL) will look like a crash to most users, even if you want to try to define it as not a crash.) > How often do we hit an error > trying to abort a transaction? And how will we report the error > anyway? Queue it up and report it at the next opportunity, as per upthread. > I thought the next thing we'd report would be the recovery > conflict, not any bizarre can't-abort-the-transaction scenario. Well, if we discard it because we're too lazy to implement error message merging, that's OK. Presumably it'll still get into the postmaster log. >> (Hm, but I wonder whether there are any hard >> timing constraints in the ssl protocol ... although hopefully xact abort >> won't ever take long enough that that's a real problem.) > That would be incredibly broken. Think "authentication timeout". I wouldn't be a bit surprised if the remote end would drop the connection if certain events didn't come back reasonably promptly. There might even be security reasons for that, ie, somebody could brute-force a key if you give them long enough. (But this is all speculation; I don't actually know SSL innards.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thu, Dec 16, 2010 at 3:18 PM, Tom Lane wrote: > Robert Haas writes: >> 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). > > Um, if that's the ground rules then we have no advance over the current > situation. > > I guess you misunderstood what I said. What I meant was that we cannot > longjmp *out to the outer level*, ie we cannot take control away from > the input stack. We could however have a TRY block inside the interrupt > handler that catches and handles (queues) any errors occurring during > transaction abort. As long as we eventually return control to openssl > I think it should work. Is there any real advantage to that? How often do we hit an error trying to abort a transaction? And how will we report the error anyway? I thought the next thing we'd report would be the recovery conflict, not any bizarre can't-abort-the-transaction scenario. > (Hm, but I wonder whether there are any hard > timing constraints in the ssl protocol ... although hopefully xact abort > won't ever take long enough that that's a real problem.) That would be incredibly broken. -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Robert Haas writes: > 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). Um, if that's the ground rules then we have no advance over the current situation. I guess you misunderstood what I said. What I meant was that we cannot longjmp *out to the outer level*, ie we cannot take control away from the input stack. We could however have a TRY block inside the interrupt handler that catches and handles (queues) any errors occurring during transaction abort. As long as we eventually return control to openssl I think it should work. (Hm, but I wonder whether there are any hard timing constraints in the ssl protocol ... although hopefully xact abort won't ever take long enough that that's a real problem.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thu, Dec 16, 2010 at 1:24 PM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Dec 16, 2010 at 12:46 PM, Tom Lane 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Robert Haas writes: > On Thu, Dec 16, 2010 at 12:46 PM, Tom Lane 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). >> 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. >> 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. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thu, Dec 16, 2010 at 12:46 PM, Tom Lane wrote: > I'm handwaving there --- I think probably the > first cut should just discard errors after the first, and see how > well that works in practice. Seems reasonable. >> 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? >> 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? > > 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. > 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()? -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Robert Haas writes: > On Thu, Dec 16, 2010 at 11:58 AM, Tom Lane wrote: >> 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. > 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. Yeah, it's a bit messy, because you really can't send multiple ERROR messages to the client when it next sends a query: the protocol says there'll be at most one. We could either discard all but the first (or last?) of the queued error events, or add code to elog.c that somehow merges them into one error event, perhaps by adding info to the DETAIL field. I'm handwaving there --- I think probably the first cut should just discard errors after the first, and see how well that works in practice. > 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. In the current FATAL cases, we're cheating a bit by sending something while idle --- what will typically happen at the client end is that it will not notice the input until it sends a query, and then it will see the already-pending input, which it will think is a (remarkably fast) response to its query. Or possibly it will bomb out on send() failure and not ever consume the input at all. But if we want to keep the connection going, we can't cheat like that. > 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? You can only send one, and in that situation you probably want the cancellation to be reported. 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. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thu, Dec 16, 2010 at 11:58 AM, Tom Lane wrote: > Greg Smith 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Greg Smith 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wednesday 15 December 2010 20:12:45 Robert Haas wrote: > On Wed, Dec 15, 2010 at 10:02 AM, Andres Freund wrote: > >> Is there a way that errstart() and/or errfinish() can know enough > >> about the state of the communication with the frontend to decide > >> whether to suppress edata->output_to_client? In other words, instead > >> of explicitly passing in a flag that says whether to inform the > >> client, it would be better for the error-reporting machinery to > >> intrinsically know whether it's right to send_message_to_frontend(). > >> Otherwise, an error thrown from an unexpected location might not have > >> the flag set correctly. > > > > You could use "DoingCommandRead" to solve that specific use-case, but the > > COMERROR ones I don't see as being replaced that easily. > > Well, again, I'm not an expert on this, but why would we need to unify > the two mechanisms? Asynchronous rollbacks (what we're trying to do > here) and protocol violations (which is what COMMERROR looks to be > used for) are really sort of different. I think its not only protocol violations. Data-Leakage during auth are also handled with it I think. > I'm not really sure we need to handle them in the same way. Let's think > about a recovery conflict where ProcessInterrupts() has been called. Right > now, if that situation occurs and we are not DoingCommandRead, then we just > throw an error. That's either safe, or an already-existing bug. That should be safe. > So the question is what to do if we ARE DoingCommandRead. Right now, we > throw a fatal error. There's no comment explaining why, but I'm > guessing that the reason is the same problem we're trying to fix here: > the protocol state gets confused - but if we throw a FATAL then the > client goes away and we don't have to worry about it any more. Our > goal here, as I understand it, is to handle that case without a FATAL. Yes, thats it. > So let's see... if we're DoingCommandRead at that point, and > whereToSendOutput == DestRemote then we set whereToSendOutput = > DestNone before throwing the error, and restore it just after we reset > DoingCommandRead? That won't be enough unfortunately. A transaction-aborted error will get re- raised before the client is ready to re-accept it. Thats what the silent_error_while_idle flag is needed for. Also I think such a simple implementation would cause problems with single user mode. Now that could get fixed by saving and resetting the old mode - that might have exception handling problems in turn (COMERRORS during an ssl connection for example) because it will leave the section without having reset whereToSendOutput. Sprinkling PG_TRY everywhere hardly seems simpler... Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Hi Greg, On Thursday 16 December 2010 13:32:46 Greg Smith wrote: > Andres Freund wrote: > > On Thursday 02 December 2010 22:21:37 Alvaro Herrera wrote: > >> Excerpts from Andres Freund's message of sáb oct 30 05:49:21 -0300 2010: > >>> Ill set this up for the next commitfest, I don't think I can do much > >>> more without further input. > >> > >> Are you reserving about 20 bits for levels, and 12 for flags? Given the > >> relatively scarce growth of levels, I think we could live with about 6 > >> or 7 bits for level, rest for flags. > > > > The number I picked was absolutely arbitrary I admit. Neither did I think > > it would be likely to see more levels, nor did I forsee many flags, so I > > just chose some number I liked in that certain moment ;-) > This bit of detail seems to have died down without being resolved; > bumping it to add a reminder about that. > I count four issues of various sizes left with this patch right now: > > 1) This levels bit If we go with that approach I think the amount of bits reserved for both is big enough to never be reached in reality. Also there is no backward-compat issue in changing as were not ABI stable between major releases anyway... > 2) Can the approach used be simplified or the code made cleaner? > 3) What is the interaction with Hot Standby error handling? It works for me - not to say that independent testing wouldn't be good though. > 4) The usual code formatting nitpicking, Kevin mentioned braces being an > issue Will redo if the other issues are cleared. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund wrote: On Thursday 02 December 2010 22:21:37 Alvaro Herrera wrote: Excerpts from Andres Freund's message of sáb oct 30 05:49:21 -0300 2010: Ill set this up for the next commitfest, I don't think I can do much more without further input. Are you reserving about 20 bits for levels, and 12 for flags? Given the relatively scarce growth of levels, I think we could live with about 6 or 7 bits for level, rest for flags. The number I picked was absolutely arbitrary I admit. Neither did I think it would be likely to see more levels, nor did I forsee many flags, so I just chose some number I liked in that certain moment ;-) This bit of detail seems to have died down without being resolved; bumping it to add a reminder about that. 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 Robert is already thinking about (2); I'll keep an eye out for someone who can test (3) now that it's been identified as a concern; and the other two are pretty small details once those are crossed. I don't see this as being ready to commit just yet though, so I don't see this going anywhere other than returned for now; will mark it as such. Hopefully this will gather enough additional review to continue moving forward now that the main issues are identified. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services and Supportwww.2ndQuadrant.us "PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wed, Dec 15, 2010 at 10:02 AM, Andres Freund wrote: >> Is there a way that errstart() and/or errfinish() can know enough >> about the state of the communication with the frontend to decide >> whether to suppress edata->output_to_client? In other words, instead >> of explicitly passing in a flag that says whether to inform the >> client, it would be better for the error-reporting machinery to >> intrinsically know whether it's right to send_message_to_frontend(). >> Otherwise, an error thrown from an unexpected location might not have >> the flag set correctly. > > You could use "DoingCommandRead" to solve that specific use-case, but the > COMERROR ones I don't see as being replaced that easily. Well, again, I'm not an expert on this, but why would we need to unify the two mechanisms? Asynchronous rollbacks (what we're trying to do here) and protocol violations (which is what COMMERROR looks to be used for) are really sort of different. I'm not really sure we need to handle them in the same way. Let's think about a recovery conflict where ProcessInterrupts() has been called. Right now, if that situation occurs and we are not DoingCommandRead, then we just throw an error. That's either safe, or an already-existing bug. So the question is what to do if we ARE DoingCommandRead. Right now, we throw a fatal error. There's no comment explaining why, but I'm guessing that the reason is the same problem we're trying to fix here: the protocol state gets confused - but if we throw a FATAL then the client goes away and we don't have to worry about it any more. Our goal here, as I understand it, is to handle that case without a FATAL. So let's see... if we're DoingCommandRead at that point, and whereToSendOutput == DestRemote then we set whereToSendOutput = DestNone before throwing the error, and restore it just after we reset DoingCommandRead? -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wednesday 15 December 2010 15:40:20 Robert Haas wrote: > On Wed, Dec 15, 2010 at 7:47 AM, Andres Freund wrote: > > I thought about doing that first. Btw, LOG_NO_CLIENT is just a more > > abstracted way of what COMERROR did before... > > Hmm, but it must not be quite the same, because that didn't require > the silent_error_while_idle flag. True. Thats a separate thing. > >> Yeah. I'll try to find some time to think about this some more. It > >> would sure be nice if we could find a solution that's a bit > >> conceptually cleaner, even if it basically works the same way as what > >> you've done here. > > > > I would like that as well. I am not sure you can achieve that in a > > reasonable amount of work. At least I couldn't. > Is there a way that errstart() and/or errfinish() can know enough > about the state of the communication with the frontend to decide > whether to suppress edata->output_to_client? In other words, instead > of explicitly passing in a flag that says whether to inform the > client, it would be better for the error-reporting machinery to > intrinsically know whether it's right to send_message_to_frontend(). > Otherwise, an error thrown from an unexpected location might not have > the flag set correctly. Currently there are no other locations where we errors could get thrown at that point but I see where youre going. You could use "DoingCommandRead" to solve that specific use-case, but the COMERROR ones I don't see as being replaced that easily. We could introduce something like NoLogToClientBegin(); NoLogToClientEnd(); int NoLogToClientCntr = 0; but that sounds like overdoing it for me. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wed, Dec 15, 2010 at 7:47 AM, Andres Freund wrote: > I thought about doing that first. Btw, LOG_NO_CLIENT is just a more abstracted > way of what COMERROR did before... Hmm, but it must not be quite the same, because that didn't require the silent_error_while_idle flag. >> Yeah. I'll try to find some time to think about this some more. It >> would sure be nice if we could find a solution that's a bit >> conceptually cleaner, even if it basically works the same way as what >> you've done here. > I would like that as well. I am not sure you can achieve that in a reasonable > amount of work. At least I couldn't. Is there a way that errstart() and/or errfinish() can know enough about the state of the communication with the frontend to decide whether to suppress edata->output_to_client? In other words, instead of explicitly passing in a flag that says whether to inform the client, it would be better for the error-reporting machinery to intrinsically know whether it's right to send_message_to_frontend(). Otherwise, an error thrown from an unexpected location might not have the flag set correctly. -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wednesday 15 December 2010 13:33:30 Robert Haas wrote: > On Wed, Dec 15, 2010 at 7:13 AM, Andres Freund wrote: > >> It sort of looks to me like the LOG_NO_CLIENT error flag and the > >> silent_error_while_idle flag are trying to cooperate to get the effect > >> of throwing an error without actually throwing an error. I'm > >> wondering if it would be at all sensible to do that more directly by > >> making ProcessInterrupts() call AbortCurrentTransaction() in this > >> case. > > > > Hm. I think you want the normal server-side error logging continuing to > > work. > > I was thinking we could get around that by doing elog(LOG), but I > guess that doesn't quite work either since we don't know what > client_min_messages is. Hrm... I thought about doing that first. Btw, LOG_NO_CLIENT is just a more abstracted way of what COMERROR did before... > >> I'm not sure if this would work, or if it's better. I'm just throwing > >> it out there, because the current approach looks a little grotty to > >> me. > > > > I with you on the grotty aspect... On the other hand the whole code is > > not exactly nice... > > Yeah. I'll try to find some time to think about this some more. It > would sure be nice if we could find a solution that's a bit > conceptually cleaner, even if it basically works the same way as what > you've done here. I would like that as well. I am not sure you can achieve that in a reasonable amount of work. At least I couldn't. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wed, Dec 15, 2010 at 7:13 AM, Andres Freund wrote: >> It sort of looks to me like the LOG_NO_CLIENT error flag and the >> silent_error_while_idle flag are trying to cooperate to get the effect >> of throwing an error without actually throwing an error. I'm >> wondering if it would be at all sensible to do that more directly by >> making ProcessInterrupts() call AbortCurrentTransaction() in this >> case. > Hm. I think you want the normal server-side error logging continuing to work. I was thinking we could get around that by doing elog(LOG), but I guess that doesn't quite work either since we don't know what client_min_messages is. Hrm... >> I'm not sure if this would work, or if it's better. I'm just throwing >> it out there, because the current approach looks a little grotty to >> me. > I with you on the grotty aspect... On the other hand the whole code is not > exactly nice... Yeah. I'll try to find some time to think about this some more. It would sure be nice if we could find a solution that's a bit conceptually cleaner, even if it basically works the same way as what you've done here. -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Wednesday 15 December 2010 02:20:31 Robert Haas wrote: > On Sat, Oct 30, 2010 at 4:49 AM, Andres Freund wrote: > >> > Here is a proposed patch which enables cancellation of $subject. > > Disclaimer: This isn't my area of expertise, so take the below with a > grain or seven of salt. I don't know whos area of expertise it is except maybe, surprise, surprise, Toms. > It sort of looks to me like the LOG_NO_CLIENT error flag and the > silent_error_while_idle flag are trying to cooperate to get the effect > of throwing an error without actually throwing an error. I'm > wondering if it would be at all sensible to do that more directly by > making ProcessInterrupts() call AbortCurrentTransaction() in this > case. Hm. I think you want the normal server-side error logging continuing to work. Its not really throwing an error without throwing one - its throwing one without confusing the heck out of the client because the protocol is not ready for that. I don't think introducing an "half-error" state is a good idea because one day the protocol maybe ready to actually transport an error while idle in txn (I would like to get there). > I'm not sure if this would work, or if it's better. I'm just throwing > it out there, because the current approach looks a little grotty to > me. I with you on the grotty aspect... On the other hand the whole code is not exactly nice... Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Sat, Oct 30, 2010 at 4:49 AM, Andres Freund wrote: >> > Here is a proposed patch which enables cancellation of $subject. Disclaimer: This isn't my area of expertise, so take the below with a grain or seven of salt. It sort of looks to me like the LOG_NO_CLIENT error flag and the silent_error_while_idle flag are trying to cooperate to get the effect of throwing an error without actually throwing an error. I'm wondering if it would be at all sensible to do that more directly by making ProcessInterrupts() call AbortCurrentTransaction() in this case. Assuming that works at all, it would presumably mean that the client would thereafter get something like this: current transaction is aborted, commands ignored until end of transaction block ...which might be thought unhelpful. But that could be fixed either by modifying errdetail_abort() or perhaps even by abstracting the "current transaction is aborted, commands..." message into a function that could produce an entirely different message if on either the first or all calls within a given transaction. I'm not sure if this would work, or if it's better. I'm just throwing it out there, because the current approach looks a little grotty to me. -- 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
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Thursday 02 December 2010 22:21:37 Alvaro Herrera wrote: > Excerpts from Andres Freund's message of sáb oct 30 05:49:21 -0300 2010: > > Ill set this up for the next commitfest, I don't think I can do much > > more without further input. > > Are you reserving about 20 bits for levels, and 12 for flags? Given the > relatively scarce growth of levels, I think we could live with about 6 > or 7 bits for level, rest for flags. The number I picked was absolutely arbitrary I admit. Neither did I think it would be likely to see more levels, nor did I forsee many flags, so I just chose some number I liked in that certain moment ;-) Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Excerpts from Andres Freund's message of sáb oct 30 05:49:21 -0300 2010: > Ill set this up for the next commitfest, I don't think I can do much more > without further input. Are you reserving about 20 bits for levels, and 12 for flags? Given the relatively scarce growth of levels, I think we could live with about 6 or 7 bits for level, rest for flags. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
I wrote: > I applied all three patches with minor offsets, and it builds, but > several regression tests fail. Sorry, after sending that I realized I hadn't done a make distclean. After that it passes. Please ignore the previous post. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund wrote: > Ok, I implemented that capability I applied all three patches with minor offsets, and it builds, but several regression tests fail. I backed out the patches in reverse order and confirmed that while the regression tests pass without any of these patches, they fail with just the first, the first and the second, or all three patches. If you're not seeing the same thing there, I'll be happy to provide the details. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund writes: > On Tuesday 02 November 2010 22:59:15 Kevin Grittner wrote: >>> Does anybody have any idea why COMMIT is allowed there? Seems >>> pretty strange to me. >> So that the "failed transaction" state can be cleared. The >> transaction as a whole has failed, but you don't want the connection >> to become useless. > Well, allowing ROLLBACK is enought there, isnt it? The client has no reason to think the transaction has failed, so what it's going to send is COMMIT, not ROLLBACK. From the point of view of the client, this should look like its COMMIT failed (or in general its next command failed); not like there was some magic action-at-a-distance on the state of its transaction. Now you could argue that if you send COMMIT, and that fails, you should have to send ROLLBACK to get to an idle state ... but that's not how this ever worked before, and I don't think it's what the SQL standard expects either. After a COMMIT, you're out of the transaction either way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Tuesday 02 November 2010 22:59:15 Kevin Grittner wrote: > > Does anybody have any idea why COMMIT is allowed there? Seems > > pretty strange to me. > > > So that the "failed transaction" state can be cleared. The > transaction as a whole has failed, but you don't want the connection > to become useless. Well, allowing ROLLBACK is enought there, isnt it? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund wrote: > On Tuesday 02 November 2010 18:33:15 Kevin Grittner wrote: >> Well, on a ROLLBACK I'm not sure it's a problem. On a COMMIT, >> couldn't you call a function to check for it in CommitTransaction >> and PrepareTransaction? > Sure, throwing an error somewhere wouldnt be that hard. But at the > moment a COMMIT is always successfull (and just reporting a > ROLLBACK in a failed txn). > Doesn't seem to be something to changed lightly. Well, I'm looking at using this with the Serializable Snapshot Isolation (SSI) patch, which can throw an error from a COMMIT, if the commit completes the conditions necessary for an anomaly to occur (i.e., the committing transaction is on the rw-conflict *out* side of a pivot, and it is the first in the set to commit). If we succeed in enhancing SSI to use lists of rw-conflicted transactions rather than its current techniques, we might be able to (and find it desirable to) always commit in that circumstance and roll back some *other* transaction which is part of the problem. Of course, that other transaction might be idle at the time, and the next thing *it* tries to execute *might* be a COMMIT. So if the SSI patch goes in, there will always be some chance that a COMMIT can fail, but doing it through the mechanism your patch provides could improve performance, because we could guarantee that nobody ever has a serialization failure without first successfully committing a transaction which wrote to one or more tables. If a commit fails due to SSI, it is highly desirable that the error use the serialization failure SQLSTATE, so that an application framework can know that it is reasonable to retry the transaction. > Does anybody have any idea why COMMIT is allowed there? Seems > pretty strange to me. So that the "failed transaction" state can be cleared. The transaction as a whole has failed, but you don't want the connection to become useless. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Excerpts from Andres Freund's message of mar nov 02 18:36:19 -0300 2010: > On Tuesday 02 November 2010 18:33:15 Kevin Grittner wrote: > > Andres Freund wrote: > > > * You wont see an error if the next command after the IDLE in > > > transaction is a COMMIT/ROLLBACK. I don*t see any sensible way > > > around that. > > Well, on a ROLLBACK I'm not sure it's a problem. On a COMMIT, > > couldn't you call a function to check for it in CommitTransaction > > and PrepareTransaction? > Sure, throwing an error somewhere wouldnt be that hard. But at the moment a > COMMIT is always successfull (and just reporting a ROLLBACK in a failed txn). > Doesn't seem to be something to changed lightly. If the user calls COMMIT, it calls EndTransactionBlock, which ends up calling AbortTransaction. Can't you do it at that point? (Says he who hasn't looked at the patch at all) -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
On Tuesday 02 November 2010 18:33:15 Kevin Grittner wrote: > Andres Freund wrote: > > * You wont see an error if the next command after the IDLE in > > transaction is a COMMIT/ROLLBACK. I don*t see any sensible way > > around that. > Well, on a ROLLBACK I'm not sure it's a problem. On a COMMIT, > couldn't you call a function to check for it in CommitTransaction > and PrepareTransaction? Sure, throwing an error somewhere wouldnt be that hard. But at the moment a COMMIT is always successfull (and just reporting a ROLLBACK in a failed txn). Doesn't seem to be something to changed lightly. Does anybody have any idea why COMMIT is allowed there? Seems pretty strange to me. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Andres Freund wrote: > * You wont see an error if the next command after the IDLE in > transaction is a COMMIT/ROLLBACK. I don*t see any sensible way > around that. Well, on a ROLLBACK I'm not sure it's a problem. On a COMMIT, couldn't you call a function to check for it in CommitTransaction and PrepareTransaction? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] V3: Idle in transaction cancellation
Hi, On Tuesday 19 October 2010 16:18:29 Kevin Grittner wrote: > Andres Freund wrote: > > Here is a proposed patch which enables cancellation of $subject. > > Cool. Some enhancements we'd like to do to Serializable Snapshot > Isolation (SSI), should the base patch make it in, would require > this capability. > > > Currently it does *not* report any special error message to the > > client if it > > > > starts sending commands in an (unbekownst to it) failed > > transaction, but just the normal "25P02: current transaction is > > aborted..." message. > > > > It shouldn't be hard to add that and I will propose a patch if > > people would like it (I personally am not very interested, but I > > can see people validly wanting it) > > For SSI purposes, it would be highly desirable to be able to set the > SQLSTATE and message generated when the canceled transaction > terminates. Ok, I implemented that capability, but the patch feels somewhat wrong to me, so its a separate patch on top the others: * it intermingles logic between elog.c and postgres.c far too much - requiring exporting variables which should not get exported. And it would still need more to allow some sensible Assert()s. I.e. Assert(DoingCommandRead) would need to be available in elog.c to avoid somebody using the re-throwing capability out of place * You wont see an error if the next command after the IDLE in transaction is a COMMIT/ROLLBACK. I don’t see any sensible way around that. * the copied edata lives in TopMemoryContext and gets only reset in the next reported error, after the re-raised error was reported. That’s how it looks like to raise one such error right now: error = ERROR if (DoingCommandRead) { silent_error_while_idle = true; error |= LOG_NO_CLIENT|LOG_RE_THROW_AFTER_SYNC; } ... ereport(error, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling statement due to user request"))); One could mingle together LOG_NO_CLIENT|LOG_RE_THROW_AFTER_SYNC into a macro and set silent_error_while_idle in elog.c, but I don’t see many callsites coming up, so I think its better to be explicit. Ill set this up for the next commitfest, I don't think I can do much more without further input. Andres From 06541b25fc11a8f17ec401de5a17eeae1bad57d1 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 21 May 2010 20:17:11 +0200 Subject: [PATCH 2/3] Implement cancellation of backends in IDLE IN TRANSACTION state. Not having support for this was the reason for HS FATALing backends which had a lock conflict for longer than max_standby_(archive|standby)_delay as it couldnt cancel them without them loosing sync with the frontend. As this is not the case anymore fail the transaction "silently". Possibly one day the protocol can cope with this... --- src/backend/tcop/postgres.c | 80 +++ 1 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index cba90a9..505d136 100644 *** a/src/backend/tcop/postgres.c --- b/src/backend/tcop/postgres.c *** static bool RecoveryConflictPending = fa *** 177,182 --- 177,188 static bool RecoveryConflictRetryable = true; static ProcSignalReason RecoveryConflictReason; + /* + * Are we disallowed from sending a "ready for query" message right + * now because it would confuse the frontend? + */ + static bool silent_error_while_idle = false; + /* * decls for routines only used in this file * *** RecoveryConflictInterrupt(ProcSignalReas *** 2877,2882 --- 2883,2890 void ProcessInterrupts(void) { + int error = ERROR; + /* OK to accept interrupt now? */ if (InterruptHoldoffCount != 0 || CritSectionCount != 0) return; *** ProcessInterrupts(void) *** 2949,2966 RecoveryConflictPending = false; DisableNotifyInterrupt(); DisableCatchupInterrupt(); ! if (DoingCommandRead) ! ereport(FATAL, ! (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), ! errmsg("terminating connection due to conflict with recovery"), ! errdetail_recovery_conflict(), ! errhint("In a moment you should be able to reconnect to the" ! " database and repeat your command."))); ! else ! ereport(ERROR, ! (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), ! errmsg("canceling statement due to conflict with recovery"), ! errdetail_recovery_conflict())); } /* --- 2957,2980 RecoveryConflictPending = false; DisableNotifyInterrupt(); DisableCatchupInterrupt(); ! ! if (DoingCommandRead){ ! /* ! * We cant issue a normal ERROR here because the ! * client doesnt expect the server to send an error at ! * that point. ! * We also may not send a "ready for query"/Z message !