Stephen Frost <sfr...@snowman.net> writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> I think the proposed fix is fine code-wise; the real problem here is
>> crummy commenting. GetRunningTransactionLocks isn't documented as
>> returning a palloc'd array, and why the heck do we have a long comment
>> about its implementation in LogStandbySnapshot?
> Certainly good questions and better comments would have helped here. I
> can go back and rework the patch either way.
I'm already working on back-patching the attached.
regards, tom lane
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 615278b8ca2adf3057e5f21057c3cedfc66480aa..c704412366d5bc4b6512e9ab673855c46f7d993f 100644
*** a/src/backend/storage/ipc/standby.c
--- b/src/backend/storage/ipc/standby.c
*************** LogStandbySnapshot(void)
*** 865,880 ****
/*
* Get details of any AccessExclusiveLocks being held at the moment.
- *
- * XXX GetRunningTransactionLocks() currently holds a lock on all
- * partitions though it is possible to further optimise the locking. By
- * reference counting locks and storing the value on the ProcArray entry
- * for each backend we can easily tell if any locks need recording without
- * trying to acquire the partition locks and scanning the lock table.
*/
locks = GetRunningTransactionLocks(&nlocks);
if (nlocks > 0)
LogAccessExclusiveLocks(nlocks, locks);
/*
* Log details of all in-progress transactions. This should be the last
--- 865,875 ----
/*
* Get details of any AccessExclusiveLocks being held at the moment.
*/
locks = GetRunningTransactionLocks(&nlocks);
if (nlocks > 0)
LogAccessExclusiveLocks(nlocks, locks);
+ pfree(locks);
/*
* Log details of all in-progress transactions. This should be the last
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 8cd871f4b40d7bfc8b0aaa7650241d5865c79ffe..273c72230274b46ba6a46bb8b295ef5375aaa36d 100644
*** a/src/backend/storage/lmgr/lock.c
--- b/src/backend/storage/lmgr/lock.c
*************** GetLockStatusData(void)
*** 3398,3415 ****
}
/*
! * Returns a list of currently held AccessExclusiveLocks, for use
! * by GetRunningTransactionData().
*/
xl_standby_lock *
GetRunningTransactionLocks(int *nlocks)
{
PROCLOCK *proclock;
HASH_SEQ_STATUS seqstat;
int i;
int index;
int els;
- xl_standby_lock *accessExclusiveLocks;
/*
* Acquire lock on the entire shared lock data structure.
--- 3398,3423 ----
}
/*
! * Returns a list of currently held AccessExclusiveLocks, for use by
! * LogStandbySnapshot(). The result is a palloc'd array,
! * with the number of elements returned into *nlocks.
! *
! * XXX This currently takes a lock on all partitions of the lock table,
! * but it's possible to do better. By reference counting locks and storing
! * the value in the ProcArray entry for each backend we could tell if any
! * locks need recording without having to acquire the partition locks and
! * scan the lock table. Whether that's worth the additional overhead
! * is pretty dubious though.
*/
xl_standby_lock *
GetRunningTransactionLocks(int *nlocks)
{
+ xl_standby_lock *accessExclusiveLocks;
PROCLOCK *proclock;
HASH_SEQ_STATUS seqstat;
int i;
int index;
int els;
/*
* Acquire lock on the entire shared lock data structure.
*************** GetRunningTransactionLocks(int *nlocks)
*** 3467,3472 ****
--- 3475,3482 ----
}
}
+ Assert(index <= els);
+
/*
* And release locks. We do this in reverse order for two reasons: (1)
* Anyone else who needs more than one of the locks will be trying to lock
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
