Skip to content

Commit 7b640b0

Browse files
alvherrealvherre
committed
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.
1 parent 30c5253 commit 7b640b0

File tree

7 files changed

+101
-19
lines changed

7 files changed

+101
-19
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.269 2008/11/19 10:34:50 heikki Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.270 2008/12/04 14:51:02 alvherre Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -1667,6 +1667,9 @@ CommitTransaction(void)
16671667
/* Clean up the relation cache */
16681668
AtEOXact_RelationCache(true);
16691669

1670+
/* Clean up the snapshot manager */
1671+
AtEarlyCommit_Snapshot();
1672+
16701673
/*
16711674
* Make catalog changes visible to all backends. This has to happen after
16721675
* relcache references are dropped (see comments for
@@ -1906,6 +1909,9 @@ PrepareTransaction(void)
19061909
/* Clean up the relation cache */
19071910
AtEOXact_RelationCache(true);
19081911

1912+
/* Clean up the snapshot manager */
1913+
AtEarlyCommit_Snapshot();
1914+
19091915
/* notify and flatfiles don't need a postprepare call */
19101916

19111917
PostPrepare_PgStat();

‎src/backend/storage/large_object/inv_api.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
*
2525
*
2626
* IDENTIFICATION
27-
* $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.135 2008/11/02 01:45:28 tgl Exp $
27+
* $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.136 2008/12/04 14:51:02 alvherre Exp $
2828
*
2929
*-------------------------------------------------------------------------
3030
*/
@@ -247,7 +247,13 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt)
247247
}
248248
else if (flags & INV_READ)
249249
{
250-
retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
250+
/*
251+
* We must register the snapshot in TopTransaction's resowner,
252+
* because it must stay alive until the LO is closed rather than until
253+
* the current portal shuts down.
254+
*/
255+
retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(),
256+
TopTransactionResourceOwner);
251257
retval->flags = IFS_RDLOCK;
252258
}
253259
else
@@ -270,8 +276,11 @@ void
270276
inv_close(LargeObjectDesc *obj_desc)
271277
{
272278
Assert(PointerIsValid(obj_desc));
279+
273280
if (obj_desc->snapshot != SnapshotNow)
274-
UnregisterSnapshot(obj_desc->snapshot);
281+
UnregisterSnapshotFromOwner(obj_desc->snapshot,
282+
TopTransactionResourceOwner);
283+
275284
pfree(obj_desc);
276285
}
277286

‎src/backend/utils/time/snapmgr.c

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* Portions Copyright (c) 1994, Regents of the University of California
2020
*
2121
* IDENTIFICATION
22-
* $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.7 2008/11/25 20:28:29 alvherre Exp $
22+
* $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.8 2008/12/04 14:51:02 alvherre Exp $
2323
*
2424
*-------------------------------------------------------------------------
2525
*/
@@ -136,7 +136,8 @@ GetTransactionSnapshot(void)
136136
*/
137137
if (IsXactIsoLevelSerializable)
138138
{
139-
CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
139+
CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
140+
TopTransactionResourceOwner);
140141
registered_serializable = true;
141142
}
142143

@@ -345,14 +346,27 @@ ActiveSnapshotSet(void)
345346

