snapshot leak and core dump with serializable transactions
The following test flashes snapshot leak warning and subsequently dumps
core. Though this looks very similar to other bug report, this is a
different issue.
postgres=# BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE ;
BEGIN
postgres=# SAVEPOINT A;
SAVEPOINT
postgres=# SELECT count(*) from pg_class;
count
-------
227
(1 row)
postgres=# RELEASE SAVEPOINT A;
WARNING: Snapshot reference leak: Snapshot 0x9e3e4d4 still referenced
RELEASE
postgres=# SELECT count(*) from pg_class;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>
I looked at this briefly and ISTM that there are couple of issues here:
1. Since "SAVEPOINT A" is the first statement in the transaction, a
subtransaction is started and CurrentResourceOwner is set to the resource
owner of the subtransaction. Later when serializable snapshot is taken, its
recorded in the subtransaction resource owner. Obviously, when the
subtransaction commits, it complains about the snapshot leak because the
serializable snapshot is not yet unregistered.
So I tried to ensure that the serializable snapshot is always recorded in
the TopTransactionResourceOwner. It solved the above issue, but there is
still a core dump when the top transaction is committed. That leads to the
second issue.
2. In CommitTransaction(), I think we should call AtEOXact_Snapshot *before*
releasing the resource owners. Otherwise, ResourceOwnerReleaseInternal
complains about snapshot leak and then forcefully unregisters the snapshot.
Later when AtEOXact_Snapshot is called, it again tries to unregister the
serializable snapshot and assertion fails.
The attached patch fixes these issues.
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
Attachments:
snapshot-leak.patchtext/x-patch; name=snapshot-leak.patchDownload
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.269
diff -c -p -r1.269 xact.c
*** src/backend/access/transam/xact.c 19 Nov 2008 10:34:50 -0000 1.269
--- src/backend/access/transam/xact.c 3 Dec 2008 12:47:35 -0000
*************** CommitTransaction(void)
*** 1685,1690 ****
--- 1685,1691 ----
smgrDoPendingDeletes(true);
AtEOXact_MultiXact();
+ AtEOXact_Snapshot(true);
ResourceOwnerRelease(TopTransactionResourceOwner,
RESOURCE_RELEASE_LOCKS,
*************** CommitTransaction(void)
*** 1706,1712 ****
AtEOXact_ComboCid();
AtEOXact_HashTables(true);
AtEOXact_PgStat(true);
- AtEOXact_Snapshot(true);
pgstat_report_xact_timestamp(0);
CurrentResourceOwner = NULL;
--- 1707,1712 ----
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.7
diff -c -p -r1.7 snapmgr.c
*** src/backend/utils/time/snapmgr.c 25 Nov 2008 20:28:29 -0000 1.7
--- src/backend/utils/time/snapmgr.c 3 Dec 2008 12:47:36 -0000
*************** GetTransactionSnapshot(void)
*** 136,142 ****
--- 136,145 ----
*/
if (IsXactIsoLevelSerializable)
{
+ ResourceOwner oldowner = CurrentResourceOwner;
+ CurrentResourceOwner = TopTransactionResourceOwner;
CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
+ CurrentResourceOwner = oldowner;
registered_serializable = true;
}
*************** AtEOXact_Snapshot(bool isCommit)
*** 480,486 ****
--- 483,494 ----
* refcount to the serializable snapshot.
*/
if (registered_serializable)
+ {
+ ResourceOwner oldowner = CurrentResourceOwner;
+ CurrentResourceOwner = TopTransactionResourceOwner;
UnregisterSnapshot(CurrentSnapshot);
+ CurrentResourceOwner = oldowner;
+ }
if (RegisteredSnapshots != 0)
elog(WARNING, "%d registered snapshots seem to remain after cleanup",
Pavan Deolasee escribi�:
2. In CommitTransaction(), I think we should call AtEOXact_Snapshot *before*
releasing the resource owners. Otherwise, ResourceOwnerReleaseInternal
complains about snapshot leak and then forcefully unregisters the snapshot.
Later when AtEOXact_Snapshot is called, it again tries to unregister the
serializable snapshot and assertion fails.
Hmm, I've been wondering if we can get away with not having
AtEOXact_Snapshot at all.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
2. In CommitTransaction(), I think we should call AtEOXact_Snapshot *before*
releasing the resource owners.
That's absolutely wrong. It'll complain about whatever snapshots the
owners still hold.
regards, tom lane
On Wed, Dec 3, 2008 at 7:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
That's absolutely wrong. It'll complain about whatever snapshots the
owners still hold.
You must be right; I don't understand that code much. But don't we expect
the snapshots to be cleanly released at that point and if not we flash
warnings anyways ? AtEOXact_Snapshot only unregisters the serialized
snapshot which otherwise release resource owner will complain about.
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
Pavan Deolasee escribi�:
On Wed, Dec 3, 2008 at 7:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
That's absolutely wrong. It'll complain about whatever snapshots the
owners still hold.You must be right; I don't understand that code much. But don't we expect
the snapshots to be cleanly released at that point and if not we flash
warnings anyways ? AtEOXact_Snapshot only unregisters the serialized
snapshot which otherwise release resource owner will complain about.
Yeah, we need two "at-commit" routines, one of which needs to be called
early. I'm prepping a patch.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera escribi�:
Yeah, we need two "at-commit" routines, one of which needs to be called
early. I'm prepping a patch.
Here it is ... the large object patch is also included. I've created
new functions to specify the resource owner to register a snapshot in;
now that there are two callers, it seems likely that there will be more
in the future.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
lobj-snapshot-4.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.269
diff -c -p -r1.269 xact.c
*** src/backend/access/transam/xact.c 19 Nov 2008 10:34:50 -0000 1.269
--- src/backend/access/transam/xact.c 3 Dec 2008 19:30:35 -0000
*************** CommitTransaction(void)
*** 1667,1672 ****
--- 1667,1675 ----
/* 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
*************** PrepareTransaction(void)
*** 1906,1911 ****
--- 1909,1917 ----
/* 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();
Index: src/backend/storage/large_object/inv_api.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/large_object/inv_api.c,v
retrieving revision 1.135
diff -c -p -r1.135 inv_api.c
*** src/backend/storage/large_object/inv_api.c 2 Nov 2008 01:45:28 -0000 1.135
--- src/backend/storage/large_object/inv_api.c 3 Dec 2008 18:59:43 -0000
*************** inv_open(Oid lobjId, int flags, MemoryCo
*** 247,253 ****
}
else if (flags & INV_READ)
{
! retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
retval->flags = IFS_RDLOCK;
}
else
--- 247,254 ----
}
else if (flags & INV_READ)
{
! retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(),
! TopTransactionResourceOwner);
retval->flags = IFS_RDLOCK;
}
else
*************** void
*** 270,277 ****
inv_close(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
if (obj_desc->snapshot != SnapshotNow)
! UnregisterSnapshot(obj_desc->snapshot);
pfree(obj_desc);
}
--- 271,281 ----
inv_close(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
+
if (obj_desc->snapshot != SnapshotNow)
! UnregisterSnapshotFromOwner(obj_desc->snapshot,
! TopTransactionResourceOwner);
!
pfree(obj_desc);
}
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.7
diff -c -p -r1.7 snapmgr.c
*** src/backend/utils/time/snapmgr.c 25 Nov 2008 20:28:29 -0000 1.7
--- src/backend/utils/time/snapmgr.c 3 Dec 2008 19:28:50 -0000
*************** GetTransactionSnapshot(void)
*** 136,142 ****
*/
if (IsXactIsoLevelSerializable)
{
! CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
registered_serializable = true;
}
--- 136,143 ----
*/
if (IsXactIsoLevelSerializable)
{
! CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
! TopTransactionResourceOwner);
registered_serializable = true;
}
*************** ActiveSnapshotSet(void)
*** 345,351 ****
/*
* RegisterSnapshot
! * Register a snapshot as being in use
*
* If InvalidSnapshot is passed, it is not registered.
*/
--- 346,352 ----
/*
* RegisterSnapshot
! * Register a snapshot as being in use by the current resource owner
*
* If InvalidSnapshot is passed, it is not registered.
*/
*************** RegisterSnapshot(Snapshot snapshot)
*** 371,376 ****
--- 372,396 ----
}
/*
+ * As above, but register it on a specific resource owner
+ */
+ Snapshot
+ RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
+ {
+ Snapshot retval;
+ ResourceOwner save_CurrentResourceOwner;
+
+ save_CurrentResourceOwner = CurrentResourceOwner;
+ CurrentResourceOwner = TopTransactionResourceOwner;
+
+ retval = RegisterSnapshot(snapshot);
+
+ CurrentResourceOwner = save_CurrentResourceOwner;
+
+ return retval;
+ }
+
+ /*
* UnregisterSnapshot
*
* Decrement the reference count of a snapshot, remove the corresponding
*************** UnregisterSnapshot(Snapshot snapshot)
*** 395,400 ****
--- 415,433 ----
}
}
+ void
+ UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner)
+ {
+ ResourceOwner save_CurrentResourceOwner;
+
+ save_CurrentResourceOwner = CurrentResourceOwner;
+ CurrentResourceOwner = TopTransactionResourceOwner;
+
+ UnregisterSnapshot(snapshot);
+
+ CurrentResourceOwner = save_CurrentResourceOwner;
+ }
+
/*
* SnapshotResetXmin
*
*************** AtSubAbort_Snapshot(int level)
*** 463,468 ****
--- 496,514 ----
SnapshotResetXmin();
}
+ void
+ AtEarlyCommit_Snapshot(void)
+ {
+ /*
+ * On a serializable transaction we must first unregister our private
+ * refcount to the serializable snapshot.
+ */
+ if (registered_serializable)
+ UnregisterSnapshotFromOwner(CurrentSnapshot,
+ TopTransactionResourceOwner);
+
+ }
+
/*
* AtEOXact_Snapshot
* Snapshot manager's cleanup function for end of transaction
*************** AtEOXact_Snapshot(bool isCommit)
*** 475,487 ****
{
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);
--- 521,526 ----
Index: src/include/utils/snapmgr.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/snapmgr.h,v
retrieving revision 1.2
diff -c -p -r1.2 snapmgr.h
*** src/include/utils/snapmgr.h 12 May 2008 20:02:02 -0000 1.2
--- src/include/utils/snapmgr.h 3 Dec 2008 19:31:06 -0000
***************
*** 13,18 ****
--- 13,19 ----
#ifndef SNAPMGR_H
#define SNAPMGR_H
+ #include "utils/resowner.h"
#include "utils/snapshot.h"
*************** extern bool ActiveSnapshotSet(void);
*** 34,42 ****
--- 35,46 ----
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 */
Index: src/test/regress/input/largeobject.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/input/largeobject.source,v
retrieving revision 1.4
diff -c -p -r1.4 largeobject.source
*** src/test/regress/input/largeobject.source 3 Mar 2007 22:57:03 -0000 1.4
--- src/test/regress/input/largeobject.source 2 Dec 2008 14:27:26 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 83,88 ****
--- 83,93 ----
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));
Index: src/test/regress/output/largeobject.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/output/largeobject.source,v
retrieving revision 1.4
diff -c -p -r1.4 largeobject.source
*** src/test/regress/output/largeobject.source 3 Mar 2007 22:57:04 -0000 1.4
--- src/test/regress/output/largeobject.source 2 Dec 2008 14:42:34 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 116,121 ****
--- 116,130 ----
(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));
Index: src/test/regress/output/largeobject_1.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/output/largeobject_1.source,v
retrieving revision 1.1
diff -c -p -r1.1 largeobject_1.source
*** src/test/regress/output/largeobject_1.source 10 Mar 2007 03:42:19 -0000 1.1
--- src/test/regress/output/largeobject_1.source 2 Dec 2008 14:42:51 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 116,121 ****
--- 116,130 ----
(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));
Alvaro Herrera wrote:
Alvaro Herrera escribi�:
Yeah, we need two "at-commit" routines, one of which needs to be called
early. I'm prepping a patch.Here it is ... the large object patch is also included. I've created
new functions to specify the resource owner to register a snapshot in;
now that there are two callers, it seems likely that there will be more
in the future.
I'm surprised you implemented RegisterSnapshotOnOwner by switching
CurrentResourceOwner and calling RegisterSnapshot, rather than
implementing RegisterSnapshot by calling RegisterSnapshotOnOwner(...,
CurrentResourceOwner).
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas escribi�:
I'm surprised you implemented RegisterSnapshotOnOwner by switching
CurrentResourceOwner and calling RegisterSnapshot, rather than
implementing RegisterSnapshot by calling RegisterSnapshotOnOwner(...,
CurrentResourceOwner).
Yeah, that was plenty silly. Updated patch attached.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
lobj-snapshot-5.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.269
diff -c -p -r1.269 xact.c
*** src/backend/access/transam/xact.c 19 Nov 2008 10:34:50 -0000 1.269
--- src/backend/access/transam/xact.c 3 Dec 2008 19:30:35 -0000
*************** CommitTransaction(void)
*** 1667,1672 ****
--- 1667,1675 ----
/* 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
*************** PrepareTransaction(void)
*** 1906,1911 ****
--- 1909,1917 ----
/* 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();
Index: src/backend/storage/large_object/inv_api.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/large_object/inv_api.c,v
retrieving revision 1.135
diff -c -p -r1.135 inv_api.c
*** src/backend/storage/large_object/inv_api.c 2 Nov 2008 01:45:28 -0000 1.135
--- src/backend/storage/large_object/inv_api.c 3 Dec 2008 18:59:43 -0000
*************** inv_open(Oid lobjId, int flags, MemoryCo
*** 247,253 ****
}
else if (flags & INV_READ)
{
! retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
retval->flags = IFS_RDLOCK;
}
else
--- 247,254 ----
}
else if (flags & INV_READ)
{
! retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(),
! TopTransactionResourceOwner);
retval->flags = IFS_RDLOCK;
}
else
*************** void
*** 270,277 ****
inv_close(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
if (obj_desc->snapshot != SnapshotNow)
! UnregisterSnapshot(obj_desc->snapshot);
pfree(obj_desc);
}
--- 271,281 ----
inv_close(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
+
if (obj_desc->snapshot != SnapshotNow)
! UnregisterSnapshotFromOwner(obj_desc->snapshot,
! TopTransactionResourceOwner);
!
pfree(obj_desc);
}
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.7
diff -c -p -r1.7 snapmgr.c
*** src/backend/utils/time/snapmgr.c 25 Nov 2008 20:28:29 -0000 1.7
--- src/backend/utils/time/snapmgr.c 3 Dec 2008 20:45:43 -0000
*************** GetTransactionSnapshot(void)
*** 136,142 ****
*/
if (IsXactIsoLevelSerializable)
{
! CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
registered_serializable = true;
}
--- 136,143 ----
*/
if (IsXactIsoLevelSerializable)
{
! CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
! TopTransactionResourceOwner);
registered_serializable = true;
}
*************** ActiveSnapshotSet(void)
*** 345,351 ****
/*
* RegisterSnapshot
! * Register a snapshot as being in use
*
* If InvalidSnapshot is passed, it is not registered.
*/
--- 346,352 ----
/*
* RegisterSnapshot
! * Register a snapshot as being in use by the current resource owner
*
* If InvalidSnapshot is passed, it is not registered.
*/
*************** RegisterSnapshot(Snapshot snapshot)
*** 357,369 ****
if (snapshot == InvalidSnapshot)
return InvalidSnapshot;
/* Static snapshot? Create a persistent copy */
snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
/* and tell resowner.c about it */
! ResourceOwnerEnlargeSnapshots(CurrentResourceOwner);
snap->regd_count++;
! ResourceOwnerRememberSnapshot(CurrentResourceOwner, snap);
RegisteredSnapshots++;
--- 358,385 ----
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;
+
/* Static snapshot? Create a persistent copy */
snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
/* and tell resowner.c about it */
! ResourceOwnerEnlargeSnapshots(owner);
snap->regd_count++;
! ResourceOwnerRememberSnapshot(owner, snap);
RegisteredSnapshots++;
*************** UnregisterSnapshot(Snapshot snapshot)
*** 383,392 ****
if (snapshot == NULL)
return;
Assert(snapshot->regd_count > 0);
Assert(RegisteredSnapshots > 0);
! ResourceOwnerForgetSnapshot(CurrentResourceOwner, snapshot);
RegisteredSnapshots--;
if (--snapshot->regd_count == 0 && snapshot->active_count == 0)
{
--- 399,421 ----
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;
+
Assert(snapshot->regd_count > 0);
Assert(RegisteredSnapshots > 0);
! ResourceOwnerForgetSnapshot(owner, snapshot);
RegisteredSnapshots--;
if (--snapshot->regd_count == 0 && snapshot->active_count == 0)
{
*************** AtSubAbort_Snapshot(int level)
*** 464,469 ****
--- 493,518 ----
}
/*
+ * 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
*/
*************** AtEOXact_Snapshot(bool isCommit)
*** 475,487 ****
{
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);
--- 524,529 ----
Index: src/include/utils/snapmgr.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/snapmgr.h,v
retrieving revision 1.2
diff -c -p -r1.2 snapmgr.h
*** src/include/utils/snapmgr.h 12 May 2008 20:02:02 -0000 1.2
--- src/include/utils/snapmgr.h 3 Dec 2008 19:31:06 -0000
***************
*** 13,18 ****
--- 13,19 ----
#ifndef SNAPMGR_H
#define SNAPMGR_H
+ #include "utils/resowner.h"
#include "utils/snapshot.h"
*************** extern bool ActiveSnapshotSet(void);
*** 34,42 ****
--- 35,46 ----
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 */
Index: src/test/regress/input/largeobject.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/input/largeobject.source,v
retrieving revision 1.4
diff -c -p -r1.4 largeobject.source
*** src/test/regress/input/largeobject.source 3 Mar 2007 22:57:03 -0000 1.4
--- src/test/regress/input/largeobject.source 2 Dec 2008 14:27:26 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 83,88 ****
--- 83,93 ----
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));
Index: src/test/regress/output/largeobject.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/output/largeobject.source,v
retrieving revision 1.4
diff -c -p -r1.4 largeobject.source
*** src/test/regress/output/largeobject.source 3 Mar 2007 22:57:04 -0000 1.4
--- src/test/regress/output/largeobject.source 2 Dec 2008 14:42:34 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 116,121 ****
--- 116,130 ----
(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));
Index: src/test/regress/output/largeobject_1.source
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/output/largeobject_1.source,v
retrieving revision 1.1
diff -c -p -r1.1 largeobject_1.source
*** src/test/regress/output/largeobject_1.source 10 Mar 2007 03:42:19 -0000 1.1
--- src/test/regress/output/largeobject_1.source 2 Dec 2008 14:42:51 -0000
*************** SELECT lo_close(fd) FROM lotest_stash_va
*** 116,121 ****
--- 116,130 ----
(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));
On Thu, Dec 4, 2008 at 2:25 AM, Alvaro Herrera
<alvherre@commandprompt.com>wrote:
Yeah, that was plenty silly. Updated patch attached.
Looks good me to, except for this warning:
snapmgr.c: In function 'RegisterSnapshot':
snapmgr.c:356: warning: unused variable 'snap'
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
Pavan Deolasee escribi�:
On Thu, Dec 4, 2008 at 2:25 AM, Alvaro Herrera
<alvherre@commandprompt.com>wrote:Yeah, that was plenty silly. Updated patch attached.
Looks good me to, except for this warning:
Applied. Many thanks for the exhaustive testing.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.