Hot Standby and query cancel

Started by Simon Riggsover 16 years ago5 messageshackers
Jump to latest
#1Simon Riggs
simon@2ndQuadrant.com

We've been chewing around query cancel on Hot Standby and I think things
have got fairly confusing, hence a new thread.

I enclose a patch that includes all the things that we all agree on so
far, in my understanding

* Recovery conflict processing uses SIGUSR1 rather than shmem per Tom,
while holding ProcArrayLock per Andres

* CONFLICT_MODE_ERROR throws ERROR when in a transaction, not idle and
not in subtransaction, otherwise becomes CONFLICT_MODE_FATAL per Tom and
other discussion

* Recovery abort message has additional detail, per Heikki

It doesn't include anything still under discussion, though is intended
as a base upon which further patches can progress independently.

* Infrastructure for supercancel, by Joachim Wieland

* Any of the many further ideas by Andres Freund

Please review this so we can move onto taking other issues one by one.
This is also a base for other HS work I need to complete.

I am still testing patch, so should be confident to commit tomorrow
barring issues.

--
Simon Riggs www.2ndQuadrant.com

Attachments:

hs_cancel.2010_Jan_13.patchtext/x-patch; charset=UTF-8; name=hs_cancel.2010_Jan_13.patchDownload+255-144
#2Andres Freund
andres@anarazel.de
In reply to: Simon Riggs (#1)
Re: Hot Standby and query cancel

Hi Simon,

On Wednesday 13 January 2010 19:24:22 Simon Riggs wrote:

We've been chewing around query cancel on Hot Standby and I think things
have got fairly confusing, hence a new thread.

Good idea.

I enclose a patch that includes all the things that we all agree on so
far, in my understanding

cool.

* Recovery conflict processing uses SIGUSR1 rather than shmem per Tom,
while holding ProcArrayLock per Andres

* CONFLICT_MODE_ERROR throws ERROR when in a transaction, not idle and
not in subtransaction, otherwise becomes CONFLICT_MODE_FATAL per Tom and
other discussion

* Recovery abort message has additional detail, per Heikki

It doesn't include anything still under discussion, though is intended
as a base upon which further patches can progress independently.

I am still testing patch, so should be confident to commit tomorrow
barring issues.

I have only looked at briefly because right now I dont have the time (going to
eat at a friends place...) but I think I spotted an issue:
The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt is not
correct right now because that returns true for TBLOCK_SUBABORT as well.
Wouldnt that mess with the case where were in a failed subxact and then
rollback only that subxact?

Andres

#3Simon Riggs
simon@2ndQuadrant.com
In reply to: Andres Freund (#2)
Re: Hot Standby and query cancel

On Wed, 2010-01-13 at 19:58 +0100, Andres Freund wrote:

I am still testing patch, so should be confident to commit tomorrow
barring issues.

I have only looked at briefly because right now I dont have the time (going to
eat at a friends place...) but I think I spotted an issue:
The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt is not
correct right now because that returns true for TBLOCK_SUBABORT as well.
Wouldnt that mess with the case where were in a failed subxact and then
rollback only that subxact?

Well spotted, yes.

--
Simon Riggs www.2ndQuadrant.com

Attachments:

hs_cancel.2010_Jan_13.v2.patchtext/x-patch; charset=UTF-8; name=hs_cancel.2010_Jan_13.v2.patchDownload+261-144
#4Simon Riggs
simon@2ndQuadrant.com
In reply to: Simon Riggs (#3)
Re: Hot Standby and query cancel

On Wed, 2010-01-13 at 19:23 +0000, Simon Riggs wrote:

On Wed, 2010-01-13 at 19:58 +0100, Andres Freund wrote:

I am still testing patch, so should be confident to commit tomorrow
barring issues.

I have only looked at briefly because right now I dont have the time (going to
eat at a friends place...) but I think I spotted an issue:
The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt is not
correct right now because that returns true for TBLOCK_SUBABORT as well.
Wouldnt that mess with the case where were in a failed subxact and then
rollback only that subxact?

Well spotted, yes.

Latest version of same patch, but uses conflict reasons passed-thru
directly from recovery to backend.

Please review, no commit before tomorrow.

--
Simon Riggs www.2ndQuadrant.com

Attachments:

hs_cancel.2010_Jan_14.patchtext/x-patch; charset=UTF-8; name=hs_cancel.2010_Jan_14.patchDownload+298-206
#5Andres Freund
andres@anarazel.de
In reply to: Simon Riggs (#4)
Re: Hot Standby and query cancel

On Thursday 14 January 2010 13:21:07 Simon Riggs wrote:

On Wed, 2010-01-13 at 19:23 +0000, Simon Riggs wrote:

On Wed, 2010-01-13 at 19:58 +0100, Andres Freund wrote:

I am still testing patch, so should be confident to commit tomorrow
barring issues.

I have only looked at briefly because right now I dont have the time
(going to eat at a friends place...) but I think I spotted an issue:
The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt
is not correct right now because that returns true for TBLOCK_SUBABORT
as well. Wouldnt that mess with the case where were in a failed
subxact and then rollback only that subxact?

Well spotted, yes.

Latest version of same patch, but uses conflict reasons passed-thru
directly from recovery to backend.

Please review, no commit before tomorrow.

I only noted a tiny thing (which was present earlier on):

snprintf(waitactivitymsg, sizeof(waitactivitymsg),
"waiting for max_standby_delay (%u ms)",
MaxStandbyDelay);
in ResolveRecoveryConflictWithVirtualXIDs.

Shouldnt that be seconds? Otherwise the check in WaitExceedsMaxStandbyDelay is
wrong...

Andres