Re: 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