pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: torikoshia <torikoshia@oss.nttdata.com>
Cc: Ian Lawrence Barwick <barwick@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: adding wait_start column to pg_locks
Date: Wed, 3 Feb 2021 11:23:37 +0900
Message-ID: <3db333a9-31f7-a225-8038-eeb835ba8f37@oss.nttdata.com> (raw)
In-Reply-To: <ef0dd522-9096-a60f-a0fa-5f0a0fdbafbd@oss.nttdata.com>
References: <a96013dc51cdc56b2a2b84fa8a16a993@oss.nttdata.com>
	<20210101214930.GH25152@telsasoft.com>
	<9e958478d8bcea814e7ce16511a18912@oss.nttdata.com>
	<CAB8KJ=hN=sz2+u3cQ+5jhZT9+TP-N7OCBLTp68L4fkVoVTqWjw@mail.gmail.com>
	<CA+TgmoYowQhMhT74AsxDib6e3LFPGvXVxVOUdP0-hMNY9c1wgw@mail.gmail.com>
	<CAB8KJ=idS0m_+65d2ujjjZj5DSwpYYV3XoHMwr-UZ-OFs3Mb4Q@mail.gmail.com>
	<23d39ee9c31643fad8f9ba9c5cf3aaf4@oss.nttdata.com>
	<0fd375a53e306566cea7f451cc8cfcde@oss.nttdata.com>
	<7b1bb07a-73ec-1fcc-2f65-41101adf0080@oss.nttdata.com>
	<a5bfd1c2f2fb21eb7ea5ddec2e094430@oss.nttdata.com>
	<8002219d-b999-12fa-2327-49afe40e5fbb@oss.nttdata.com>
	<88c451c7-2729-4329-fbae-6ed7664adea1@oss.nttdata.com>
	<f9153182845e5584b954ab6f03514d13@oss.nttdata.com>
	<ef0dd522-9096-a60f-a0fa-5f0a0fdbafbd@oss.nttdata.com>



On 2021/02/03 1:49, Fujii Masao wrote:
> 
> 
> On 2021/02/02 22:00, torikoshia wrote:
>> On 2021-01-25 23:44, Fujii Masao wrote:
>>> Another comment is; Doesn't the change of MyProc->waitStart need the
>>> lock table's partition lock? If yes, we can do that by moving
>>> LWLockRelease(partitionLock) just after the change of
>>> MyProc->waitStart, but which causes the time that lwlock is being held
>>> to be long. So maybe we need another way to do that.
>>
>> Thanks for your comments!
>>
>> It would be ideal for the consistency of the view to record "waitstart" during holding the table partition lock.
>> However, as you pointed out, it would give non-negligible performance impacts.
>>
>> I may miss something, but as far as I can see, the influence of not holding the lock is that "waitstart" can be NULL even though "granted" is false.
>>
>> I think people want to know the start time of the lock when locks are held for a long time.
>> In that case, "waitstart" should have already been recorded.
> 
> Sounds reasonable.
> 
> 
>> If this is true, I think the current implementation may be enough on the condition that users understand it can happen that "waitStart" is NULL and "granted" is false.
>>
>> Attached a patch describing this in the doc and comments.
>>
>>
>> Any Thoughts?
> 
> 64-bit fetches are not atomic on some platforms. So spinlock is necessary when updating "waitStart" without holding the partition lock? Also GetLockStatusData() needs spinlock when reading "waitStart"?

Also it might be worth thinking to use 64-bit atomic operations like pg_atomic_read_u64(), for that.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION





view thread (27+ messages)  latest in thread

Message-ID: <3db333a9-31f7-a225-8038-eeb835ba8f37@oss.nttdata.com>
Permalink:  ../3db333a9-31f7-a225-8038-eeb835ba8f37@oss.nttdata.com/
Also on:    postgresql.org/message-id/3db333a9-31f7-a225-8038-eeb835ba8f37@oss.nttdata.com

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: masao.fujii@oss.nttdata.com, torikoshia@oss.nttdata.com, barwick@gmail.com, robertmhaas@gmail.com, pryzby@telsasoft.com
  Subject: Re: adding wait_start column to pg_locks
  In-Reply-To: <3db333a9-31f7-a225-8038-eeb835ba8f37@oss.nttdata.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox