Skip to content

Commit fe455ee

Browse files
tglsfdctglsfdc
committed
Revise ResourceOwner code to avoid accumulating ResourceOwner objects
for every command executed within a transaction. For long transactions this was a significant memory leak. Instead, we can delete a portal's or subtransaction's ResourceOwner immediately, if we physically transfer the information about its locks up to the parent owner. This does not fully solve the leak problem; we need to do something about counting multiple acquisitions of the same lock in order to fix it. But it's a necessary step along the way.
1 parent b662311 commit fe455ee

File tree

4 files changed

+72
-30
lines changed

4 files changed

+72
-30
lines changed

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

Lines changed: 38 additions & 17 deletions
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.178 2004/08/11 04:07:15 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.179 2004/08/25 18:43:42 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -1716,11 +1716,16 @@ CommitTransactionCommand(void)
17161716
/*
17171717
* We were issued a RELEASE command, so we end the current
17181718
* subtransaction and return to the parent transaction.
1719+
*
1720+
* Since RELEASE can exit multiple levels of subtransaction,
1721+
* we must loop here until we get out of all SUBEND'ed levels.
17191722
*/
17201723
case TBLOCK_SUBEND:
1721-
CommitSubTransaction();
1722-
PopTransaction();
1723-
s = CurrentTransactionState; /* changed by pop */
1724+
do {
1725+
CommitSubTransaction();
1726+
PopTransaction();
1727+
s = CurrentTransactionState; /* changed by pop */
1728+
} while (s->blockState == TBLOCK_SUBEND);
17241729
break;
17251730

17261731
/*
@@ -2411,9 +2416,10 @@ void
24112416
ReleaseSavepoint(List *options)
24122417
{
24132418
TransactionState s = CurrentTransactionState;
2414-
TransactionState target = s;
2415-
char *name = NULL;
2419+
TransactionState target,
2420+
xact;
24162421
ListCell *cell;
2422+
char *name = NULL;
24172423

24182424
/*
24192425
* Check valid block state transaction status.
@@ -2462,19 +2468,38 @@ ReleaseSavepoint(List *options)
24622468

24632469
Assert(PointerIsValid(name));
24642470

2465-
while (target != NULL)
2471+
for (target = s; PointerIsValid(target); target = target->parent)
24662472
{
24672473
if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
24682474
break;
2469-
target = target->parent;
24702475
}
24712476

24722477
if (!PointerIsValid(target))
24732478
ereport(ERROR,
24742479
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
24752480
errmsg("no such savepoint")));
24762481

2477-
CommitTransactionToLevel(target->nestingLevel);
2482+
/* disallow crossing savepoint level boundaries */
2483+
if (target->savepointLevel != s->savepointLevel)
2484+
ereport(ERROR,
2485+
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
2486+
errmsg("no such savepoint")));
2487+
2488+
/*
2489+
* Mark "commit pending" all subtransactions up to the target
2490+
* subtransaction. The actual commits will happen when control
2491+
* gets to CommitTransactionCommand.
2492+
*/
2493+
xact = CurrentTransactionState;
2494+
for (;;)
2495+
{
2496+
Assert(xact->blockState == TBLOCK_SUBINPROGRESS);
2497+
xact->blockState = TBLOCK_SUBEND;
2498+
if (xact == target)
2499+
break;
2500+
xact = xact->parent;
2501+
Assert(PointerIsValid(xact));
2502+
}
24782503
}
24792504

24802505
/*
@@ -2969,18 +2994,13 @@ CommitSubTransaction(void)
29692994

29702995
CallXactCallbacks(XACT_EVENT_COMMIT_SUB, s->parent->transactionIdData);
29712996

2972-
/*
2973-
* Note that we just release the resource owner's resources and don't
2974-
* delete it. This is because locks are not actually released here.
2975-
* The owner object continues to exist as a child of its parent owner
2976-
* (namely my parent transaction's resource owner), and the locks
2977-
* effectively become that owner object's responsibility.
2978-
*/
29792997
ResourceOwnerRelease(s->curTransactionOwner,
29802998
RESOURCE_RELEASE_BEFORE_LOCKS,
29812999
true, false);
29823000
AtEOSubXact_Inval(true);
2983-
/* we can skip the LOCKS phase */
3001+
ResourceOwnerRelease(s->curTransactionOwner,
3002+
RESOURCE_RELEASE_LOCKS,
3003+
true, false);
29843004
ResourceOwnerRelease(s->curTransactionOwner,
29853005
RESOURCE_RELEASE_AFTER_LOCKS,
29863006
true, false);
@@ -3003,6 +3023,7 @@ CommitSubTransaction(void)
30033023

