Thread: Cancelling idle in transaction state
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.comPostgreSQL Training, Services and Support
Added to TODO:Allow administrators to cancel multi-statement idletransactions This allows locks to be released, but itis 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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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 <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > -- PostgreSQL - XMPP: jdrake@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997
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 forlong periods of time -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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 <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > -- Simon Riggs www.2ndQuadrant.com
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 thatwill result in InFailedTransaction ERRORs. With such a solution, COMMITs issued to administrator cancelled transactions should result in an ERROR. Well, I suppose thatwould 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 usea dedicated LISTEN/NOTIFY channel? "LISTEN pg_darn_admin_zapped_my_xact;" to opt-in for transaction cancellation eventsthat 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 transactionidle to go do other expensive operations that "should" be immediately interrupted if this particular transactionwere 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 clientcould just occasionally poll the transaction with 'SELECT 1's; no special WARNING or NOTIFY's would be necessary.
On Sat, Dec 5, 2009 at 8:13 PM, James Pye <lists@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 thatwill result in InFailedTransaction ERRORs. > [...] > Also, if immediate notification is seen as a necessity, a WARNING with a special code could be leveraged. Oh, or maybeuse a dedicated LISTEN/NOTIFY channel? "LISTEN pg_darn_admin_zapped_my_xact;" to opt-in for transaction cancellationevents 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
Robert Haas <robertmhaas@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
On Sat, Dec 5, 2009 at 10:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@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
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
On Sun, 2009-12-06 at 07:58 +0000, 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
On Dec 6, 2009, at 2:58 AM, Simon Riggs <simon@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
Hannu Krosing <hannu@2ndquadrant.com> writes:
> On Sun, 2009-12-06 at 07:58 +0000, 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
On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane <tgl@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
Attachment
On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote: > On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane <tgl@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
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
On Wed, Dec 30, 2009 at 12:28 AM, Simon Riggs <simon@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
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
On Wed, 2009-12-30 at 11:43 +0100, Joachim Wieland wrote: > On Wed, Dec 30, 2009 at 12:28 AM, Simon Riggs <simon@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
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
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
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
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
Craig Ringer <craig@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
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
On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote: > On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane <tgl@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
On Thu, 2009-12-31 at 11:10 +0000, 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 <tgl@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
Attachment
On Thu, Dec 31, 2009 at 3:03 PM, Simon Riggs <simon@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
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
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
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
On Jan 1, 2010, at 6:48 AM, Simon Riggs <simon@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
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
<div dir="ltr"><div class="gmail_quote">On Fri, Jan 1, 2010 at 8:38 PM, Robert Haas <span dir="ltr"><<a href="mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</a>></span>wrote:<br /><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> On Jan 1, 2010, at 6:48AM, Simon Riggs <simon@2ndQuadrant.com> w<div class="im"><br /><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> We could either endlesslyrepeat this<br /><br /> ERROR: current transaction is aborted because of conflict with<br /> recovery, commandsignored until end of transaction block<br /></blockquote><br /></div> +1 for this option.<div class="im"><br /><br/><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left:1ex;"> I'm also not sure why we would want to single out Hot Standby to<br /> generate the reason "because ofconflict with recovery" when no other<br /> ERROR source would generate such a reason.<br /></blockquote><br /></div> Well,most times when the transaction is aborted, it's because you did something wrong. Or at least, the failure is associatedwith some particular statement.<br /><br /> 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 suchcases.<br /><font color="#888888"> </font><br /></blockquote></div><br />Why not do the finger pointing (to HS) in theDETAIL field of the ERROR, and let the error message remain the same.<br /><br />Best regards,<br />-- <br />gurjeet.singh<br/>@ EnterpriseDB - The Enterprise Postgres Company<br /><a href="http://www.enterprisedb.com">http://www.enterprisedb.com</a><br/><br />singh.gurjeet@{ gmail | yahoo }.com<br />Twitter/Skype:singh_gurjeet<br /><br />Mail sent from my BlackLaptop device<br /></div>
On Fri, 2010-01-01 at 07:08 -0800, Robert Haas wrote: > On Jan 1, 2010, at 6:48 AM, Simon Riggs <simon@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
On Jan 1, 2010, at 8:30 AM, Simon Riggs <simon@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 <simon@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
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 <joe@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
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
On Jan 1, 2010, at 9:42 AM, Simon Riggs <simon@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
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
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
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
On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs <simon@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
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
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 <kleptog@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.
On Fri, Jan 1, 2010 at 1:39 PM, Simon Riggs <simon@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
On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs <simon@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
On Wed, Jan 6, 2010 at 4:37 PM, Joachim Wieland <joe@mcknight.de> wrote: > On Fri, Jan 1, 2010 at 10:45 PM, Simon Riggs <simon@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
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