Re: pg_locks documentation vs. SSI

While taking a look at the existing documentation for pg_locks I came
across the following paragraph:

<para>
When the <structname>pg_locks</structname> view is accessed, the
internal lock manager data structures are momentarily locked, and
a copy is made for the view to display. This ensures that the
view produces a consistent set of results, while not blocking
normal lock manager operations longer than necessary. Nonetheless
there could be some impact on database performance if this view is
frequently accessed.
</para>

AFAICS, this is no longer quite true. The view of the data from the
main lock manager will be self-consistent, and the view of the data
from the predicate lock manager will be self-consistent, but they need
not be consistent with each other. I don't think that's a problem we
need to fix, but we probably ought to make the documentation match the
behavior.

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

Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> While taking a look at the existing documentation for pg_locks I
> came across the following paragraph:
>
> <para>
> When the <structname>pg_locks</structname> view is accessed,
> the internal lock manager data structures are momentarily
> locked, and a copy is made for the view to display. This
> ensures that the view produces a consistent set of results,
> while not blocking normal lock manager operations longer than
> necessary. Nonetheless there could be some impact on database
> performance if this view is frequently accessed.
> </para>
>
> AFAICS, this is no longer quite true. The view of the data from
> the main lock manager will be self-consistent, and the view of the
> data from the predicate lock manager will be self-consistent, but
> they need not be consistent with each other. I don't think that's
> a problem we need to fix, but we probably ought to make the
> documentation match the behavior.

It remains true that the heavyweight locking structures are
momentarily locked to capture a consistent view of those, and it is
also true that the predicate locking structures are momentarily
locked to get a consistent view of that data. Both are not covered
by some over-arching lock, but that's true at *all* times --
SIReadLock entries can disappear mid-transaction (for READ ONLY
transactions) and can persist past transaction completion (if there
are overlapping transactions with which the completed transaction
can still form a dangerous structure). So there is never a very
close tie between the timing of the appearance or disappearance for
SIRead locks versus any heavyweight locks.

That is covered to some degree in the section on serializable
transactions:

http://developer.postgresql.org/pgdocs/postgres/transaction-iso.html#XACT-SERIALIZABLE

Any thoughts on what else the docs need to be more clear?

-Kevin

On Fri, Jun 24, 2011 at 12:27 PM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> While taking a look at the existing documentation for pg_locks I
>> came across the following paragraph:
>>
>>   <para>
>>    When the <structname>pg_locks</structname> view is accessed,
>>    the internal lock manager data structures are momentarily
>>    locked, and a copy is made for the view to display.  This
>>    ensures that the view produces a consistent set of results,
>>    while not blocking normal lock manager operations longer than
>>    necessary. Nonetheless there could be some impact on database
>>    performance if this view is frequently accessed.
>>   </para>
>>
>> AFAICS, this is no longer quite true.  The view of the data from
>> the main lock manager will be self-consistent, and the view of the
>> data from the predicate lock manager will be self-consistent, but
>> they need not be consistent with each other.  I don't think that's
>> a problem we need to fix, but we probably ought to make the
>> documentation match the behavior.
>
> It remains true that the heavyweight locking structures are
> momentarily locked to capture a consistent view of those, and it is
> also true that the predicate locking structures are momentarily
> locked to get a consistent view of that data. Both are not covered
> by some over-arching lock, but...

...but they could be, if we were so inclined. We could acquire all of
the lock manager partition locks, all of the predicate lock manager
partition locks, and the lock on SerializableXactHashLock - then dump
all the lock state from both tables - then release all the locks.
What we actually do is acquire all of the lock manager locks, snapshot
the state, release those locks, then get the predicate lock manager
locks and SerializableXactHashLock, snapshot the state over there,
release those locks, and then dump everything out. Since we don't do
that, it's possible that "select * from pg_locks" could see a state
that never really existed.