346347
/*
347348
* RegisterSnapshot
348-
* Register a snapshot as being in use
349+
* Register a snapshot as being in use by the current resource owner
349350
*
350351
* If InvalidSnapshot is passed, it is not registered.
351352
*/
352353
Snapshot
353354
RegisterSnapshot(Snapshot snapshot)
354355
{
355-
Snapshot snap;
356+
if (snapshot == InvalidSnapshot)
357+
return InvalidSnapshot;
358+
359+
return RegisterSnapshotOnOwner(snapshot, CurrentResourceOwner);
360+
}
361+
362+
/*
363+
* RegisterSnapshotOnOwner
364+
* As above, but use the specified resource owner
365+
*/
366+
Snapshot
367+
RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
368+
{
369+
Snapshot snap;
356370

357371
if (snapshot == InvalidSnapshot)
358372
return InvalidSnapshot;
@@ -361,9 +375,9 @@ RegisterSnapshot(Snapshot snapshot)
361375
snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
362376

363377
/* and tell resowner.c about it */
364-
ResourceOwnerEnlargeSnapshots(CurrentResourceOwner);
378+
ResourceOwnerEnlargeSnapshots(owner);
365379
snap->regd_count++;
366-
ResourceOwnerRememberSnapshot(CurrentResourceOwner, snap);
380+
ResourceOwnerRememberSnapshot(owner, snap);
367381

368382
RegisteredSnapshots++;
369383

@@ -379,14 +393,27 @@ RegisterSnapshot(Snapshot snapshot)
379393
*/
380394
void
381395
UnregisterSnapshot(Snapshot snapshot)
396+
{
397+
if (snapshot == NULL)
398+
return;
399+
400+
UnregisterSnapshotFromOwner(snapshot, CurrentResourceOwner);
401+
}
402+
403+
/*
404+
* UnregisterSnapshotFromOwner
405+
* As above, but use the specified resource owner
406+
*/
407+
void
408+
UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner)
382409
{
383410
if (snapshot == NULL)
384411
return;
385412

386413
Assert(snapshot->regd_count > 0);
387414
Assert(RegisteredSnapshots > 0);
388415

389-
ResourceOwnerForgetSnapshot(CurrentResourceOwner, snapshot);
416+
ResourceOwnerForgetSnapshot(owner, snapshot);
390417
RegisteredSnapshots--;
391418
if (--snapshot->regd_count == 0 && snapshot->active_count == 0)
392419
{
@@ -463,6 +490,26 @@ AtSubAbort_Snapshot(int level)
463490
SnapshotResetXmin();
464491
}
465492

493+
/*
494+
* AtEarlyCommit_Snapshot
495+
*
496+
* Snapshot manager's cleanup function, to be called on commit, before
497+
* doing resowner.c resource release.
498+
*/
499+
void
500+
AtEarlyCommit_Snapshot(void)
501+
{
502+
/*
503+
* On a serializable transaction we must unregister our private refcount to
504+
* the serializable snapshot.
505+
*/
506+
if (registered_serializable)
507+
UnregisterSnapshotFromOwner(CurrentSnapshot,
508+
TopTransactionResourceOwner);
509+
registered_serializable = false;
510+
511+
}
512+
466513
/*
467514
* AtEOXact_Snapshot
468515
* Snapshot manager's cleanup function for end of transaction
@@ -475,13 +522,6 @@ AtEOXact_Snapshot(bool isCommit)
475522
{
476523
ActiveSnapshotElt *active;
477524

478-
/*
479-
* On a serializable snapshot we must first unregister our private
480-
* refcount to the serializable snapshot.
481-
*/
482-
if (registered_serializable)
483-
UnregisterSnapshot(CurrentSnapshot);
484-
485525
if (RegisteredSnapshots != 0)
486526
elog(WARNING, "%d registered snapshots seem to remain after cleanup",
487527
RegisteredSnapshots);

‎src/include/utils/snapmgr.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.2 2008/05/12 20:02:02 alvherre Exp $
9+
* $PostgreSQL: pgsql/src/include/utils/snapmgr.h,v 1.3 2008/12/04 14:51:02 alvherre Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
1313
#ifndef SNAPMGR_H
1414
#define SNAPMGR_H
1515

16+
#include "utils/resowner.h"
1617
#include "utils/snapshot.h"
1718

1819

@@ -34,9 +35,12 @@ extern bool ActiveSnapshotSet(void);
3435

3536
extern Snapshot RegisterSnapshot(Snapshot snapshot);
3637
extern void UnregisterSnapshot(Snapshot snapshot);
38+
extern Snapshot RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner);
39+
extern void UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner);
3740

3841
extern void AtSubCommit_Snapshot(int level);
3942
extern void AtSubAbort_Snapshot(int level);
43+
extern void AtEarlyCommit_Snapshot(void);
4044
extern void AtEOXact_Snapshot(bool isCommit);
4145

4246
#endif /* SNAPMGR_H */

‎src/test/regress/input/largeobject.source

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ SELECT lo_close(fd) FROM lotest_stash_values;
8383

8484
END;
8585

86+
-- Test resource management
87+
BEGIN;
88+
SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
89+
ABORT;
90+
8691
-- Test truncation.
8792
BEGIN;
8893
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));

‎src/test/regress/output/largeobject.source

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ SELECT lo_close(fd) FROM lotest_stash_values;
116116
(1 row)
117117

118118
END;
119+
-- Test resource management
120+
BEGIN;
121+
SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
122+
lo_open
123+
---------
124+
0
125+
(1 row)
126+
127+
ABORT;
119128
-- Test truncation.
120129
BEGIN;
121130
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));

‎src/test/regress/output/largeobject_1.source

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ SELECT lo_close(fd) FROM lotest_stash_values;
116116
(1 row)
117117

118118
END;
119+
-- Test resource management
120+
BEGIN;
121+
SELECT lo_open(loid, x'40000'::int) from lotest_stash_values;
122+
lo_open
123+
---------
124+
0
125+
(1 row)
126+
127+
ABORT;
119128
-- Test truncation.
120129
BEGIN;
121130
UPDATE lotest_stash_values SET fd=lo_open(loid, CAST(x'20000' | x'40000' AS integer));

0 commit comments

Comments
 (0)