I've been thinking more about Gaetano Mendola's report of a
FlushRelationBuffers failure during VACUUM FULL.  I still don't see how
that could happen in 7.4, but I do see a real related risk in 8.0.
The problem is that in many situations we assume that a lock on an
index's parent table is sufficient to protect accesses to the index
too.  This is broken in CVS tip by ALTER INDEX SET TABLESPACE, which
physically moves the index with only an exclusive lock on the index
itself, and therefore could cause problems for concurrent accesses
to the index that are expecting a lock on the table to protect them.

We could fix this specific issue by hacking ATExecSetTableSpace to
take out a lock on the parent table when it's processing an index,
but the more I think about it the more I think that it's time for a
new approach.  The current way is just too fragile.

What I'd like to propose are the following New Rules:

1. The schema definition of an index cannot be altered without first
getting exclusive lock on the parent table.  This means it is still
safe for the planner to examine the set of available indexes and develop
a query plan without locking every index.

2. Physical access to the index contents always requires getting an
appropriate lock on the index itself.

#1 is existing practice, so no problem there.  The places that would
need to change to support #2 are (so far as I've found)
ExecOpenIndices/ExecCloseIndices, which should take RowExclusiveLock as
foreseen in the comments therein, and VACUUM/VACUUM FULL/ANALYZE which
should take RowExclusiveLock, AccessExclusiveLock, AccessShareLock
respectively.

Plain SELECT operations already take AccessShareLock when using an index
(in index_beginscan) so there is no additional overhead for them.
Updating commands would take out a few more locks than before, but I
doubt this will really be a serious overhead.

One immediate benefit of this is that REINDEX INDEX would no longer need
to get an exclusive lock on the index's parent table; ShareLock would be
sufficient.  This would greatly reduce the deadlock risk noted in
reindex_index().

Comments?

			regards, tom lane

Thu, 30 Sep 2004 15:54:57 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Index locking considerations
Tom Lane wrote:
> I've been thinking more about Gaetano Mendola's report of a
> FlushRelationBuffers failure during VACUUM FULL.  I still don't see how
> that could happen in 7.4, but I do see a real related risk in 8.0.

Just another information that could help your analysis.

The index's parent is a table that implement a "materialized view" so is
heavily updated by triggers.


Regards
Gaetano Mendola


Fri, 01 Oct 2004 00:14:20 +0200
Gaetano Mendola <mendola@bigfoot.com>
Re: Index locking considerations