For example, it's possible that, after doing GetLockStatusData() but
before doing GetPredicateLockStatusData(), some backend might commit a
transaction, releasing a heavyweight lock, begin a new transaction,
and acquire a predicate lock. Now, the backend looking at the lock
table is going to see both of those locks held at the same time even
though in reality that never happened. The existing documentation for
pg_locks indicates that such anomalies are possible because it's based
on the (heretofore correct) idea that we're going to grab every single
relevant lwlock at the same time before taking a snapshot of the
system state.

What I think we should do is replace the existing paragraph with
something along these lines:

The <structname>pg_locks<structname> view displays data from both the
regular lock manager and the predicate lock manager, which are
separate systems. When this view is accessed, the internal data
structures of each lock manager are momentarily locked, and copies are
made for the view to display. Each lock manager will therefore
produce a consistent set of results, but as we do not lock both lock
managers simultaneously, it is possible for locks to be taken or
released after we interrogate the regular lock manager and before we
interrogate the predicate lock manager. Each lock manager is only
locked for the minimum possible time so as to reduce the performance
impact of querying this view, but there could nevertheless be some
impact on database performance if it is frequently accessed.

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

Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:

>> It remains true that the heavyweight locking structures are
>> momentarily locked to capture a consistent view of those, and it
>> is also true that the predicate locking structures are
>> momentarily locked to get a consistent view of that data. Both
>> are not covered by some over-arching lock, but...
>
> ...but they could be, if we were so inclined.

Yes.

> What we actually do is acquire all of the lock manager locks,
> snapshot the state, release those locks, then get the predicate
> lock manager locks and SerializableXactHashLock, snapshot the
> state over there, release those locks, and then dump everything
> out. Since we don't do that, it's possible that "select * from
> pg_locks" could see a state that never really existed.
>
> For example, it's possible that, after doing GetLockStatusData()
> but before doing GetPredicateLockStatusData(), some backend might
> commit a transaction, releasing a heavyweight lock, begin a new
> transaction, and acquire a predicate lock. Now, the backend
> looking at the lock table is going to see both of those locks held
> at the same time even though in reality that never happened.

That's a fair point.

> What I think we should do is replace the existing paragraph with
> something along these lines:
>
> The <structname>pg_locks<structname> view displays data from both
> the regular lock manager and the predicate lock manager, which are
> separate systems. When this view is accessed, the internal data
> structures of each lock manager are momentarily locked, and copies
> are made for the view to display. Each lock manager will
> therefore produce a consistent set of results, but as we do not
> lock both lock managers simultaneously, it is possible for locks
> to be taken or released after we interrogate the regular lock
> manager and before we interrogate the predicate lock manager.
> Each lock manager is only locked for the minimum possible time so
> as to reduce the performance impact of querying this view, but
> there could nevertheless be some impact on database performance if
> it is frequently accessed.

I agree that it's probably better to document it than to increase
the time that both systems are locked. If we get complaints about
it, we can always revisit the issue in a later release.

The above wording looks fine to me.

-Kevin

On Fri, Jun 24, 2011 at 1:08 PM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
>> What I think we should do is replace the existing paragraph with
>> something along these lines:
>>
>> The <structname>pg_locks<structname> view displays data from both
>> the regular lock manager and the predicate lock manager, which are
>> separate systems.  When this view is accessed, the internal data
>> structures of each lock manager are momentarily locked, and copies
>> are made for the view to display.  Each lock manager will
>> therefore produce a consistent set of results, but as we do not
>> lock both lock managers simultaneously, it is possible for locks
>> to be taken or released after we interrogate the regular lock
>> manager and before we interrogate the predicate lock manager.
>> Each lock manager is only locked for the minimum possible time so
>> as to reduce the performance impact of querying this view, but
>> there could nevertheless be some impact on database performance if
>> it is frequently accessed.
>
> I agree that it's probably better to document it than to increase
> the time that both systems are locked.  If we get complaints about
> it, we can always revisit the issue in a later release.
>
> The above wording looks fine to me.

OK, committed that change.

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