From 23645f058231203d29f92162640a087003794137 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Mon, 6 Sep 2004 17:56:33 +0000 Subject: [PATCH] Fix incorrect ordering of smgr cleanup relative to buffer pin cleanup during transaction abort. Add a regression test case to catch related mistakes in future. Alvaro Herrera and Tom Lane. --- src/backend/access/transam/xact.c | 24 +++++++++++++--------- src/backend/storage/smgr/smgr.c | 4 ++-- src/test/regress/expected/transactions.out | 15 ++++++++++++++ src/test/regress/sql/transactions.sql | 13 ++++++++++++ 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index cc36548b181..92c9de0ea75 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.185 2004/08/30 19:00:03 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.186 2004/09/06 17:56:04 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1333,9 +1333,6 @@ CommitTransaction(void) * backend-wide state. */ - smgrDoPendingDeletes(true); - /* smgrcommit already done */ - CallXactCallbacks(XACT_EVENT_COMMIT, InvalidTransactionId); ResourceOwnerRelease(TopTransactionResourceOwner, @@ -1352,6 +1349,14 @@ CommitTransaction(void) */ AtEOXact_Inval(true); + /* + * Likewise, dropping of files deleted during the transaction is best done + * after releasing relcache and buffer pins. (This is not strictly + * necessary during commit, since such pins should have been released + * already, but this ordering is definitely critical during abort.) + */ + smgrDoPendingDeletes(true); + ResourceOwnerRelease(TopTransactionResourceOwner, RESOURCE_RELEASE_LOCKS, true, true); @@ -1363,6 +1368,7 @@ CommitTransaction(void) AtEOXact_SPI(true); AtEOXact_on_commit_actions(true, s->transactionIdData); AtEOXact_Namespace(true); + /* smgrcommit already done */ AtEOXact_Files(); pgstat_count_xact_commit(); @@ -1481,15 +1487,13 @@ AbortTransaction(void) * ordering. */ - smgrDoPendingDeletes(false); - smgrabort(); - CallXactCallbacks(XACT_EVENT_ABORT, InvalidTransactionId); ResourceOwnerRelease(TopTransactionResourceOwner, RESOURCE_RELEASE_BEFORE_LOCKS, false, true); AtEOXact_Inval(false); + smgrDoPendingDeletes(false); ResourceOwnerRelease(TopTransactionResourceOwner, RESOURCE_RELEASE_LOCKS, false, true); @@ -1501,6 +1505,7 @@ AbortTransaction(void) AtEOXact_SPI(false); AtEOXact_on_commit_actions(false, s->transactionIdData); AtEOXact_Namespace(false); + smgrabort(); AtEOXact_Files(); pgstat_count_xact_rollback(); @@ -3014,7 +3019,6 @@ CommitSubTransaction(void) AtSubCommit_Notify(); AtEOSubXact_UpdatePasswordFile(true, s->transactionIdData, s->parent->transactionIdData); - AtSubCommit_smgr(); CallXactCallbacks(XACT_EVENT_COMMIT_SUB, s->parent->transactionIdData); @@ -3024,6 +3028,7 @@ CommitSubTransaction(void) AtEOSubXact_RelationCache(true, s->transactionIdData, s->parent->transactionIdData); AtEOSubXact_Inval(true); + AtSubCommit_smgr(); ResourceOwnerRelease(s->curTransactionOwner, RESOURCE_RELEASE_LOCKS, true, false); @@ -3109,8 +3114,6 @@ AbortSubTransaction(void) RecordSubTransactionAbort(); /* Post-abort cleanup */ - AtSubAbort_smgr(); - CallXactCallbacks(XACT_EVENT_ABORT_SUB, s->parent->transactionIdData); ResourceOwnerRelease(s->curTransactionOwner, @@ -3119,6 +3122,7 @@ AbortSubTransaction(void) AtEOSubXact_RelationCache(false, s->transactionIdData, s->parent->transactionIdData); AtEOSubXact_Inval(false); + AtSubAbort_smgr(); ResourceOwnerRelease(s->curTransactionOwner, RESOURCE_RELEASE_LOCKS, false, false); diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 71e82d71ede..80b8c9f74a7 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/smgr/smgr.c,v 1.81 2004/08/30 02:54:39 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/smgr/smgr.c,v 1.82 2004/09/06 17:56:16 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -784,7 +784,7 @@ smgrcommit(void) } /* - * smgrabort() -- Abort changes made during the current transaction. + * smgrabort() -- Clean up after transaction abort. */ void smgrabort(void) diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 2b8470c931d..955558d690b 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -374,6 +374,21 @@ ERROR: portal "c" cannot be run FETCH 10 FROM c; ERROR: portal "c" cannot be run COMMIT; +-- test case for problems with dropping an open relation during abort +BEGIN; + savepoint x; + CREATE TABLE koju (a INT UNIQUE); +NOTICE: CREATE TABLE / UNIQUE will create implicit index "koju_a_key" for table "koju" + INSERT INTO koju VALUES (1); + INSERT INTO koju VALUES (1); +ERROR: duplicate key violates unique constraint "koju_a_key" + rollback to x; + CREATE TABLE koju (a INT UNIQUE); +NOTICE: CREATE TABLE / UNIQUE will create implicit index "koju_a_key" for table "koju" + INSERT INTO koju VALUES (1); + INSERT INTO koju VALUES (1); +ERROR: duplicate key violates unique constraint "koju_a_key" +ROLLBACK; DROP TABLE foo; DROP TABLE baz; DROP TABLE barbaz; diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index d83a9f077fa..81006d16d23 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -231,6 +231,19 @@ BEGIN; FETCH 10 FROM c; COMMIT; +-- test case for problems with dropping an open relation during abort +BEGIN; + savepoint x; + CREATE TABLE koju (a INT UNIQUE); + INSERT INTO koju VALUES (1); + INSERT INTO koju VALUES (1); + rollback to x; + + CREATE TABLE koju (a INT UNIQUE); + INSERT INTO koju VALUES (1); + INSERT INTO koju VALUES (1); +ROLLBACK; + DROP TABLE foo; DROP TABLE baz; DROP TABLE barbaz; -- GitLab