Re: [HACKERS] Cancelling idle in transaction state
On Sun, 2010-01-03 at 11:55 +0100, Martijn van Oosterhout wrote: On Fri, Jan 01, 2010 at 03:31:58PM -0500, Kris Jurka wrote: The JDBC driver does want cancel if active behavior. The JDBC API specifies Statement.cancel() where Statement is running one particular backend query. So it really does want to cancel just that one query. Already this is tough because of the asynchronous nature of the cancel protocol and the inability to say exactly what should be cancelled. I've looked in the JDBC documentation but I don't quickly see how they expect this to work with transactions. What is being proposed seems to me to be: If statement active: put transaction in aborted state If no statement active: do nothing However, I see that the documentation wants to be able to abort a *specific* statement, which is not being proposed here. Can that be implemented on top of the current proposal? That would require Statement-level abort, which we don't have. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs si...@2ndquadrant.com wrote: CancelRequest's behaviour currently equates to SIGINT, so processCancelRequest() can only use SIGINT if SIGINT's behaviour remains same. I would recommend we make SIGINT do cancel-anything, and handle everything else via SIGUSR1, including CancelRequest. Actually, now that I look into it, if we wanted to send SIGUSR1 with a reason to a backend from within postmaster (where processCancelRequest() lives), we'd need to have shared memory access in postmaster which we have not. So the easiest way would be to keep SIGINTs behavior (cancel running statements, not idle transactions) and allow cancellation of idle transactions only via SQL but not via command line. Other ideas? Joachim -- 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] Cancelling idle in transaction state
On Wed, Jan 6, 2010 at 4:37 PM, Joachim Wieland j...@mcknight.de wrote: On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs si...@2ndquadrant.com wrote: CancelRequest's behaviour currently equates to SIGINT, so processCancelRequest() can only use SIGINT if SIGINT's behaviour remains same. I would recommend we make SIGINT do cancel-anything, and handle everything else via SIGUSR1, including CancelRequest. Actually, now that I look into it, if we wanted to send SIGUSR1 with a reason to a backend from within postmaster (where processCancelRequest() lives), we'd need to have shared memory access in postmaster which we have not. So the easiest way would be to keep SIGINTs behavior (cancel running statements, not idle transactions) and allow cancellation of idle transactions only via SQL but not via command line. +1. That seems like the right approach to me. ...Robert -- 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] Cancelling idle in transaction state
On Fri, Jan 01, 2010 at 03:31:58PM -0500, Kris Jurka wrote: The JDBC driver does want cancel if active behavior. The JDBC API specifies Statement.cancel() where Statement is running one particular backend query. So it really does want to cancel just that one query. Already this is tough because of the asynchronous nature of the cancel protocol and the inability to say exactly what should be cancelled. I've looked in the JDBC documentation but I don't quickly see how they expect this to work with transactions. What is being proposed seems to me to be: If statement active: put transaction in aborted state If no statement active: do nothing However, I see that the documentation wants to be able to abort a *specific* statement, which is not being proposed here. Can that be implemented on top of the current proposal? Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ Please line up in a tree and maintain the heap invariant while boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] Cancelling idle in transaction state
On Fri, Jan 1, 2010 at 1:39 PM, Simon Riggs si...@2ndquadrant.com wrote: Interesting. It's not obvious to me how killing an *idle* session can resolve a deadlock. AIUI a deadlock requires a cycle in the waits-for graph, and an idle transaction is not waiting for a lock acquisition. In strict theory, yes. In practice, many lock contention situations are caused by long running idle transactions, so having a deadlock detector be able to resolve a situation by deciding that an idle xact is actually in some kind of wait state would be very useful. Some people have asked for a idle-in-transaction-timeout. I would be more inclined to have a settable time after which an idle-in-transaction session that blocks an active lock requestor can be aborted by the deadlock detector as a way of resolving a lock wait. Idle-in-transaction sessions that don't hold any locks aren't the same kind of annoyance, though there may be other reasons to remove them. I think the biggest issue with idle-in-transaction sessions is MVCC bloat, which has been considerably mitigated in 8.4 (shout-out to Alvaro). It could still be an issue for serializable transactions, though. So I'm not 100% sure what is most useful down the road, but it seems you've solved the immediate problem here, which is good. ...Robert -- 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] Cancelling idle in transaction state
Hi, Simon Riggs wrote: In practice, many lock contention situations are caused by long running idle transactions, so having a deadlock detector be able to resolve a situation by deciding that an idle xact is actually in some kind of wait state would be very useful. Hm.. so you'd abort the transaction that's been idle the longest? Is that really the one you want to abort in every case? We currently abort the one which is checking for deadlocks, right? That's a pretty random pick, then. And randomization might have benefits here (namely giving all kinds of transaction, whether interactive or automated, the same chance of surviving a deadlock). I'm not sure whether or not this is a good or required thing, though. Allow me to also point out the related requirement of several replication solutions to gather information about such a deadlock or maybe even control the choice of which transaction to abort. See http://wiki.postgresql.org/wiki/ClusterFeatures#A_standard_way_to_get_global_deadlock_information Some people have asked for a idle-in-transaction-timeout. I would be more inclined to have a settable time after which an idle-in-transaction session that blocks an active lock requestor can be aborted by the deadlock detector as a way of resolving a lock wait. Idle-in-transaction sessions that don't hold any locks aren't the same kind of annoyance, though there may be other reasons to remove them. Aha, yes I see. That sounds more controllable (and should probably default to no timeout). Regards Markus Wanner -- 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] Cancelling idle in transaction state
Simon Riggs wrote: Attached is the patch I intend to commit, barring objections. This patch extends SIGINT to allow cancellation of transactions while idle in both HS and normal mode. How does AbortTransactionAndAnySubtransactions() differ from AbortOutOfAnyTransaction()? Having followed the discussions on this patch I think I know the answer: I'm guessing that AbortTransactionAndAnySubtransactions() leaves the backend in aborted state, while AbortOutOfAnyTransaction() leaves it in idle state. It would be good to point that out more clearly in the comment above AbortTransactionAndAnySubtransactions(). I agree with Joachim's comments upthread that we should have a different function for forcefully aborting the whole transaction, and leave the current pg_cancel_backend() behavior alone. In any case, the documentation of pg_cancel_backend() or the new function needs to explain what exactly it does. I believe people liked the idea of giving a different ERROR message when a transaction is canceled because of conflict with recovery. It would also be good to give a different error message when an idle transaction is canceled using query cancel. Otherwise you don't know what hit you, especially if you're not paying attention to NOTICEs. It also changes the standard message reported on an idle transaction in aborted state to 'IDLE in transaction (aborted)', so that once aborted we keep the message even if the user tries to issue further statements other than ROLLBACK or COMMIT. That's nice. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] Cancelling idle in transaction state
On Fri, 2010-01-01 at 12:53 +0200, Heikki Linnakangas wrote: Simon Riggs wrote: Attached is the patch I intend to commit, barring objections. This patch extends SIGINT to allow cancellation of transactions while idle in both HS and normal mode. How does AbortTransactionAndAnySubtransactions() differ from AbortOutOfAnyTransaction()? Having followed the discussions on this patch I think I know the answer: I'm guessing that AbortTransactionAndAnySubtransactions() leaves the backend in aborted state, while AbortOutOfAnyTransaction() leaves it in idle state. It would be good to point that out more clearly in the comment above AbortTransactionAndAnySubtransactions(). OK I agree with Joachim's comments upthread that we should have a different function for forcefully aborting the whole transaction, and leave the current pg_cancel_backend() behavior alone. That would require multiple behaviours. Joachim already proposed multiplexing SIGINT and Tom disagreed, on the simultaneous thread Hot Standby introduced problem with query cancel behaviour. I agree that there seems little point in multiplexing both signals. So the only way to have multiple cancel behaviours is to do this cancellation via SIGUSR1 options and have additional functions to request them. Which amounts to rejecting this patch, since *this* patch changes the behaviour of SIGINT for all senders, which is what I understood people desired, i.e. not just a change for Hot Standby. I assumed Joachim did not mean to veto his own patch, but I'm not sure what you think here? (I don't mind either way). In any case, the documentation of pg_cancel_backend() or the new function needs to explain what exactly it does. ...The patch changes the docs for pg_cancel_backend(). I believe people liked the idea of giving a different ERROR message when a transaction is canceled because of conflict with recovery. It would also be good to give a different error message when an idle transaction is canceled using query cancel. Otherwise you don't know what hit you, especially if you're not paying attention to NOTICEs. I did like that idea when I heard it, but it seemed to have a few problems on implementation. Currently we say ERROR: current transaction is aborted, commands ignored until end of transaction block and we repeat this every time a new statement is sent other than COMMIT or ROLLBACK. We could either endlessly repeat this ERROR: current transaction is aborted because of conflict with recovery, commands ignored until end of transaction block or just say it once and then revert to the normal message. Neither seemed very useful. I'm also not sure why we would want to single out Hot Standby to generate the reason because of conflict with recovery when no other ERROR source would generate such a reason. It also changes the standard message reported on an idle transaction in aborted state to 'IDLE in transaction (aborted)', so that once aborted we keep the message even if the user tries to issue further statements other than ROLLBACK or COMMIT. That's nice. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w We could either endlessly repeat this ERROR: current transaction is aborted because of conflict with recovery, commands ignored until end of transaction block +1 for this option. I'm also not sure why we would want to single out Hot Standby to generate the reason because of conflict with recovery when no other ERROR source would generate such a reason. Well, most times when the transaction is aborted, it's because you did something wrong. Or at least, the failure is associated with some particular statement. If we have other events that can asynchronously roll back a transaction, I would think they would deserve similar handling. Off the top of my head, I'm not sure if there are any such cases. ...Robert -- 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] Cancelling idle in transaction state
Simon Riggs wrote: On Fri, 2010-01-01 at 12:53 +0200, Heikki Linnakangas wrote: I agree with Joachim's comments upthread that we should have a different function for forcefully aborting the whole transaction, and leave the current pg_cancel_backend() behavior alone. That would require multiple behaviours. Joachim already proposed multiplexing SIGINT and Tom disagreed, on the simultaneous thread Hot Standby introduced problem with query cancel behaviour. I agree that there seems little point in multiplexing both signals. So the only way to have multiple cancel behaviours is to do this cancellation via SIGUSR1 options and have additional functions to request them. Which amounts to rejecting this patch, since *this* patch changes the behaviour of SIGINT for all senders, which is what I understood people desired, i.e. not just a change for Hot Standby. I assumed Joachim did not mean to veto his own patch, but I'm not sure what you think here? (I don't mind either way). I don't know, I don't feel strongly about this. Is there really no other way? In any case, the documentation of pg_cancel_backend() or the new function needs to explain what exactly it does. ...The patch changes the docs for pg_cancel_backend(). I don't think that's enough. Cancel a backend's current query or abort an idle transaction leaves a lot of questions. When does it abort the transaction? The whole top-level transaction or just the current subtransaction? I believe people liked the idea of giving a different ERROR message when a transaction is canceled because of conflict with recovery. It would also be good to give a different error message when an idle transaction is canceled using query cancel. Otherwise you don't know what hit you, especially if you're not paying attention to NOTICEs. I did like that idea when I heard it, but it seemed to have a few problems on implementation. Currently we say ERROR: current transaction is aborted, commands ignored until end of transaction block and we repeat this every time a new statement is sent other than COMMIT or ROLLBACK. We could either endlessly repeat this ERROR: current transaction is aborted because of conflict with recovery, commands ignored until end of transaction block or just say it once and then revert to the normal message. Neither seemed very useful. Seems useful to me, so that you know why your transaction was cancelled. It's rather weird to see no ERRORs in the previous steps, and suddenly you see that the transaction is aborted. And none your savepoints exist anymore either. I'm also not sure why we would want to single out Hot Standby to generate the reason because of conflict with recovery when no other ERROR source would generate such a reason. Because you don't get any other ERROR for it. If your transaction is aborted because of e.g a unique key violation, you get the ERROR about unique key violation first, and only the subsequent commands produce the current transaction is aborted message. With hot standby conflicts, the first and only symptom the client sees is that the subsequent commands fail with current transaction is aborted, so it would be nice to know why. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] Cancelling idle in transaction state
On Fri, Jan 1, 2010 at 8:38 PM, Robert Haas robertmh...@gmail.com wrote:
On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w
We could either endlessly repeat this
ERROR: current transaction is aborted because of conflict with
recovery, commands ignored until end of transaction block
+1 for this option.
I'm also not sure why we would want to single out Hot Standby to
generate the reason because of conflict with recovery when no other
ERROR source would generate such a reason.
Well, most times when the transaction is aborted, it's because you did
something wrong. Or at least, the failure is associated with some
particular statement.
If we have other events that can asynchronously roll back a transaction, I
would think they would deserve similar handling. Off the top of my head,
I'm not sure if there are any such cases.
Why not do the finger pointing (to HS) in the DETAIL field of the ERROR, and
let the error message remain the same.
Best regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.enterprisedb.com
singh.gurj...@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device
Re: [HACKERS] Cancelling idle in transaction state
On Fri, 2010-01-01 at 07:08 -0800, Robert Haas wrote: On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w We could either endlessly repeat this ERROR: current transaction is aborted because of conflict with recovery, commands ignored until end of transaction block +1 for this option. I'm also not sure why we would want to single out Hot Standby to generate the reason because of conflict with recovery when no other ERROR source would generate such a reason. Well, most times when the transaction is aborted, it's because you did something wrong. Or at least, the failure is associated with some particular statement. If we have other events that can asynchronously roll back a transaction, I would think they would deserve similar handling. Off the top of my head, I'm not sure if there are any such cases. Serialization failures, deadlocks, timeouts, SIGINT, out of memory errors etc.. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Jan 1, 2010, at 8:30 AM, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2010-01-01 at 07:08 -0800, Robert Haas wrote: On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w We could either endlessly repeat this ERROR: current transaction is aborted because of conflict with recovery, commands ignored until end of transaction block +1 for this option. I'm also not sure why we would want to single out Hot Standby to generate the reason because of conflict with recovery when no other ERROR source would generate such a reason. Well, most times when the transaction is aborted, it's because you did something wrong. Or at least, the failure is associated with some particular statement. If we have other events that can asynchronously roll back a transaction, I would think they would deserve similar handling. Off the top of my head, I'm not sure if there are any such cases. Serialization failures, deadlocks, timeouts, SIGINT, out of memory errors etc.. Hmm. I don't think I can get a serialization failure, deadlock, or out of memory error while my session is idle. An idle timeout or SIGINT is analagous, I think. ...Robert -- 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] Cancelling idle in transaction state
On Fri, 2010-01-01 at 17:14 +0200, Heikki Linnakangas wrote: Which amounts to rejecting this patch, since *this* patch changes the behaviour of SIGINT for all senders, which is what I understood people desired, i.e. not just a change for Hot Standby. I assumed Joachim did not mean to veto his own patch, but I'm not sure what you think here? (I don't mind either way). I don't know, I don't feel strongly about this. Is there really no other way? Multiple behaviours on signal implies multiplexing, AFAICS. Question on the table is: Should SIGINT be extended to cancel an idle-in-transaction session, or not? I'll wait a little while longer before committing this to make sure I have the full spread of opinion. Tom's opinion was... On Tue, 2009-12-29 at 10:22 -0500, Tom Lane wrote: Joachim Wieland j...@mcknight.de writes: If we use the same signal for both cases, the receiving backend cannot tell what the intention of the sending backend was. That's why I proposed to make SIGINT similar to SIGUSR1 where we write a reason to a shared memory structure first and then send the signal (see http://archives.postgresql.org/pgsql-hackers/2009-12/msg02067.php from a few days ago). This seems like a fairly bad idea. One of the intended use-cases is to be able to manually kill -INT a misbehaving backend. Assuming that there will be valid info about the signal in shared memory will break that. So it seems that we have at least one vote in favour of making SIGINT blow anything away, no matter what its state. I support that also, but I don't need it for HS, its just an objective opinion. So that's plus 2, unsure about Joachim. Any others? Seems useful to me, so that you know why your transaction was cancelled. It's rather weird to see no ERRORs in the previous steps, and suddenly you see that the transaction is aborted. And none your savepoints exist anymore either. I agree we need a message to explain, it just seems wrong to me to do this in a way that appears to accentuate this particular source of error over similar sources. However, I will do as requested, though will leave existing error sources alone. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Fri, 2010-01-01 at 09:24 -0800, Robert Haas wrote: If we have other events that can asynchronously roll back a transaction, I would think they would deserve similar handling. Off the top of my head, I'm not sure if there are any such cases. Serialization failures, deadlocks, timeouts, SIGINT, out of memory errors etc.. Hmm. I don't think I can get a serialization failure, deadlock, or out of memory error while my session is idle. Agreed. As a point of note, now that we can cancel idle transactions there isn't any future blocker from making serialization failures or deadlocks cancel such transactions... Other RDBMS have deadlock detectors that can pick any session to resolve, not just the one doing the deadlock checking. An idle timeout or SIGINT is analagous, I think. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Jan 1, 2010, at 9:42 AM, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2010-01-01 at 09:24 -0800, Robert Haas wrote: If we have other events that can asynchronously roll back a transaction, I would think they would deserve similar handling. Off the top of my head, I'm not sure if there are any such cases. Serialization failures, deadlocks, timeouts, SIGINT, out of memory errors etc.. Hmm. I don't think I can get a serialization failure, deadlock, or out of memory error while my session is idle. Agreed. As a point of note, now that we can cancel idle transactions there isn't any future blocker from making serialization failures or deadlocks cancel such transactions... Other RDBMS have deadlock detectors that can pick any session to resolve, not just the one doing the deadlock checking. Interesting. It's not obvious to me how killing an *idle* session can resolve a deadlock. AIUI a deadlock requires a cycle in the waits-for graph, and an idle transaction is not waiting for a lock acquisition. I can see how it could be useful in handling serialization failures, though, and there may be other applications as well. This is a nice improvement; I'm pleased to see it going in. ...Robert -- 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] Cancelling idle in transaction state
On Fri, 2010-01-01 at 10:15 -0800, Robert Haas wrote: Hmm. I don't think I can get a serialization failure, deadlock, or out of memory error while my session is idle. Agreed. As a point of note, now that we can cancel idle transactions there isn't any future blocker from making serialization failures or deadlocks cancel such transactions... Other RDBMS have deadlock detectors that can pick any session to resolve, not just the one doing the deadlock checking. Interesting. It's not obvious to me how killing an *idle* session can resolve a deadlock. AIUI a deadlock requires a cycle in the waits-for graph, and an idle transaction is not waiting for a lock acquisition. In strict theory, yes. In practice, many lock contention situations are caused by long running idle transactions, so having a deadlock detector be able to resolve a situation by deciding that an idle xact is actually in some kind of wait state would be very useful. Some people have asked for a idle-in-transaction-timeout. I would be more inclined to have a settable time after which an idle-in-transaction session that blocks an active lock requestor can be aborted by the deadlock detector as a way of resolving a lock wait. Idle-in-transaction sessions that don't hold any locks aren't the same kind of annoyance, though there may be other reasons to remove them. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Thu, 31 Dec 2009, Simon Riggs wrote: On Thu, 2009-12-31 at 15:41 +0100, Joachim Wieland wrote: I still think that we should have three transaction cancel modes, one to cancel an idle transaction, another one to cancel a running query and a third one that just cancels the transaction regardless of it being idle or not. This last one is what you are implementing now, and it is what HS wants to do. pg_cancel_backend() is currently conditional on whether a statement is active or not, so should really be called pg_cancel_if_active(). What people want is an unconditional way to stop a transaction. I don't see the need for 3 modes (and that has nothing to do with HS). The JDBC driver does want cancel if active behavior. The JDBC API specifies Statement.cancel() where Statement is running one particular backend query. So it really does want to cancel just that one query. Already this is tough because of the asynchronous nature of the cancel protocol and the inability to say exactly what should be cancelled. Kris Jurka -- 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] Cancelling idle in transaction state
On Fri, 2010-01-01 at 15:31 -0500, Kris Jurka wrote: On Thu, 31 Dec 2009, Simon Riggs wrote: On Thu, 2009-12-31 at 15:41 +0100, Joachim Wieland wrote: I still think that we should have three transaction cancel modes, one to cancel an idle transaction, another one to cancel a running query and a third one that just cancels the transaction regardless of it being idle or not. This last one is what you are implementing now, and it is what HS wants to do. pg_cancel_backend() is currently conditional on whether a statement is active or not, so should really be called pg_cancel_if_active(). What people want is an unconditional way to stop a transaction. I don't see the need for 3 modes (and that has nothing to do with HS). The JDBC driver does want cancel if active behavior. The JDBC API specifies Statement.cancel() where Statement is running one particular backend query. So it really does want to cancel just that one query. Already this is tough because of the asynchronous nature of the cancel protocol and the inability to say exactly what should be cancelled. OK, I think that is conclusive. CancelRequest's behaviour currently equates to SIGINT, so processCancelRequest() can only use SIGINT if SIGINT's behaviour remains same. I would recommend we make SIGINT do cancel-anything, and handle everything else via SIGUSR1, including CancelRequest. I'm not going to do that; I'm going to make HS conflict resolution work, which means putting in enough infrastructure to allow someone else to make SIGINT changes work at a later time, if appropriate. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs si...@2ndquadrant.com wrote: I would recommend we make SIGINT do cancel-anything, and handle everything else via SIGUSR1, including CancelRequest. I'm not going to do that; I'm going to make HS conflict resolution work, which means putting in enough infrastructure to allow someone else to make SIGINT changes work at a later time, if appropriate. If this is the final consent then please go ahead with HS and I will see if I can take care of the rest. Joachim -- 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] Cancelling idle in transaction state
On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote: On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: We are using NOTICE, not NOTIFY, assuming that we use anything at all (which I still regard as unnecessary). Please stop injecting confusion into the discussion. Attached is a minimal POC patch that allows to cancel an idle transaction with SIGINT. The HS patch also allows this in its current form but as Simon points out the client gets out of sync with it. The proposal is to send an additional NOTICE to the client and abort all open transactions and subtransactions (this is what I got from the previous discussion). This all works and I'm looking to post a reviewed patch soon. I had to write an additional function AbortAnyTransaction() which aborts all transactions and subtransactions and leaves the transaction in the aborted state, is there an existing function to do this? My use of AbortOutOfAnyTransaction() was what caused the problem-I-couldn't-solve. It aborted too far, confusing clients. Joachim's function does the right thing and leaves the transaction state correctly, so that clients don't get confused. Problem solved, thanks Joachim. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Thu, 2009-12-31 at 11:10 +, Simon Riggs wrote:
On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote:
On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
We are using NOTICE, not NOTIFY, assuming that we use anything at all
(which I still regard as unnecessary). Please stop injecting confusion
into the discussion.
Attached is a minimal POC patch that allows to cancel an idle
transaction with SIGINT. The HS patch also allows this in its current
form but as Simon points out the client gets out of sync with it.
The proposal is to send an additional NOTICE to the client and abort
all open transactions and subtransactions (this is what I got from the
previous discussion).
This all works and I'm looking to post a reviewed patch soon.
Attached is the patch I intend to commit, barring objections.
This patch extends SIGINT to allow cancellation of transactions while
idle in both HS and normal mode. It also changes the standard message
reported on an idle transaction in aborted state to 'IDLE in
transaction (aborted)', so that once aborted we keep the message even if
the user tries to issue further statements other than ROLLBACK or
COMMIT.
This also solves the bug reported by Kris Jurka.
Joachim, credit will be to you, so please re-check.
(Further changes pending on HS side, so not all issues resolved by this.
I intend to use this mechanism for HS cancellations when
CONFLICT_MODE_ERROR, and another mechanism for CONFLICT_MODE_FATAL.)
--
Simon Riggs www.2ndQuadrant.com
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 12899,12905 SELECT set_config('log_statement_stats', 'off', false);
literalfunctionpg_cancel_backend/function(parameterpid/parameter typeint/)/literal
/entry
entrytypeboolean/type/entry
!entryCancel a backend's current query/entry
/row
row
entry
--- 12899,12905
literalfunctionpg_cancel_backend/function(parameterpid/parameter typeint/)/literal
/entry
entrytypeboolean/type/entry
!entryCancel a backend's current query or abort an idle transaction/entry
/row
row
entry
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 55,60 postgres 965 0.0 1.1 6152 1512 pts/1SN 13:17 0:00 postgres: stats c
--- 55,61
postgres 998 0.0 2.3 6532 2992 pts/1SN 13:18 0:00 postgres: tgl runbug 127.0.0.1 idle
postgres 1003 0.0 2.4 6532 3128 pts/1SN 13:19 0:00 postgres: tgl regression [local] SELECT waiting
postgres 1016 0.1 2.4 6532 3080 pts/1SN 13:19 0:00 postgres: tgl regression [local] idle in transaction
+ postgres 1066 0.1 2.4 6532 3080 pts/1SN 13:19 0:00 postgres: tgl regression [local] idle in transaction (aborted)
/screen
(The appropriate invocation of commandps/ varies across different
***
*** 77,82 postgres: replaceableuser/ replaceabledatabase/ replaceablehost/ re
--- 78,84
the life of the client connection, but the activity indicator changes.
The activity can be literalidle/ (i.e., waiting for a client command),
literalidle in transaction/ (waiting for client inside a commandBEGIN/ block),
+ literalidle in transaction (aborted)/ (waiting for client to commandROLLBACK/),
or a command type name such as literalSELECT/. Also,
literalwaiting/ is attached if the server process is presently waiting
on a lock held by another server process. In the above example we can infer
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***
*** 313,320 IsTransactionState(void)
/*
* IsAbortedTransactionBlockState
*
! * This returns true if we are currently running a query
! * within an aborted transaction block.
*/
bool
IsAbortedTransactionBlockState(void)
--- 313,319
/*
* IsAbortedTransactionBlockState
*
! * This returns true if we are within an aborted transaction block.
*/
bool
IsAbortedTransactionBlockState(void)
***
*** 2692,2697 AbortCurrentTransaction(void)
--- 2691,2737
}
/*
+ * AbortTransactionAndAnySubtransactions
+ *
+ * Similar to AbortCurrentTransaction but if any subtransactions
+ * in progress we abort them and all of their parents. So this is
+ * used when the caller wishes to make the abort untrappable by the user.
+ */
+ void
+ AbortTransactionAndAnySubtransactions(void)
+ {
+ TransactionState s = CurrentTransactionState;
+
+ switch (s-blockState)
+ {
+ case TBLOCK_DEFAULT:
+ case TBLOCK_STARTED:
+ case TBLOCK_BEGIN:
+ case TBLOCK_INPROGRESS:
+ case TBLOCK_END:
+ case TBLOCK_ABORT:
+ case TBLOCK_SUBABORT:
+ case TBLOCK_ABORT_END:
+ case TBLOCK_ABORT_PENDING:
+ case TBLOCK_PREPARE:
+ case TBLOCK_SUBABORT_END:
+ case TBLOCK_SUBABORT_RESTART:
+ AbortCurrentTransaction();
Re: [HACKERS] Cancelling idle in transaction state
On Thu, Dec 31, 2009 at 3:03 PM, Simon Riggs si...@2ndquadrant.com wrote: This patch extends SIGINT to allow cancellation of transactions while idle in both HS and normal mode. It also changes the standard message reported on an idle transaction in aborted state to 'IDLE in transaction (aborted)', so that once aborted we keep the message even if the user tries to issue further statements other than ROLLBACK or COMMIT. This also solves the bug reported by Kris Jurka. Was the bug reported by Kris really only about lost synchronization or was it about SIGINT now cancelling idle transactions which it did not do previously? I still think that we should have three transaction cancel modes, one to cancel an idle transaction, another one to cancel a running query and a third one that just cancels the transaction regardless of it being idle or not. This last one is what you are implementing now, and it is what HS wants to do. However I think that Kris only wants to cancel a running query but not an idle transaction. And an administrator who wants to cancel an idle transaction can never be sure that the transaction that he checked which has just been idle is still idle... (Further changes pending on HS side, so not all issues resolved by this. I intend to use this mechanism for HS cancellations when CONFLICT_MODE_ERROR, and another mechanism for CONFLICT_MODE_FATAL.) CONFLICT_MODE_FATAL is what you are planning to implement via SIGUSR1 multiplexing then? Joachim -- 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] Cancelling idle in transaction state
On Thu, 2009-12-31 at 15:41 +0100, Joachim Wieland wrote: I still think that we should have three transaction cancel modes, one to cancel an idle transaction, another one to cancel a running query and a third one that just cancels the transaction regardless of it being idle or not. This last one is what you are implementing now, and it is what HS wants to do. pg_cancel_backend() is currently conditional on whether a statement is active or not, so should really be called pg_cancel_if_active(). What people want is an unconditional way to stop a transaction. I don't see the need for 3 modes (and that has nothing to do with HS). -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Tue, 29 Dec 2009, Simon Riggs wrote: The proposal is to send an additional NOTICE to the client and abort all open transactions and subtransactions (this is what I got from the previous discussion). Would this work with JDBC driver and/or general protocol clients? A Notice would be easy to overlook. The JDBC driver wraps that as a SQLWarning which callers need to explicitly check for (and rarely do in my experience). So when they run their next statement they'll get an error saying that the current transaction is aborted, but they'll have no idea why as the warning was silently eaten. I'd prefer the transaction cancellation to come as an Error because that's what it really is. The only downside I can see is that a client would get confused if: 1) Transaction starts. 2) Idle transaction is killed and error message is given. 3) Client issues rollback 4) Client gets error message from saying the transaction was cancelled. Kris Jurka -- 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] Cancelling idle in transaction state
On Wed, Dec 30, 2009 at 12:28 AM, Simon Riggs si...@2ndquadrant.com wrote: I had to write an additional function AbortAnyTransaction() which aborts all transactions and subtransactions and leaves the transaction in the aborted state, is there an existing function to do this? AbortOutOfAnyTransaction() But this would clean up completely and not leave the transaction in the aborted state. Subsequent commands will be executed just fine instead of being refused with the error message that the transaction is already aborted... Right? Joachim -- 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] Cancelling idle in transaction state
On Wed, 2009-12-30 at 05:02 -0500, Kris Jurka wrote: On Tue, 29 Dec 2009, Simon Riggs wrote: The proposal is to send an additional NOTICE to the client and abort all open transactions and subtransactions (this is what I got from the previous discussion). Would this work with JDBC driver and/or general protocol clients? A Notice would be easy to overlook. The JDBC driver wraps that as a SQLWarning which callers need to explicitly check for (and rarely do in my experience). So when they run their next statement they'll get an error saying that the current transaction is aborted, but they'll have no idea why as the warning was silently eaten. I'd prefer the transaction cancellation to come as an Error because that's what it really is. I'm not certain of all of these points, but here goes: AFAIK, NOTICE was suggested because it can be sent at any time, whereas ERRORs are only associated with statements. http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-ASYNC It is possible for NoticeResponse messages to be generated due to outside activity; for example, if the database administrator commands a fast database shutdown, the backend will send a NoticeResponse indicating this fact before closing the connection. Accordingly, frontends should always be prepared to accept and display NoticeResponse messages, even when the connection is nominally idle. Can JDBC accept a NOTICE, yet throw an error? NOTICEs have a SQLState field just like ERRORs do, so you should be able to special case that. I understand that this will mean that we are enhancing the protocol for this release, but I don't have a better suggestion. The only downside I can see is that a client would get confused if: 1) Transaction starts. 2) Idle transaction is killed and error message is given. 3) Client issues rollback 4) Client gets error message from saying the transaction was cancelled. Are you saying that the client should send rollback and that it should generate no message? -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Wed, 2009-12-30 at 11:43 +0100, Joachim Wieland wrote: On Wed, Dec 30, 2009 at 12:28 AM, Simon Riggs si...@2ndquadrant.com wrote: I had to write an additional function AbortAnyTransaction() which aborts all transactions and subtransactions and leaves the transaction in the aborted state, is there an existing function to do this? AbortOutOfAnyTransaction() But this would clean up completely and not leave the transaction in the aborted state. True Subsequent commands will be executed just fine instead of being refused with the error message that the transaction is already aborted... True, but it is a subsequent transaction, not the same one. (I've checked). -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
Simon Riggs wrote: On Wed, 2009-12-30 at 05:02 -0500, Kris Jurka wrote: On Tue, 29 Dec 2009, Simon Riggs wrote: The proposal is to send an additional NOTICE to the client and abort all open transactions and subtransactions (this is what I got from the previous discussion). Would this work with JDBC driver and/or general protocol clients? A Notice would be easy to overlook. The JDBC driver wraps that as a SQLWarning which callers need to explicitly check for (and rarely do in my experience). So when they run their next statement they'll get an error saying that the current transaction is aborted, but they'll have no idea why as the warning was silently eaten. I'd prefer the transaction cancellation to come as an Error because that's what it really is. I'm not certain of all of these points, but here goes: AFAIK, NOTICE was suggested because it can be sent at any time, whereas ERRORs are only associated with statements. http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-ASYNC It is possible for NoticeResponse messages to be generated due to outside activity; for example, if the database administrator commands a fast database shutdown, the backend will send a NoticeResponse indicating this fact before closing the connection. Accordingly, frontends should always be prepared to accept and display NoticeResponse messages, even when the connection is nominally idle. Could we send an asynchronous notification immediately when the transaction is cancelled, but also change the error message you get in the subsequent commands. Clients that ignore the async notification would still see a proper error message at the ERROR. Something like: ERROR: current transaction was aborted because of conflict with recovery, commands ignored until end of transaction block instead of the usual current transaction is aborted, commands ignored until end of transaction block message. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] Cancelling idle in transaction state
On Wed, 30 Dec 2009, Simon Riggs wrote: http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-ASYNC It is possible for NoticeResponse messages to be generated due to outside activity; for example, if the database administrator commands a fast database shutdown, the backend will send a NoticeResponse indicating this fact before closing the connection. Accordingly, frontends should always be prepared to accept and display NoticeResponse messages, even when the connection is nominally idle. The problem is that frontends won't check the backend connection until they've already been given the next command to execute at which point it's too late. I think a lot of the discussion on this thread is wishful thinking about when a frontend will see the message and what they'll do with it. You would either need a multithreaded frontend that had some type of callback mechanism for these notices, or you'd need to poll the socket every so often to see if you'd received a notice. I don't think that describes most applications or client libraries. Can JDBC accept a NOTICE, yet throw an error? NOTICEs have a SQLState field just like ERRORs do, so you should be able to special case that. Yes, that's possible. The only downside I can see is that a client would get confused if: 1) Transaction starts. 2) Idle transaction is killed and error message is given. 3) Client issues rollback 4) Client gets error message from saying the transaction was cancelled. Are you saying that the client should send rollback and that it should generate no message? No, I'm saying if for some business logic reason the client decided it needed to rollback as it hadn't seen the error message yet. Kris Jurka -- 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] Cancelling idle in transaction state
On Wed, 30 Dec 2009, Heikki Linnakangas wrote: Could we send an asynchronous notification immediately when the transaction is cancelled, but also change the error message you get in the subsequent commands. Clients that ignore the async notification would still see a proper error message at the ERROR. +1 Kris Jurka -- 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] Cancelling idle in transaction state
On 30/12/2009 7:37 PM, Simon Riggs wrote: Can JDBC accept a NOTICE, yet throw an error? NOTICEs have a SQLState field just like ERRORs do, so you should be able to special case that. The JDBC driver would have to throw when the app code next interacted with the connection object anyway. It can't asynchronously throw an exception. Since the next interaction that can throw SQLException() is likely to be setup for or execution of a query, I'm not sure it makes any difference to the JDBC user whether query cancellation is reported as a NOTICE or an ERROR behind the scenes. Since the proposed patch leaves cancelled transactions in the error state, rather than closing them and leaving the connection clean and idle, it doesn't matter much if a client doesn't understand or check for the NOTICE. The app code will try to do work on the connection and that work will fail because the transaction is aborted, resulting in a normal SQLException reporting that the current transaction is aborted JDBC-using code has to be prepared to handle exceptions at any point of interaction with the JDBC driver anyway, and any code that isn't is buggy. Consequently there's LOTS of buggy JDBC code out there :-( as people often ignore exceptions thrown during operations they think can't fail. However, such buggy code is already broken by pg_cancel_backend() and pg_terminate_backend(), and won't be broken any more or differently by the proposed change, so I don't see a problem with it. Personally, I'd be happy to leave the JDBC driver as it was. It might be kind of handy if I could getWarnings() on the connection object without blocking so I could call it before I executed a statement on the connection ... but that'd always introduce a race between transaction cancellation/timeout and statement execution, so code must always be prepared to handle timeout/cancellation related failure anyway. As you say, the driver can special-case connection cancelled NOTICE mesages as errors and throw them at next user interaction it wants. But I'm not sure that's anything more than a kind of nice-to-have cosmetic feature. If the JDBC driver handled the NOTICE and threw a more informative SQLException to tell the app why the transaction was dead, that'd be nice, but hardly vital. It'd want to preserve the notice as an SQLWarning as well. I understand that this will mean that we are enhancing the protocol for this release, but I don't have a better suggestion. Only in an extremely backward compatible way - and it's more of a behavior change for the backend than a protocol change. Pg's backends change behaviour a whole lot more than that in a typical release... The only downside I can see is that a client would get confused if: 1) Transaction starts. 2) Idle transaction is killed and error message is given. 3) Client issues rollback 4) Client gets error message from saying the transaction was cancelled. For JDBC users, there is no transaction in progress is only reported as a SQLWarning via getWarnings(), so I'd be surprised if anything used it for more than logging or debugging purposes. Are you saying that the client should send rollback and that it should generate no message? -- Craig Ringer -- 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] Cancelling idle in transaction state
Craig Ringer cr...@postnewspapers.com.au wrote: It might be kind of handy if I could getWarnings() on the connection object without blocking so I could call it before I executed a statement on the connection ... but that'd always introduce a race between transaction cancellation/timeout and statement execution, so code must always be prepared to handle timeout/cancellation related failure anyway. +1 (I think) If I'm understanding this, it sounds to me like it would be most appropriate for the NOTICE to generate a warning at the connection level and for the next request to throw an exception in the format suggested by Heikki -- which I think is what Craig is suggesting. -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] Cancelling idle in transaction state
On Wed, 2009-12-30 at 14:15 +0200, Heikki Linnakangas wrote: Simon Riggs wrote: AFAIK, NOTICE was suggested because it can be sent at any time, whereas ERRORs are only associated with statements. http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-ASYNC It is possible for NoticeResponse messages to be generated due to outside activity; for example, if the database administrator commands a fast database shutdown, the backend will send a NoticeResponse indicating this fact before closing the connection. Accordingly, frontends should always be prepared to accept and display NoticeResponse messages, even when the connection is nominally idle. Could we send an asynchronous notification immediately when the transaction is cancelled, but also change the error message you get in the subsequent commands. Clients that ignore the async notification would still see a proper error message at the ERROR. Something like: ERROR: current transaction was aborted because of conflict with recovery, commands ignored until end of transaction block instead of the usual current transaction is aborted, commands ignored until end of transaction block message. This is possible, yes. I have an added complication, hinted at by Joachim, currently investigating. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote: On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: We are using NOTICE, not NOTIFY, assuming that we use anything at all (which I still regard as unnecessary). Please stop injecting confusion into the discussion. Attached is a minimal POC patch that allows to cancel an idle transaction with SIGINT. The HS patch also allows this in its current form but as Simon points out the client gets out of sync with it. Thanks for working on this. The proposal is to send an additional NOTICE to the client and abort all open transactions and subtransactions (this is what I got from the previous discussion). (Adding Kris into the discussion here.) Would this work with JDBC driver and/or general protocol clients? I had to write an additional function AbortAnyTransaction() which aborts all transactions and subtransactions and leaves the transaction in the aborted state, is there an existing function to do this? AbortOutOfAnyTransaction() We'd probably want to add a timeout for idle transactions also (which is a wishlist item since quite some time) and could also offer user functions like pg_cancel_idle_transaction(). Along this we might need to add internal reasons like we do for SIGUSR1 because we are now multiplexing different functionality onto the SIGINT signal. One might want to cancel an idle transaction only and not a running query, without keeping track of internal reasons one risks to cancel a legitimate query if that backend has started to work on a query again. Next project, not both at once. -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
We are using NOTICE, not NOTIFY, assuming that we use anything at all
(which I still regard as unnecessary). Please stop injecting confusion
into the discussion.
Attached is a minimal POC patch that allows to cancel an idle
transaction with SIGINT. The HS patch also allows this in its current
form but as Simon points out the client gets out of sync with it.
The proposal is to send an additional NOTICE to the client and abort
all open transactions and subtransactions (this is what I got from the
previous discussion).
I had to write an additional function AbortAnyTransaction() which
aborts all transactions and subtransactions and leaves the transaction
in the aborted state, is there an existing function to do this?
We'd probably want to add a timeout for idle transactions also (which
is a wishlist item since quite some time) and could also offer user
functions like pg_cancel_idle_transaction(). Along this we might need
to add internal reasons like we do for SIGUSR1 because we are now
multiplexing different functionality onto the SIGINT signal. One might
want to cancel an idle transaction only and not a running query,
without keeping track of internal reasons one risks to cancel a
legitimate query if that backend has started to work on a query again.
Comments?
Joachim
diff -cr cvs/src/backend/access/transam/xact.c cvs.build/src/backend/access/transam/xact.c
*** cvs/src/backend/access/transam/xact.c 2009-12-24 13:55:12.0 +0100
--- cvs.build/src/backend/access/transam/xact.c 2009-12-24 20:55:17.0 +0100
***
*** 2692,2697
--- 2692,2735
}
/*
+ * AbortAnyTransaction
+ */
+ void
+ AbortAnyTransaction(void)
+ {
+ TransactionState s = CurrentTransactionState;
+
+ switch (s-blockState)
+ {
+ case TBLOCK_DEFAULT:
+ case TBLOCK_STARTED:
+ case TBLOCK_BEGIN:
+ case TBLOCK_INPROGRESS:
+ case TBLOCK_END:
+ case TBLOCK_ABORT:
+ case TBLOCK_SUBABORT:
+ case TBLOCK_ABORT_END:
+ case TBLOCK_ABORT_PENDING:
+ case TBLOCK_PREPARE:
+ case TBLOCK_SUBABORT_END:
+ case TBLOCK_SUBABORT_RESTART:
+ AbortCurrentTransaction();
+ break;
+
+ case TBLOCK_SUBINPROGRESS:
+ case TBLOCK_SUBBEGIN:
+ case TBLOCK_SUBEND:
+ case TBLOCK_SUBABORT_PENDING:
+ case TBLOCK_SUBRESTART:
+ AbortSubTransaction();
+ CleanupSubTransaction();
+ AbortAnyTransaction();
+ break;
+ }
+ }
+
+
+ /*
* PreventTransactionChain
*
* This routine is to be called by statements that must not run inside
diff -cr cvs/src/backend/tcop/postgres.c cvs.build/src/backend/tcop/postgres.c
*** cvs/src/backend/tcop/postgres.c 2009-12-24 13:55:18.0 +0100
--- cvs.build/src/backend/tcop/postgres.c 2009-12-24 20:55:17.0 +0100
***
*** 2637,2643
if (!proc_exit_inprogress)
{
InterruptPending = true;
! QueryCancelPending = true;
/*
* If it's safe to interrupt, and we're waiting for a lock, service
--- 2637,2647
if (!proc_exit_inprogress)
{
InterruptPending = true;
!
! if (DoingCommandRead)
! TransactionCancelPending = true;
! else
! QueryCancelPending = true;
/*
* If it's safe to interrupt, and we're waiting for a lock, service
***
*** 2789,2794
--- 2793,2821
errmsg(canceling statement due to user request)));
}
}
+ if (TransactionCancelPending)
+ {
+ QueryCancelPending = false;
+ ImmediateInterruptOK = false; /* not idle anymore */
+
+ if (!IsTransactionOrTransactionBlock())
+ return;
+
+ if (IsAbortedTransactionBlockState())
+ return;
+
+ DisableNotifyInterrupt();
+ DisableCatchupInterrupt();
+
+ ereport(NOTICE,
+ (errcode(ERRCODE_QUERY_CANCELED),
+ errmsg(canceling transaction due to user request)));
+
+ AbortAnyTransaction();
+
+ set_ps_display(idle in transaction (aborted), false);
+ pgstat_report_activity(IDLE in transaction (aborted));
+ }
/* If we get here, do nothing (probably, QueryCancelPending was reset) */
}
diff -cr cvs/src/backend/utils/init/globals.c cvs.build/src/backend/utils/init/globals.c
*** cvs/src/backend/utils/init/globals.c 2009-12-09 11:24:42.0 +0100
--- cvs.build/src/backend/utils/init/globals.c 2009-12-24 20:55:17.0 +0100
***
*** 27,32
--- 27,33
volatile bool InterruptPending = false;
volatile bool QueryCancelPending = false;
+ volatile bool TransactionCancelPending = false;
volatile bool ProcDiePending = false;
volatile bool ImmediateInterruptOK = false;
volatile uint32 InterruptHoldoffCount = 0;
diff -cr cvs/src/include/access/xact.h cvs.build/src/include/access/xact.h
*** cvs/src/include/access/xact.h 2009-12-24 13:55:28.0 +0100
--- cvs.build/src/include/access/xact.h 2009-12-24 20:55:17.0 +0100
***
*** 204,209
--- 204,210
extern bool IsTransactionOrTransactionBlock(void);
extern char
Re: [HACKERS] Cancelling idle in transaction state
On Sun, 2009-12-06 at 07:58 +, Simon Riggs wrote: On Sat, 2009-12-05 at 18:13 -0700, James Pye wrote: On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: Thanks. Looks like good input. With the further clarification that we use NOTIFY it seems a solution is forming. If we use notify, then the sufficiently smart client (tm) should probably declared that it is waiting for such notify , no ? That would mean, that it should have issued either LISTEN CANCEL_IDLE_TRX_pid or with the new payload enabled NOTIFY just LISTEN CANCEL_IDLE_TRX and then the NOTIFY would include the pid of rolled back backend and possibly some other extra info. Otoh, we could also come up with something that looks like a NOTIFY from client end, but is sent only to one connection that is canceled instead of all listeners. -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training -- 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] Cancelling idle in transaction state
On Dec 6, 2009, at 2:58 AM, Simon Riggs si...@2ndquadrant.com wrote: On Sat, 2009-12-05 at 18:13 -0700, James Pye wrote: On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: Thanks. Looks like good input. With the further clarification that we use NOTIFY it seems a solution is forming. Notice, not NOTIFY. ...Robert -- 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] Cancelling idle in transaction state
Hannu Krosing ha...@2ndquadrant.com writes: On Sun, 2009-12-06 at 07:58 +, Simon Riggs wrote: Thanks. Looks like good input. With the further clarification that we use NOTIFY it seems a solution is forming. If we use notify, then the sufficiently smart client (tm) should probably declared that it is waiting for such notify , no ? We are using NOTICE, not NOTIFY, assuming that we use anything at all (which I still regard as unnecessary). Please stop injecting confusion into the discussion. 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] Cancelling idle in transaction state
I'd be very grateful to any hackers out there who are looking for a project before close of 8.5 to consider working on this. It's dang useful, both for Hot Standby and normal processing. (You'll have the added bonus of proving you're smarter than me!) On Wed, 2009-01-21 at 15:22 -0500, Bruce Momjian wrote: Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php --- Simon Riggs wrote: Currently SIGINT is ignored during IDLE in transaction, but we have recently agreed to allow this to cancel the transaction. We said we would do this in all cases, so this is a separate feature/patch (though Hot Standby requires it). A simple change allows the transaction to be cancelled, but there are some loose ends that I wish to discuss. If we are running a statement and a cancel is received, then we return the ERROR to the client, who is expecting it. If we cancel a transaction while the connection is idle, we have no way of signalling to the client program this has occurred. So the client finds out about this much later, not in fact until the next message is sent. Is there a mechanism for communicating the state back to the client? Will this be handled correctly with existing code? psql appears to be confused by a cancelled backend. I'm not familiar with these aspects of the code, so some clear suggestions are needed to allow me to work this out. I'm worried that this will delay things further otherwise. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: This allows locks to be released, but it is complex to report the cancellation back to the client. I think the answer here is that the server should *not* report the cancellation. Rather, it should mark the transaction as failed and let the client eventually sync its state on subsequent requests that will result in InFailedTransaction ERRORs. With such a solution, COMMITs issued to administrator cancelled transactions should result in an ERROR. Well, I suppose that would only be a requirement when: BEGIN; ... some work ... idle admin zapped this transaction more idle COMMIT; -- client needs to know that this failed, and it should be something louder than a ROLLBACK tag. :P So, if a command were issued to a cancelled transaction prior to a COMMIT: BEGIN; ... some work ... idle admin zapped this transaction SELECT * FROM something; -- fails, IFX ERROR emitted to client COMMIT; -- client was already notified of the xact failure by a prior command's error, so the normal ROLLBACK would be fine. Also, if immediate notification is seen as a necessity, a WARNING with a special code could be leveraged. Oh, or maybe use a dedicated LISTEN/NOTIFY channel? LISTEN pg_darn_admin_zapped_my_xact; to opt-in for transaction cancellation events that occur in *this* backend.. [Note: this is in addition to COMMITs emitting ERRORs] I can't see immediate notification being useful excepting some rather strange situations where the client left the transaction idle to go do other expensive operations that should be immediately interrupted if this particular transaction were to be cancelled for some reason.. Such a situation might even make sense if those expensive operations somehow depended on the locks held by the transaction, but I think that's a stretch. Not to mention that the client could just occasionally poll the transaction with 'SELECT 1's; no special WARNING or NOTIFY's would be necessary. -- 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] Cancelling idle in transaction state
On Sat, Dec 5, 2009 at 8:13 PM, James Pye li...@jwp.name wrote: I think the answer here is that the server should *not* report the cancellation. Rather, it should mark the transaction as failed and let the client eventually sync its state on subsequent requests that will result in InFailedTransaction ERRORs. [...] Also, if immediate notification is seen as a necessity, a WARNING with a special code could be leveraged. Oh, or maybe use a dedicated LISTEN/NOTIFY channel? LISTEN pg_darn_admin_zapped_my_xact; to opt-in for transaction cancellation events that occur in *this* backend.. [Note: this is in addition to COMMITs emitting ERRORs] I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. Currently, it appears that the only messages that the server can send back asynchronously are ParameterStatus and NotificationResponse. So we need to decide whether it's feasible/better to shoehorn this functionality into one of those message types, or whether we should bump the protocol version and add a new message type (cue: panic in the streets). On first examination (and I am not an expert in this area), ParameterStatus would seem to be the better choice, because it appears to me that all clients must be prepared to cope with such messages, whereas in theory a client might be unprepared for a NotificationResponse if it never executes LISTEN. (It seems clearly preferable not to require clients to issue an explicit LISTEN in order to enable this feature.) Going with that theory, we could pick a magical parameter status value, either something like __transaction_cancelled, or maybe even something that contains a character that isn't even legal in a normal parameter, like $transaction_cancelled, if we don't think that will break any clients. Then we could just report a value change for this whenever an idle transaction is cancelled. Clients who ignore this will find out when they next issue a query; others will know immediately. ...Robert -- 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] Cancelling idle in transaction state
Robert Haas robertmh...@gmail.com writes: I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. If you must have that, send a NOTICE. I don't actually see the point though. If the client was as smart and well-coded as all that, it wouldn't be sitting on an open transaction in the first place. Currently, it appears that the only messages that the server can send back asynchronously are ParameterStatus and NotificationResponse. Using either of those is completely inappropriate. 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] Cancelling idle in transaction state
On Sat, Dec 5, 2009 at 10:15 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. If you must have that, send a NOTICE. Ah ha! I missed that one. That's perfect. I don't actually see the point though. If the client was as smart and well-coded as all that, it wouldn't be sitting on an open transaction in the first place. Think about an interactive client. It's not the client's fault that the user has chosen to begin a transaction and then sit there cogitating, but the client would like to let the user know right away that their current transaction is defunct. ...Robert -- 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] Cancelling idle in transaction state
On Sat, 2009-12-05 at 18:13 -0700, James Pye wrote: On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: Thanks. Looks like good input. With the further clarification that we use NOTIFY it seems a solution is forming. Any other takers? -- Simon Riggs www.2ndQuadrant.com -- 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] Cancelling idle in transaction state
Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php --- Simon Riggs wrote: Currently SIGINT is ignored during IDLE in transaction, but we have recently agreed to allow this to cancel the transaction. We said we would do this in all cases, so this is a separate feature/patch (though Hot Standby requires it). A simple change allows the transaction to be cancelled, but there are some loose ends that I wish to discuss. If we are running a statement and a cancel is received, then we return the ERROR to the client, who is expecting it. If we cancel a transaction while the connection is idle, we have no way of signalling to the client program this has occurred. So the client finds out about this much later, not in fact until the next message is sent. Is there a mechanism for communicating the state back to the client? Will this be handled correctly with existing code? psql appears to be confused by a cancelled backend. I'm not familiar with these aspects of the code, so some clear suggestions are needed to allow me to work this out. I'm worried that this will delay things further otherwise. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Cancelling idle in transaction state
Simon Riggs wrote: On Wed, 2009-01-21 at 15:22 -0500, Bruce Momjian wrote: Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php This is part of Hot Standby. The bug is on the TODO list. Well, if it gets done for 8.4 then we can mark it completed; it not it will be there for 8.5. The behavior is useful independent of hot standby. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Cancelling idle in transaction state
On Wed, 2009-01-21 at 15:46 -0500, Bruce Momjian wrote: Simon Riggs wrote: On Wed, 2009-01-21 at 15:22 -0500, Bruce Momjian wrote: Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php This is part of Hot Standby. The bug is on the TODO list. Well, if it gets done for 8.4 then we can mark it completed; it not it will be there for 8.5. The behavior is useful independent of hot standby. At one time there was also a positive discussion on having something like: idle_in_transaction_timeout Does this play along with that? Joshua D.D rake -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- 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] Cancelling idle in transaction state
Joshua D. Drake wrote: On Wed, 2009-01-21 at 15:46 -0500, Bruce Momjian wrote: Simon Riggs wrote: On Wed, 2009-01-21 at 15:22 -0500, Bruce Momjian wrote: Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php This is part of Hot Standby. The bug is on the TODO list. Well, if it gets done for 8.4 then we can mark it completed; it not it will be there for 8.5. The behavior is useful independent of hot standby. At one time there was also a positive discussion on having something like: idle_in_transaction_timeout Yep, and already a TODO: Add idle_in_transaction_timeout GUC so locks are not held for long periods of time -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
