Re: adding wait_start column to pg_locks

On 2021-01-21 12:48, Fujii Masao wrote:

> Thanks for updating the patch! I think that this is really useful
> feature!!

Thanks for reviewing!

> I have two minor comments.
>
> + <entry role="catalog_table_entry"><para
> role="column_definition">
> + <structfield>wait_start</structfield> <type>timestamptz</type>
>
> The column name "wait_start" should be "waitstart" for the sake of
> consistency
> with other column names in pg_locks? pg_locks seems to avoid including
> an underscore in column names, so "locktype" is used instead of
> "lock_type",
> "virtualtransaction" is used instead of "virtual_transaction", etc.
>
> + Lock acquisition wait start time. <literal>NULL</literal> if
> + lock acquired.
>

Agreed.

I also changed the variable name "wait_start" in struct PGPROC and
LockInstanceData to "waitStart" for the same reason.

> There seems the case where the wait start time is NULL even when
> "grant"
> is false. It's better to add note about that case into the docs? For
> example,
> I found that the wait start time is NULL while the startup process is
> waiting
> for the lock. Is this only that case?

Thanks, this is because I set 'waitstart' in the following
condition.

---src/backend/storage/lmgr/proc.c
> 1250 if (!InHotStandby)

As far as considering this, I guess startup process would
be the only case.

I also think that in case of startup process, it seems possible
to set 'waitstart' in ResolveRecoveryConflictWithLock(), so I
did it in the attached patch.

Any thoughts?

Regards,

--
Atsushi Torikoshi