Обсуждение: Wait event names mismatch: oldserxid

Hi,
Name for wait event "LWTRANCHE_OLDSERXID_BUFFERS" is printed as
"oldserxid", but documentation at
https://www.postgresql.org/docs/10/static/monitoring-stats.html does
not have exact same event there. Instead it has

OldSerXidLock     Waiting to read or record conflicting serializable
transactions.

Are they same. In that case, should we change the documentation or
code in OldSerXidInit()
 808     SimpleLruInit(OldSerXidSlruCtl, "oldserxid",
 809                   NUM_OLDSERXID_BUFFERS, 0, OldSerXidLock, "pg_serial",
 810                   LWTRANCHE_OLDSERXID_BUFFERS);

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Fri, Feb 09, 2018 at 06:04:39PM +0530, Ashutosh Bapat wrote:
> Name for wait event "LWTRANCHE_OLDSERXID_BUFFERS" is printed as
> "oldserxid", but documentation at
> https://www.postgresql.org/docs/10/static/monitoring-stats.html does
> not have exact same event there. Instead it has
>
> OldSerXidLock     Waiting to read or record conflicting serializable
> transactions.

I see two events defined here in the code of type LWLock dedicated to
oldserxid:
- OldSerXidLock which is a wait event defined as it is part of
LWLockNames.
- oldserxid, which gets defined in SimpleLruInit(), which itself calls
LWLockRegisterTranche() to define a second event of type LWLock.

So the docs look correct to me on this side.  What I find weird is the
phrasing to define oldserxid.  Instead of that, the current description:
Waiting to I/O on an oldserxid buffer.
I would suggest "waiting *for* I/O"

A small patch is attached.

Thanks,
--
Michael

On Fri, Feb 9, 2018 at 7:23 PM, Michael Paquier <michael@paquier.xyz> wrote:
> On Fri, Feb 09, 2018 at 06:04:39PM +0530, Ashutosh Bapat wrote:
>> Name for wait event "LWTRANCHE_OLDSERXID_BUFFERS" is printed as
>> "oldserxid", but documentation at
>> https://www.postgresql.org/docs/10/static/monitoring-stats.html does
>> not have exact same event there. Instead it has
>>
>> OldSerXidLock     Waiting to read or record conflicting serializable
>> transactions.
>
> I see two events defined here in the code of type LWLock dedicated to
> oldserxid:
> - OldSerXidLock which is a wait event defined as it is part of
> LWLockNames.
> - oldserxid, which gets defined in SimpleLruInit(), which itself calls
> LWLockRegisterTranche() to define a second event of type LWLock.

I didn't pay attention to the second one. Thanks for pointing that
out. But then like me a user may first land on OldSerXidLock since
that is first in the list and get confused. May be we should use names
which are not prefix of others.

>
> So the docs look correct to me on this side.  What I find weird is the
> phrasing to define oldserxid.  Instead of that, the current description:
> Waiting to I/O on an oldserxid buffer.
> I would suggest "waiting *for* I/O"
>

+1.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Fri, Feb 09, 2018 at 07:33:57PM +0530, Ashutosh Bapat wrote:
> I didn't pay attention to the second one. Thanks for pointing that
> out. But then like me a user may first land on OldSerXidLock since
> that is first in the list and get confused. May be we should use names
> which are not prefix of others.

Yeah, I don't think that it would be good to have users abuse of things
like ~ 'oldserxid' to track different event types.  This encourages such
filters on pg_stat_activity.
--
Michael

On Fri, Feb 9, 2018 at 8:53 AM, Michael Paquier <michael@paquier.xyz> wrote:
> So the docs look correct to me on this side.  What I find weird is the
> phrasing to define oldserxid.  Instead of that, the current description:
> Waiting to I/O on an oldserxid buffer.
> I would suggest "waiting *for* I/O"
>
> A small patch is attached.

Agreed.  Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Feb 27, 2018 at 02:44:37PM -0500, Robert Haas wrote:
> On Fri, Feb 9, 2018 at 8:53 AM, Michael Paquier <michael@paquier.xyz> wrote:
>> So the docs look correct to me on this side.  What I find weird is the
>> phrasing to define oldserxid.  Instead of that, the current description:
>> Waiting to I/O on an oldserxid buffer.
>> I would suggest "waiting *for* I/O"
>>
>> A small patch is attached.
>
> Agreed.  Committed.

Thanks, Robert.
--
Michael