From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Sergei Kornilov <sk@zsrv.org>
Cc: Guillaume Lelarge <guillaume@lelarge.info>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Expose lock group leader pid in pg_stat_activity
Date: Tue, 28 Jan 2020 14:09:10 +0100
Message-ID: <20200128130910.anay3mlhaminob6w@development> (raw)
In-Reply-To: <CAOBaU_Z6WcT8FJfcCMU_Bp+PTzUpTEY9BfBQE4gbGKpE5wpfwQ@mail.gmail.com>
References: <CAOBaU_b0Zh7Sdt-FHa0Zg5P3R1sHirLE4Lb4rWd84fJn=RK+RA@mail.gmail.com>
<5153051577359132@sas8-b090c2642e35.qloud-c.yandex.net>
<CAOBaU_aFdiM=4w-UXtLfKgmcC7gwTQnJFWuxFA0CnPe9wgTnjg@mail.gmail.com>
<2519941577437281@vla3-985002032993.qloud-c.yandex.net>
<CAOBaU_Yaj8Zv-jXQu5VoowSf4kkTLGvFDg6ZZyfireBSVgooNQ@mail.gmail.com>
<20200116072727.GI3117@paquier.xyz>
<20200116074912.GA98418@paquier.xyz>
<CAOBaU_ZPHkriFVYF=OrFyLsrXzcdzPizr0SPh+QdNBsHjT2GmQ@mail.gmail.com>
<20200118025102.GB230692@paquier.xyz>
<CAOBaU_Z6WcT8FJfcCMU_Bp+PTzUpTEY9BfBQE4gbGKpE5wpfwQ@mail.gmail.com>
On Tue, Jan 28, 2020 at 12:36:41PM +0100, Julien Rouhaud wrote:
>On Sat, Jan 18, 2020 at 3:51 AM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Fri, Jan 17, 2020 at 05:07:55PM +0100, Julien Rouhaud wrote:
>> > Oh indeed. But unless we hold some LWLock during the whole function
>> > execution, we cannot guarantee a consistent view right?
>>
>> Yep. That's possible.
>>
>> > And isn't it already possible to e.g. see a parallel worker in
>> > pg_stat_activity while all other queries are shown are idle, if
>> > you're unlucky enough?
>>
>> Yep. That's possible.
>>
>> > Also, LockHashPartitionLockByProc requires the leader PGPROC, and
>> > there's no guarantee that we'll see the leader before any of the
>> > workers, so I'm unsure how to implement what you said. Wouldn't it be
>> > better to simply fetch the leader PGPROC after acquiring a shared
>> > ProcArrayLock, and using that copy to display the pid, after checking
>> > that we retrieved a non-null PGPROC?
>>
>> Another idea would be to check if the current PGPROC entry is a leader
>> and print in an int[] the list of PIDs of all the workers while
>> holding a shared LWLock to avoid anything to be unregistered. Less
>> handy, but a bit more consistent. One problem with doing that is
>> that you may have in your list of PIDs some worker processes that are
>> already gone when going through all the other backend entries. An
>> advantage is that an empty array could mean "I am the leader, but
>> nothing has been registered yet to my group lock." (note that the
>> leader adds itself to lockGroupMembers).
>
>So, AFAICT the LockHashPartitionLockByProc is required when
>iterating/modifying lockGroupMembers or lockGroupLink, but just
>getting the leader pid should be safe. Since we'll never be able to
>get a totally consistent view of data here, I'm in favor of avoiding
>taking extra locks here. I agree that outputting an array of the pid
>would be more consistent for the leader, but will have its own set of
>corner cases. It seems to me that a new leader_pid column is easier
>to handle at SQL level, so I kept that approach in attached v4. If
>you have strong objections to it. I can still change it.
I agree a separate "leader_id" column is easier to work with, as it does
not require unnesting and so on.
As for the consistency, I agree we probably can't make this perfect, as
we're fetching and processing the PGPROC records one by one. Fixing that
would require acquiring a much stronger lock on PGPROC, and perhaps some
other locks. That's pre-existing behavior, of course, it's just not very
obvious as we don't have any dependencies between the rows, I think.
Adding the leader_id will change, that, of course. But I think it's
still mostly OK, even with the possible inconsistency.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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: tomas.vondra@2ndquadrant.com, rjuju123@gmail.com, michael@paquier.xyz, sk@zsrv.org, guillaume@lelarge.info, pgsql-hackers@lists.postgresql.org
Subject: Re: Expose lock group leader pid in pg_stat_activity
In-Reply-To: <20200128130910.anay3mlhaminob6w@development>
* 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