Thread: Hot Standby and query cancel
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
Attachment
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
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
Attachment
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
Attachment
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