From 7b640b0345dc4fbd39ff568700985b432f6afa07 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 4 Dec 2008 14:51:02 +0000
Subject: [PATCH] Fix a couple of snapshot management bugs in the new
ResourceOwner world: non-writable large objects need to have their snapshots
registered on the transaction resowner, not the current portal's, because it
must persist until the large object is closed (which the portal does not).
Also, ensure that the serializable snapshot is recorded by the transaction
resource owner too, even when a subtransaction has changed the current
resource owner before serializable is taken.
Per bug reports from Pavan Deolasee.
---
src/backend/access/transam/xact.c | 8 ++-
src/backend/storage/large_object/inv_api.c | 15 ++++-
src/backend/utils/time/snapmgr.c | 68 ++++++++++++++++----
src/include/utils/snapmgr.h | 6 +-
src/test/regress/input/largeobject.source | 5 ++
src/test/regress/output/largeobject.source | 9 +++
src/test/regress/output/largeobject_1.source | 9 +++
7 files changed, 101 insertions(+), 19 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b61fe41083f..c5a1b33a9e9 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.269 2008/11/19 10:34:50 heikki Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.270 2008/12/04 14:51:02 alvherre Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1667,6 +1667,9 @@ CommitTransaction(void)
/* Clean up the relation cache */
AtEOXact_RelationCache(true);
+ /* Clean up the snapshot manager */
+ AtEarlyCommit_Snapshot();
+
/*
* Make catalog changes visible to all backends. This has to happen after
* relcache references are dropped (see comments for
@@ -1906,6 +1909,9 @@ PrepareTransaction(void)
/* Clean up the relation cache */
AtEOXact_RelationCache(true);
+ /* Clean up the snapshot manager */
+ AtEarlyCommit_Snapshot();
+
/* notify and flatfiles don't need a postprepare call */
PostPrepare_PgStat();
diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
index 3936260e6cf..14f101adb34 100644
--- a/src/backend/storage/large_object/inv_api.c
+++ b/src/backend/storage/large_object/inv_api.c
@@ -24,7 +24,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.135 2008/11/02 01:45:28 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.136 2008/12/04 14:51:02 alvherre Exp $
*
*-------------------------------------------------------------------------
*/
@@ -247,7 +247,13 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt)
}
else if (flags & INV_READ)
{
- retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
+ /*
+ * We must register the snapshot in TopTransaction's resowner,
+ * because it must stay alive until the LO is closed rather than until
+ * the current portal shuts down.
+ */
+ retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(),
+ TopTransactionResourceOwner);
retval->flags = IFS_RDLOCK;
}
else
@@ -270,8 +276,11 @@ void
inv_close(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
+
if (obj_desc->snapshot != SnapshotNow)
- UnregisterSnapshot(obj_desc->snapshot);
+ UnregisterSnapshotFromOwner(obj_desc->snapshot,
+ TopTransactionResourceOwner);
+
pfree(obj_desc);
}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 1107abdf27a..d37dc5df84c 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -19,7 +19,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.7 2008/11/25 20:28:29 alvherre Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.8 2008/12/04 14:51:02 alvherre Exp $
*
*-------------------------------------------------------------------------
*/
@@ -136,7 +136,8 @@ GetTransactionSnapshot(void)
*/
if (IsXactIsoLevelSerializable)
{
- CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
+ CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
+ TopTransactionResourceOwner);
registered_serializable = true;
}
@@ -345,14 +346,27 @@ ActiveSnapshotSet(void)
/*
* RegisterSnapshot
- * Register a snapshot as being in use
+ * Register a snapshot as being in use by the current resource owner
*
* If InvalidSnapshot is passed, it is not registered.
*/
Snapshot
RegisterSnapshot(Snapshot snapshot)
{
- Snapshot snap;
+ if (snapshot == InvalidSnapshot)
+ return InvalidSnapshot;
+
+ return RegisterSnapshotOnOwner(snapshot, CurrentResourceOwner);
+}
+
+/*
+ * RegisterSnapshotOnOwner
+ * As above, but use the specified resource owner
+ */
+Snapshot
+RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
+{
+ Snapshot snap;
if (snapshot == InvalidSnapshot)
return InvalidSnapshot;
@@ -361,9 +375,9 @@ RegisterSnapshot(Snapshot snapshot)
snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
/* and tell resowner.c about it */
- ResourceOwnerEnlargeSnapshots(CurrentResourceOwner);
+ ResourceOwnerEnlargeSnapshots(owner);
snap->regd_count++;
- ResourceOwnerRememberSnapshot(CurrentResourceOwner, snap);
+ ResourceOwnerRememberSnapshot(owner, snap);
RegisteredSnapshots++;
@@ -379,6 +393,19 @@ RegisterSnapshot(Snapshot snapshot)
*/
void
UnregisterSnapshot(Snapshot snapshot)
+{
+ if (snapshot == NULL)
+ return;
+
+ UnregisterSnapshotFromOwner(snapshot, CurrentResourceOwner);
+}
+
+/*
+ * UnregisterSnapshotFromOwner
+ * As above, but use the specified resource owner
+ */
+void
+UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner)
{
if (snapshot == NULL)
return;
@@ -386,7 +413,7 @@ UnregisterSnapshot(Snapshot snapshot)
Assert(snapshot->regd_count > 0);
Assert(RegisteredSnapshots > 0);
- ResourceOwnerForgetSnapshot(CurrentResourceOwner, snapshot);
+ ResourceOwnerForgetSnapshot(owner, snapshot);
RegisteredSnapshots--;
if (--snapshot->regd_count == 0 && snapshot->active_count == 0)
{
@@ -463,6 +490,26 @@ AtSubAbort_Snapshot(int level)
SnapshotResetXmin();
}
+/*
+ * AtEarlyCommit_Snapshot
+ *
+ * Snapshot manager's cleanup function, to be called on commit, before
+ * doing resowner.c resource release.
+ */
+void
+AtEarlyCommit_Snapshot(void)
+{
+ /*
+ * On a serializable transaction we must unregister our private refcount to
+ * the serializable snapshot.
+ */
+ if (registered_serializable)
+ UnregisterSnapshotFromOwner(CurrentSnapshot,
+ TopTransactionResourceOwner);
+ registered_serializable = false;
+
+}
+
/*
* AtEOXact_Snapshot
* Snapshot manager's cleanup function for end of transaction
@@ -475,13 +522,6 @@ AtEOXact_Snapshot(bool isCommit)
{
ActiveSnapshotElt *active;
- /*
- * On a serializable snapshot we must first unregister our private
- * refcount to the serializable snapshot.
- */
- if (registered_serializable)
- UnregisterSnapshot(CurrentSnapshot);
-
if (RegisteredSnapshots != 0)
elog(WARNING, "%d registered snapshots seem to remain after cleanup",
RegisteredSnapshots);
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 8f3cc44c57b..f4e55c9930f 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -6,13 +6,14 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.2 2008/05/12 20:02:02 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.3 2008/12/04 14:51:02 alvherre Exp $
*
*-------------------------------------------------------------------------
*/
#ifndef SNAPMGR_H
#define SNAPMGR_H
+#include "utils/resowner.h"
#include "utils/snapshot.h"
@@ -34,9 +35,12 @@ extern bool ActiveSnapshotSet(void);
extern Snapshot RegisterSnapshot(Snapshot snapshot);
extern void UnregisterSnapshot(Snapshot snapshot);
+extern Snapshot RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner);
+extern void UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner);
extern void AtSubCommit_Snapshot(int level);
extern void AtSubAbort_Snapshot(int level);
+extern void AtEarlyCommit_Snapshot(void);
extern void AtEOXact_Snapshot(bool isCommit);
#endif /* SNAPMGR_H */
diff --git a/src/test/regress/input/largeobject.source b/src/test/regress/input/largeobject.source
index 1d62caa3eaf..46ba9261ac5 100644
--- a/src/test/regress/input/largeobject.source
+++ b/src/test/regress/input/largeobject.source
@@ -83,6 +83,11 @@ SELECT lo_close(fd) FROM lotest_stash_values;
END;
+-- Test resource management
+BEGIN;
+SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
+ABORT;
+
-- Test truncation.
BEGIN;
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));
diff --git a/src/test/regress/output/largeobject.source b/src/test/regress/output/largeobject.source
index 36b51fdccdd..9d69f6c913e 100644
--- a/src/test/regress/output/largeobject.source
+++ b/src/test/regress/output/largeobject.source
@@ -116,6 +116,15 @@ SELECT lo_close(fd) FROM lotest_stash_values;
(1 row)
END;
+-- Test resource management
+BEGIN;
+SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
+ lo_open
+---------
+ 0
+(1 row)
+
+ABORT;
-- Test truncation.
BEGIN;
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));
diff --git a/src/test/regress/output/largeobject_1.source b/src/test/regress/output/largeobject_1.source
index f1157e45718..1fbc29c2517 100644
--- a/src/test/regress/output/largeobject_1.source
+++ b/src/test/regress/output/largeobject_1.source
@@ -116,6 +116,15 @@ SELECT lo_close(fd) FROM lotest_stash_values;
(1 row)
END;
+-- Test resource management
+BEGIN;
+SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
+ lo_open
+---------
+ 0
+(1 row)
+
+ABORT;
-- Test truncation.
BEGIN;
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));
--
GitLab