Обсуждение: RFC: listing lock status

I've been working on the TODO list item "Add SHOW command to display locks". The
code is basically finished, but I'd like to make sure the user interface is okay
with everyone before I send it in to -patches (if you're interested, the patch
is attached).

Rather than adding another SHOW command, I think using a table function
is a better idea. That's because the information returned by the lock
listing code will often need to be correlated with other information in
the system catalogs, or sorted/aggregated in various ways (e.g. "show me
the names of all locked relations", or "show me the relation with the most
AccessShareLocks'"). Written as a table function, the lock listing code
itself can be fairly simple, and the DBA can write the necessary SQL
queries to produce the information he needs. It also makes it easier to
parse the lock status information, if you're writing (for example) a
GUI admin tool.

Usage examples:

Basic information returned from function:

nconway=# select * from show_locks();
 relation | database | backendpid |      mode       | isgranted
----------+----------+------------+-----------------+-----------
    16575 |    16689 |      13091 | AccessShareLock | t
      376 |        0 |      13091 | ExclusiveLock   | t

After creating a simple relation and starting 2 transactions, one
of which has acquired the lock and one which is waiting on it:

nconway=# select l.backendpid, l.mode, l.isgranted from show_locks() l,
pg_class c where l.relation = c.oid and c.relname = 'a';

 backendpid |         mode          | isgranted
------------+-----------------------+-----------
      13098 | RowExclusiveLock      | t
      13108 | ShareRowExclusiveLock | f

During a 128 client pgbench run:

pgbench1=# select c.relname, count(l.isgranted) from show_locks() l,
           pg_class c where c.oid = l.relation group by c.relname
           order by count desc;
       relname       | count
---------------------+-------
 accounts            |  1081
 tellers             |   718
 pg_xactlock         |   337
 branches            |   208
 history             |     4
 pg_class            |     3
 __show_locks_result |     1

And so on -- I think you get the idea.

Regarding performance, the only performance-critical aspect of the patch
is the place where we need to acquire the LockMgrLock, to ensure that
we get a consistent view of data from the lock manager's hash tables.
The patch is designed so that this lock is held for as short a period
as possible: the lock is acquired, the data is copied from shared memory
to local memory, the lock is released, and then the data is processed.
Any suggestions on how to optimize performance any further would be
welcome.

Let me know if there are any objections or suggestions for improvement.
In particular, should we provide some pre-defined views that correlate
the show_locks() data with data from the system catalogs? And if so,
which views should be pre-defined?

Also, should locks on special relations (e.g. pg_xactlock) or on
system catalogs be shown?

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

On Thu, Jul 18, 2002 at 02:35:42PM -0400, Neil Conway wrote:
> I've been working on the TODO list item "Add SHOW command to display locks". The
> code is basically finished, but I'd like to make sure the user interface is okay
> with everyone before I send it in to -patches (if you're interested, the patch
> is attached).

Woops, forgot to 'cvs add' a newly created file. (Thanks to Joe Conway
for letting me know.)

A fixed patch is attached.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

Neil Conway wrote:> I've been working on the TODO list item "Add SHOW command to display> locks". The code is basically
finished,but I'd like to make sure the> user interface is okay with everyone before I send it in to -patches> (if
you'reinterested, the patch is attached).>> Rather than adding another SHOW command, I think using a table> function is
abetter idea. That's because the information returned by> the lock listing code will often need to be correlated with
other>information in the system catalogs, or sorted/aggregated in various> ways (e.g. "show me the names of all locked
relations",or "show me> the relation with the most AccessShareLocks'"). Written as a table> function, the lock listing
codeitself can be fairly simple, and the> DBA can write the necessary SQL queries to produce the information he> needs.
Italso makes it easier to parse the lock status information,> if you're writing (for example) a GUI admin tool.
 

I'm undoubtedly biased ;-), but I like your approach. Applies and works 
fine here.
> Also, should locks on special relations (e.g. pg_xactlock) or on> system catalogs be shown?

Maybe the function should take a boolean parameter to indicate whether
or not to show locks on objects in pg_* schema?

Joe

On Thu, Jul 18, 2002 at 03:12:53PM -0700, Joe Conway wrote:
> Neil Conway wrote:
> > Also, should locks on special relations (e.g. pg_xactlock) or on
> > system catalogs be shown?
> 
> Maybe the function should take a boolean parameter to indicate whether
> or not to show locks on objects in pg_* schema?

I had thought about that, but it occurs to me that the DBA can
effectively choose this for himself using the relID and databaseID
returned by the SRF, in combination with pg_database.datlastsysoid.

Cheers,

Neil

-- 
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

>  > Rather than adding another SHOW command, I think using a table
>  > function is a better idea. That's because the information returned by
>  > the lock listing code will often need to be correlated with other
>  > information in the system catalogs, or sorted/aggregated in various
>  > ways (e.g. "show me the names of all locked relations", or "show me
>  > the relation with the most AccessShareLocks'"). Written as a table
>  > function, the lock listing code itself can be fairly simple, and the
>  > DBA can write the necessary SQL queries to produce the information he
>  > needs. It also makes it easier to parse the lock status information,
>  > if you're writing (for example) a GUI admin tool.

Out of interest - why do SRFs need to have a table or view defined that
matches their return type?  Why can't you just create the type for the
function and set it up as a dependency?

Chris

Christopher Kings-Lynne wrote:
> Out of interest - why do SRFs need to have a table or view defined that
> matches their return type?  Why can't you just create the type for the
> function and set it up as a dependency?
> 

The only current way to create a composite type (and hence have it for 
the function to reference) is to define a table or view.

We have discussed the need for a stand-alone composite type, but I think 
Tom favors doing that as part of a larger project, namely changing the 
association of pg_attributes to pg_type instead of pg_class (if I 
understand/remember it correctly).

Joe

Joe Conway <mail@joeconway.com> writes:
> Christopher Kings-Lynne wrote:
>> Out of interest - why do SRFs need to have a table or view defined that
>> matches their return type?  Why can't you just create the type for the
>> function and set it up as a dependency?

> The only current way to create a composite type (and hence have it for 
> the function to reference) is to define a table or view.

> We have discussed the need for a stand-alone composite type, but I think 
> Tom favors doing that as part of a larger project, namely changing the 
> association of pg_attributes to pg_type instead of pg_class (if I 
> understand/remember it correctly).

Well, it's not an optional larger project: there just isn't any way ATM
to define a composite type that's not linked to a pg_class entry.  The
only way to show fields of a composite type is through pg_attribute
entries, and pg_attribute entries are bound to pg_class entries not
pg_type entries.

The clean way to restructure this would be to link pg_attribute entries
to pg_type not pg_class.  But that would break approximately every
client that looks at the system catalogs.

An alternative that just now occurred to me is to invent a new "dummy"
relkind for a pg_class entry that isn't a real relation, but merely a
front for a composite type in pg_type.  Not sure of all the
implications, but it might be worth pursuing.
        regards, tom lane

nconway@klamath.dyndns.org (Neil Conway) writes:
> On Thu, Jul 18, 2002 at 03:12:53PM -0700, Joe Conway wrote:
>> Maybe the function should take a boolean parameter to indicate whether
>> or not to show locks on objects in pg_* schema?

> I had thought about that, but it occurs to me that the DBA can
> effectively choose this for himself using the relID and databaseID
> returned by the SRF, in combination with pg_database.datlastsysoid.

datlastsysoid is obsolete IMHO --- it was never trustworthy when one
considers the possibility of OID wraparound.

My opinion on this point is (a) pgxactlock locks are special and should
be shown specially --- in the form of "xact a waits for xact b";
(b) locks on other system catalogs are normal locks and should NOT be
discriminated against.  If you have a deadlock condition, the fact that
one of the elements of the lock cycle is on a system catalog isn't going
to magically get you out of the deadlock; nor can you avoid waiting just
because the lock you need is on a system catalog.  Since AFAICS the
only value of a lock status displayer is to investigate problems of one
of those two forms, I can fathom no reason at all that anyone would have
the slightest use for a displayer that arbitrarily omits some locks.
        regards, tom lane

Tom Lane wrote:
> Well, it's not an optional larger project: there just isn't any way ATM
> to define a composite type that's not linked to a pg_class entry.  The
> only way to show fields of a composite type is through pg_attribute
> entries, and pg_attribute entries are bound to pg_class entries not
> pg_type entries.
> 
> The clean way to restructure this would be to link pg_attribute entries
> to pg_type not pg_class.  But that would break approximately every
> client that looks at the system catalogs.
> 
> An alternative that just now occurred to me is to invent a new "dummy"
> relkind for a pg_class entry that isn't a real relation, but merely a
> front for a composite type in pg_type.  Not sure of all the
> implications, but it might be worth pursuing.
> 

I was originally thinking the same thing, but I guess I didn't think it 
would fly. Could we steal the needed parts from CREATE and DROP VIEW, 
except make a new relkind 'f' and skip the RULEs? Something like:

CREATE TYPE typename AS ( column_name data_type [, ... ])

FWIW, you can see an example of Oracle's CREATE TYPE here:
http://download-west.oracle.com/otndoc/oracle9i/901_doc/appdev.901/a89856/08_subs.htm#19677

And perhaps we could do:

CREATE [ OR REPLACE ] FUNCTION name ( [ argtype [, ...] ] )
RETURNS [setof] { data_type | (column_name data_type [, ... ]) } . . .

to automatically create a composite type with a system generated name 
for a function. Someone reported a similar syntax for InterBase here:
http://archives.postgresql.org/pgsql-sql/2002-07/msg00011.php

Thoughts?

Joe

On Thu, Jul 18, 2002 at 11:30:46PM -0400, Tom Lane wrote:
> My opinion on this point is (a) pgxactlock locks are special and should
> be shown specially --- in the form of "xact a waits for xact b";

Not sure how that would fit into a UI based on returning sets of tuples.

> I can fathom no reason at all that anyone would have
> the slightest use for a displayer that arbitrarily omits some locks.

I agree. I think a reasonable solution is to have the low-level SRF
return data on both pg_xactlock locks and locks on system catalogs.
If the DBA wants to disregard one or the other, it should be pretty
easy to do (particularly pg_xactlock).

Cheers,

Neil

-- 
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC