Relation locking and relcache load (was Re: Going for "all green" buildfarm results)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Relation locking and relcache load (was Re: Going for "all green" buildfarm results)
Date: 2006-07-30 18:39:44
Message-ID: 2880.1154284784@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> I bet Alvaro's spotted the problem. ALTER INDEX RENAME doesn't seem to
> take any lock on the index's parent table, only on the index itself.
> That means that a query on "onek" could be trying to read the pg_class
> entries for onek's indexes concurrently with someone trying to commit
> a pg_class update to rename an index. If the query manages to visit
> the new and old versions of the row in that order, and the commit
> happens between, *neither* of the versions would look valid. MVCC
> doesn't save us because this is all SnapshotNow.

On further reflection, it seems this is a special case of the general
problem that we ought to lock a relation *before* we attempt to load
the relcache entry, not after. We've seen previous complaints about
this, for instance a process erroring out with a message about
pg_trigger having a different number of entries than it expected because
someone had committed an ADD/DROP TRIGGER between it reading pg_class
and reading pg_trigger.

A long time ago (30 Oct 2002) I wrote:

> Thinking further, it's really kinda bogus that LockRelation() works on
> an already-opened Relation; if possible we should acquire the lock
> before attempting to create a relcache entry. (We only need to know the
> OID and the relisshared status before we can make a locktag, so it'd be
> possible to acquire the lock using only the contents of the pg_class row.)
> Not sure how much code restructuring might be involved to make this
> happen, but it'd be worth thinking about for 7.4.

but that never got off the back burner. The current example shows that
"read the pg_class row and then lock" doesn't work anyway, because we
might fail to find any version of the pg_class row that passes
SnapshotNow.

One thing that has improved since 7.3 is that we only do relcache loads
by OID, never by name. This means the only thing stopping us from
taking lock before we invoke relcache is lack of knowledge about the
rel's relisshared status. Given that the set of shared relations is
pretty small, and fixed in any given backend version, it wouldn't be
unreasonable to handle this by keeping a hardwired list of shared
relation OIDs somewhere in the backend.

Anyway I really think we need to move to a coding rule that says thou
shalt not access a relcache entry without *already* having some kind
of lock on the relation.

Comments?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hannu Krosing 2006-07-30 18:55:05 Re: PATCH to allow concurrent VACUUMs to not lock each
Previous Message Sergey E. Koposov 2006-07-30 18:33:56 problem with volatile functions in subselects ?