mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-12-21 08:29:39 +08:00
Run a portal's cleanup hook immediately when pushing it to FAILED state.
This extends the changes of commit6252c4f9e2
so that we run the cleanup hook earlier for failure cases as well as success cases. As before, the point is to avoid an assertion failure from an Assert I added in commita874fe7b4c
, which was meant to check that no user-written code can be called during portal cleanup. This fixes a case reported by Pavan Deolasee in which the Assert could be triggered during backend exit (see the new regression test case), and also prevents the possibility that the cleanup hook is run after portions of the portal's state have already been recycled. That doesn't really matter in current usage, but it foreseeably could matter in the future. Back-patch to 9.1 where the Assert in question was added.
This commit is contained in:
parent
edec8c8e00
commit
4bfe68dfab
@ -224,7 +224,7 @@ PerformPortalClose(const char *name)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Note: PortalCleanup is called as a side-effect
|
* Note: PortalCleanup is called as a side-effect, if not already done.
|
||||||
*/
|
*/
|
||||||
PortalDrop(portal, false);
|
PortalDrop(portal, false);
|
||||||
}
|
}
|
||||||
@ -234,6 +234,10 @@ PerformPortalClose(const char *name)
|
|||||||
*
|
*
|
||||||
* Clean up a portal when it's dropped. This is the standard cleanup hook
|
* Clean up a portal when it's dropped. This is the standard cleanup hook
|
||||||
* for portals.
|
* for portals.
|
||||||
|
*
|
||||||
|
* Note: if portal->status is PORTAL_FAILED, we are probably being called
|
||||||
|
* during error abort, and must be careful to avoid doing anything that
|
||||||
|
* is likely to fail again.
|
||||||
*/
|
*/
|
||||||
void
|
void
|
||||||
PortalCleanup(Portal portal)
|
PortalCleanup(Portal portal)
|
||||||
@ -420,7 +424,7 @@ PersistHoldablePortal(Portal portal)
|
|||||||
PG_CATCH();
|
PG_CATCH();
|
||||||
{
|
{
|
||||||
/* Uncaught error while executing portal: mark it dead */
|
/* Uncaught error while executing portal: mark it dead */
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* Restore global vars and propagate error */
|
/* Restore global vars and propagate error */
|
||||||
ActivePortal = saveActivePortal;
|
ActivePortal = saveActivePortal;
|
||||||
|
@ -607,7 +607,7 @@ PortalStart(Portal portal, ParamListInfo params, bool use_active_snapshot)
|
|||||||
PG_CATCH();
|
PG_CATCH();
|
||||||
{
|
{
|
||||||
/* Uncaught error while executing portal: mark it dead */
|
/* Uncaught error while executing portal: mark it dead */
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* Restore global vars and propagate error */
|
/* Restore global vars and propagate error */
|
||||||
ActivePortal = saveActivePortal;
|
ActivePortal = saveActivePortal;
|
||||||
@ -829,7 +829,7 @@ PortalRun(Portal portal, long count, bool isTopLevel,
|
|||||||
PG_CATCH();
|
PG_CATCH();
|
||||||
{
|
{
|
||||||
/* Uncaught error while executing portal: mark it dead */
|
/* Uncaught error while executing portal: mark it dead */
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* Restore global vars and propagate error */
|
/* Restore global vars and propagate error */
|
||||||
if (saveMemoryContext == saveTopTransactionContext)
|
if (saveMemoryContext == saveTopTransactionContext)
|
||||||
@ -1446,7 +1446,7 @@ PortalRunFetch(Portal portal,
|
|||||||
PG_CATCH();
|
PG_CATCH();
|
||||||
{
|
{
|
||||||
/* Uncaught error while executing portal: mark it dead */
|
/* Uncaught error while executing portal: mark it dead */
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* Restore global vars and propagate error */
|
/* Restore global vars and propagate error */
|
||||||
ActivePortal = saveActivePortal;
|
ActivePortal = saveActivePortal;
|
||||||
|
@ -405,6 +405,8 @@ UnpinPortal(Portal portal)
|
|||||||
/*
|
/*
|
||||||
* MarkPortalDone
|
* MarkPortalDone
|
||||||
* Transition a portal from ACTIVE to DONE state.
|
* Transition a portal from ACTIVE to DONE state.
|
||||||
|
*
|
||||||
|
* NOTE: never set portal->status = PORTAL_DONE directly; call this instead.
|
||||||
*/
|
*/
|
||||||
void
|
void
|
||||||
MarkPortalDone(Portal portal)
|
MarkPortalDone(Portal portal)
|
||||||
@ -418,8 +420,36 @@ MarkPortalDone(Portal portal)
|
|||||||
* well do that now, since the portal can't be executed any more.
|
* well do that now, since the portal can't be executed any more.
|
||||||
*
|
*
|
||||||
* In some cases involving execution of a ROLLBACK command in an already
|
* In some cases involving execution of a ROLLBACK command in an already
|
||||||
* aborted transaction, this prevents an assertion failure from reaching
|
* aborted transaction, this prevents an assertion failure caused by
|
||||||
* AtCleanup_Portals with the cleanup hook still unexecuted.
|
* reaching AtCleanup_Portals with the cleanup hook still unexecuted.
|
||||||
|
*/
|
||||||
|
if (PointerIsValid(portal->cleanup))
|
||||||
|
{
|
||||||
|
(*portal->cleanup) (portal);
|
||||||
|
portal->cleanup = NULL;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* MarkPortalFailed
|
||||||
|
* Transition a portal into FAILED state.
|
||||||
|
*
|
||||||
|
* NOTE: never set portal->status = PORTAL_FAILED directly; call this instead.
|
||||||
|
*/
|
||||||
|
void
|
||||||
|
MarkPortalFailed(Portal portal)
|
||||||
|
{
|
||||||
|
/* Perform the state transition */
|
||||||
|
Assert(portal->status != PORTAL_DONE);
|
||||||
|
portal->status = PORTAL_FAILED;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Allow portalcmds.c to clean up the state it knows about. We might as
|
||||||
|
* well do that now, since the portal can't be executed any more.
|
||||||
|
*
|
||||||
|
* In some cases involving cleanup of an already aborted transaction, this
|
||||||
|
* prevents an assertion failure caused by reaching AtCleanup_Portals with
|
||||||
|
* the cleanup hook still unexecuted.
|
||||||
*/
|
*/
|
||||||
if (PointerIsValid(portal->cleanup))
|
if (PointerIsValid(portal->cleanup))
|
||||||
{
|
{
|
||||||
@ -455,6 +485,9 @@ PortalDrop(Portal portal, bool isTopCommit)
|
|||||||
* hook's responsibility to not try to do that more than once, in the case
|
* hook's responsibility to not try to do that more than once, in the case
|
||||||
* that failure occurs and then we come back to drop the portal again
|
* that failure occurs and then we come back to drop the portal again
|
||||||
* during transaction abort.
|
* during transaction abort.
|
||||||
|
*
|
||||||
|
* Note: in most paths of control, this will have been done already in
|
||||||
|
* MarkPortalDone or MarkPortalFailed. We're just making sure.
|
||||||
*/
|
*/
|
||||||
if (PointerIsValid(portal->cleanup))
|
if (PointerIsValid(portal->cleanup))
|
||||||
{
|
{
|
||||||
@ -713,7 +746,7 @@ AtAbort_Portals(void)
|
|||||||
|
|
||||||
/* Any portal that was actually running has to be considered broken */
|
/* Any portal that was actually running has to be considered broken */
|
||||||
if (portal->status == PORTAL_ACTIVE)
|
if (portal->status == PORTAL_ACTIVE)
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Do nothing else to cursors held over from a previous transaction.
|
* Do nothing else to cursors held over from a previous transaction.
|
||||||
@ -728,9 +761,12 @@ AtAbort_Portals(void)
|
|||||||
* AtSubAbort_Portals.
|
* AtSubAbort_Portals.
|
||||||
*/
|
*/
|
||||||
if (portal->status == PORTAL_READY)
|
if (portal->status == PORTAL_READY)
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* let portalcmds.c clean up the state it knows about */
|
/*
|
||||||
|
* Allow portalcmds.c to clean up the state it knows about, if we
|
||||||
|
* haven't already.
|
||||||
|
*/
|
||||||
if (PointerIsValid(portal->cleanup))
|
if (PointerIsValid(portal->cleanup))
|
||||||
{
|
{
|
||||||
(*portal->cleanup) (portal);
|
(*portal->cleanup) (portal);
|
||||||
@ -862,9 +898,12 @@ AtSubAbort_Portals(SubTransactionId mySubid,
|
|||||||
*/
|
*/
|
||||||
if (portal->status == PORTAL_READY ||
|
if (portal->status == PORTAL_READY ||
|
||||||
portal->status == PORTAL_ACTIVE)
|
portal->status == PORTAL_ACTIVE)
|
||||||
portal->status = PORTAL_FAILED;
|
MarkPortalFailed(portal);
|
||||||
|
|
||||||
/* let portalcmds.c clean up the state it knows about */
|
/*
|
||||||
|
* Allow portalcmds.c to clean up the state it knows about, if we
|
||||||
|
* haven't already.
|
||||||
|
*/
|
||||||
if (PointerIsValid(portal->cleanup))
|
if (PointerIsValid(portal->cleanup))
|
||||||
{
|
{
|
||||||
(*portal->cleanup) (portal);
|
(*portal->cleanup) (portal);
|
||||||
|
@ -207,6 +207,7 @@ extern Portal CreateNewPortal(void);
|
|||||||
extern void PinPortal(Portal portal);
|
extern void PinPortal(Portal portal);
|
||||||
extern void UnpinPortal(Portal portal);
|
extern void UnpinPortal(Portal portal);
|
||||||
extern void MarkPortalDone(Portal portal);
|
extern void MarkPortalDone(Portal portal);
|
||||||
|
extern void MarkPortalFailed(Portal portal);
|
||||||
extern void PortalDrop(Portal portal, bool isTopCommit);
|
extern void PortalDrop(Portal portal, bool isTopCommit);
|
||||||
extern Portal GetPortalByName(const char *name);
|
extern Portal GetPortalByName(const char *name);
|
||||||
extern void PortalDefineQuery(Portal portal,
|
extern void PortalDefineQuery(Portal portal,
|
||||||
|
@ -601,28 +601,11 @@ fetch from foo;
|
|||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
abort;
|
abort;
|
||||||
-- tests for the "tid" type
|
-- Test for successful cleanup of an aborted transaction at session exit.
|
||||||
SELECT '(3, 3)'::tid = '(3, 4)'::tid;
|
-- THIS MUST BE THE LAST TEST IN THIS FILE.
|
||||||
?column?
|
begin;
|
||||||
----------
|
select 1/0;
|
||||||
f
|
ERROR: division by zero
|
||||||
(1 row)
|
rollback to X;
|
||||||
|
ERROR: no such savepoint
|
||||||
SELECT '(3, 3)'::tid = '(3, 3)'::tid;
|
-- DO NOT ADD ANYTHING HERE.
|
||||||
?column?
|
|
||||||
----------
|
|
||||||
t
|
|
||||||
(1 row)
|
|
||||||
|
|
||||||
SELECT '(3, 3)'::tid <> '(3, 3)'::tid;
|
|
||||||
?column?
|
|
||||||
----------
|
|
||||||
f
|
|
||||||
(1 row)
|
|
||||||
|
|
||||||
SELECT '(3, 3)'::tid <> '(3, 4)'::tid;
|
|
||||||
?column?
|
|
||||||
----------
|
|
||||||
t
|
|
||||||
(1 row)
|
|
||||||
|
|
||||||
|
@ -368,8 +368,11 @@ fetch from foo;
|
|||||||
|
|
||||||
abort;
|
abort;
|
||||||
|
|
||||||
-- tests for the "tid" type
|
-- Test for successful cleanup of an aborted transaction at session exit.
|
||||||
SELECT '(3, 3)'::tid = '(3, 4)'::tid;
|
-- THIS MUST BE THE LAST TEST IN THIS FILE.
|
||||||
SELECT '(3, 3)'::tid = '(3, 3)'::tid;
|
|
||||||
SELECT '(3, 3)'::tid <> '(3, 3)'::tid;
|
begin;
|
||||||
SELECT '(3, 3)'::tid <> '(3, 4)'::tid;
|
select 1/0;
|
||||||
|
rollback to X;
|
||||||
|
|
||||||
|
-- DO NOT ADD ANYTHING HERE.
|
||||||
|
Loading…
Reference in New Issue
Block a user