30043024
CurrentResourceOwner = s->parent->curTransactionOwner;
30053025
CurTransactionResourceOwner = s->parent->curTransactionOwner;
3026+
ResourceOwnerDelete(s->curTransactionOwner);
30063027
s->curTransactionOwner = NULL;
30073028

30083029
AtSubCommit_Memory();

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.68 2004/08/02 21:42:18 tgl Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.69 2004/08/25 18:43:43 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -354,8 +354,7 @@ PortalDrop(Portal portal, bool isTopCommit)
354354
ResourceOwnerRelease(portal->resowner,
355355
RESOURCE_RELEASE_AFTER_LOCKS,
356356
isCommit, false);
357-
if (!isCommit)
358-
ResourceOwnerDelete(portal->resowner);
357+
ResourceOwnerDelete(portal->resowner);
359358
}
360359
portal->resowner = NULL;
361360

‎src/backend/utils/resowner/README

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
$PostgreSQL: pgsql/src/backend/utils/resowner/README,v 1.2 2004/07/31 00:45:40 tgl Exp $
1+
$PostgreSQL: pgsql/src/backend/utils/resowner/README,v 1.3 2004/08/25 18:43:43 tgl Exp $
22

33
Notes about resource owners
44
---------------------------
@@ -30,8 +30,9 @@ ownership of the acquired resources in that ResourceOwner object.
3030
When a Portal is closed, any remaining resources (typically only locks)
3131
become the responsibility of the current transaction. This is represented
3232
by making the Portal's ResourceOwner a child of the current transaction's
33-
ResourceOwner. Similarly, subtransaction ResourceOwners are children of
34-
their immediate parent.
33+
ResourceOwner. resowner.c automatically transfers the resources to the
34+
parent object when releasing the child. Similarly, subtransaction
35+
ResourceOwners are children of their immediate parent.
3536

3637
We need transaction-related ResourceOwners as well as Portal-related ones
3738
because transactions may initiate operations that require resources (such
@@ -53,6 +54,12 @@ The basic operations on a ResourceOwner are:
5354
* delete a ResourceOwner (including child owner objects); all resources
5455
must have been released beforehand
5556

57+
Locks are handled specially because in non-error situations a lock should
58+
be held until end of transaction, even if it was originally taken by a
59+
subtransaction or portal. Therefore, the "release" operation on a child
60+
ResourceOwner transfers lock ownership to the parent instead of actually
61+
releasing the lock, if isCommit is true.
62+
5663
Currently, ResourceOwners contain direct support for recording ownership
5764
of buffer pins, lmgr locks, and catcache and relcache references. Other
5865
objects can be associated with a ResourceOwner by recording the address of

‎src/backend/utils/resowner/resowner.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.2 2004/07/31 00:45:40 tgl Exp $
17+
* $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.3 2004/08/25 18:43:43 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -277,25 +277,40 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
277277
/* Mark object as holding no locks, just for sanity */
278278
owner->nlocks = 0;
279279
}
280-
else if (!isCommit)
280+
else
281281
{
282282
/*
283283
* Release locks retail. Note that LockRelease will remove
284284
* the lock entry from my list, so I just have to iterate till
285285
* there are none. Also note that if we are committing a
286-
* subtransaction, we do NOT release its locks yet.
286+
* subtransaction, we do NOT release its locks yet, but transfer
287+
* them to the parent.
287288
*
288289
* XXX as above, this is a bit inefficient but probably not worth
289290
* the trouble to optimize more.
290291
*/
292+
Assert(owner->parent != NULL);
291293
while (owner->nlocks > 0)
292294
{
293295
LockIdData *lockid = &owner->locks[owner->nlocks - 1];
294296

295-
LockRelease(lockid->locktag.lockmethodid,
296-
&lockid->locktag,
297-
lockid->xid,
298-
lockid->lockmode);
297+
if (isCommit)
298+
{
299+
ResourceOwnerEnlargeLocks(owner->parent);
300+
ResourceOwnerRememberLock(owner->parent,
301+
&lockid->locktag,
302+
lockid->xid,
303+
lockid->lockmode);
304+
owner->nlocks--;
305+
}
306+
else
307+
{
308+
LockRelease(lockid->locktag.lockmethodid,
309+
&lockid->locktag,
310+
lockid->xid,
311+
lockid->lockmode);
312+
/* LockRelease will have removed the entry from list */
313+
}
299314
}
300315
}
301316
}

0 commit comments

Comments
 (0)