Обсуждение: How to expose session vs txn lock info in pg_locks view?
Presently there doesn't seem to be a way to tell whether a lock is session-level or transaction-level in the pg_locks view.
I was expecting this to be a quick patch, but the comment on the definition of PROCLOCKTAG in lock.h notes that shmem state for heavyweight locks does not track whether the lock is session-level or txn-level. That explains why it's not already exposed in pg_locks.
AFAICS it'd be necessary to expand PROCLOG to expose this in shmem. Probably by adding a small bitfield where bit 0 is set if there's a txn level lock and bit 1 is set if there's a session level lock. But I'm not convinced that expanding PROCLOCK is justifiable for this. sizeof(PROCLOCK) is 64 on a typical x64 machine. Adding anything to it increases it to 72 bytes.
(gdb) ptype /o struct PROCLOCK
/* offset | size */ type = struct PROCLOCK {
/* 0 | 16 */ PROCLOCKTAG tag;
/* 16 | 8 */ PGPROC *groupLeader;
/* 24 | 4 */ LOCKMASK holdMask;
/* 28 | 4 */ LOCKMASK releaseMask;
/* 32 | 16 */ SHM_QUEUE lockLink;
/* 48 | 16 */ SHM_QUEUE procLink;
/* 64 | 1 */ unsigned char locktypes;
/* XXX 7-byte padding */
/* total size (bytes): 72 */
}
/* offset | size */ type = struct PROCLOCK {
/* 0 | 16 */ PROCLOCKTAG tag;
/* 16 | 8 */ PGPROC *groupLeader;
/* 24 | 4 */ LOCKMASK holdMask;
/* 28 | 4 */ LOCKMASK releaseMask;
/* 32 | 16 */ SHM_QUEUE lockLink;
/* 48 | 16 */ SHM_QUEUE procLink;
/* 64 | 1 */ unsigned char locktypes;
/* XXX 7-byte padding */
/* total size (bytes): 72 */
}
Going over 64 sets off possible alarm bells about cache line sizing to me, but maybe it's not that critical? It'd also require (8 * max_locks_per_xact * (MaxBackends+max_prepared_xacts)) extra shmem space; that could land up being 128k on a default setup and a couple of megabytes on a big system. Not huge, but not insignificant if it's hot data.
It's frustrating to be unable to tell the difference between session-level and txn-level locks in diagnostic output. And the deadlock detector has no way to tell the difference when selecting a victim for a deadlock abort - it'd probably make sense to prefer to send a deadlock abort for txn-only lockers. But I'm not sure I see a sensible way to add the info - PROCLOCK is already free of any padding, and I wouldn't want to use hacks like pointer-tagging.
Thoughts anyone?
Hi, On 2021-01-19 14:16:07 +0800, Craig Ringer wrote: > AFAICS it'd be necessary to expand PROCLOG to expose this in shmem. > Probably by adding a small bitfield where bit 0 is set if there's a txn > level lock and bit 1 is set if there's a session level lock. But I'm not > convinced that expanding PROCLOCK is justifiable for this. sizeof(PROCLOCK) > is 64 on a typical x64 machine. Adding anything to it increases it to 72 > bytes. Indeed - I really don't want to increase the size, it's already a problem. > It's frustrating to be unable to tell the difference between session-level > and txn-level locks in diagnostic output. It'd be useful, I agree. > And the deadlock detector has no way to tell the difference when > selecting a victim for a deadlock abort - it'd probably make sense to > prefer to send a deadlock abort for txn-only lockers. I'm doubtful this is worth going for. > But I'm not sure I see a sensible way to add the info - PROCLOCK is > already free of any padding, and I wouldn't want to use hacks like > pointer-tagging. I think there's an easy way to squeeze out space: make groupLeader be an integer index into allProcs instead. That requires only 4 bytes... Alternatively, I think it'd be reasonably easy to add the scope as a bit in LOCKMASK - there's plenty space. Greetings, Andres Freund
On Sun, 24 Jan 2021 at 09:12, Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2021-01-19 14:16:07 +0800, Craig Ringer wrote:
> AFAICS it'd be necessary to expand PROCLOG to expose this in shmem.
> Probably by adding a small bitfield where bit 0 is set if there's a txn
> level lock and bit 1 is set if there's a session level lock. But I'm not
> convinced that expanding PROCLOCK is justifiable for this. sizeof(PROCLOCK)
> is 64 on a typical x64 machine. Adding anything to it increases it to 72
> bytes.
Indeed - I really don't want to increase the size, it's already a
problem.
> It's frustrating to be unable to tell the difference between session-level
> and txn-level locks in diagnostic output.
It'd be useful, I agree.
> And the deadlock detector has no way to tell the difference when
> selecting a victim for a deadlock abort - it'd probably make sense to
> prefer to send a deadlock abort for txn-only lockers.
I'm doubtful this is worth going for.
> But I'm not sure I see a sensible way to add the info - PROCLOCK is
> already free of any padding, and I wouldn't want to use hacks like
> pointer-tagging.
I think there's an easy way to squeeze out space: make groupLeader be an
integer index into allProcs instead. That requires only 4 bytes...
Alternatively, I think it'd be reasonably easy to add the scope as a bit
in LOCKMASK - there's plenty space.
I was wondering about that, but concerned that there would be impacts I did not understand.
I'm happy to pursue that angle.
On Mon, 1 Feb 2021 at 18:42, Craig Ringer <craig.ringer@enterprisedb.com> wrote:
On Sun, 24 Jan 2021 at 09:12, Andres Freund <andres@anarazel.de> wrote:Hi,
On 2021-01-19 14:16:07 +0800, Craig Ringer wrote:
> AFAICS it'd be necessary to expand PROCLOG to expose this in shmem.
> Probably by adding a small bitfield where bit 0 is set if there's a txn
> level lock and bit 1 is set if there's a session level lock. But I'm not
> convinced that expanding PROCLOCK is justifiable for this. sizeof(PROCLOCK)
> is 64 on a typical x64 machine. Adding anything to it increases it to 72
> bytes.
Indeed - I really don't want to increase the size, it's already a
problem.
> It's frustrating to be unable to tell the difference between session-level
> and txn-level locks in diagnostic output.
It'd be useful, I agree.
> And the deadlock detector has no way to tell the difference when
> selecting a victim for a deadlock abort - it'd probably make sense to
> prefer to send a deadlock abort for txn-only lockers.
I'm doubtful this is worth going for.
> But I'm not sure I see a sensible way to add the info - PROCLOCK is
> already free of any padding, and I wouldn't want to use hacks like
> pointer-tagging.
I think there's an easy way to squeeze out space: make groupLeader be an
integer index into allProcs instead. That requires only 4 bytes...
Alternatively, I think it'd be reasonably easy to add the scope as a bit
in LOCKMASK - there's plenty space.I was wondering about that, but concerned that there would be impacts I did not understand.I'm happy to pursue that angle.
Just so this thread isn't left dangling, I'm just not going to get time to follow up on this work with a concrete patch and test suite change.
If anyone else later on wants to differentiate between session and txn LWLocks they could start with the approach proposed here.