Re: ALTER TABLE lock strength reduction patch is unsafe - Mailing list pgsql-hackers

On Wed, Nov 30, 2011 at 4:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> It strikes me that there are really two separate problems here.
>
> 1. If you are scanning a system catalog using SnapshotNow, and a
> commit happens at just the right moment, you might see two versions of
> the row or zero rather than one.  You'll see two if you scan the old
> row version, then the concurrent transaction commits, and then you
> scan the new one.  You'll see zero if you scan the new row (ignoring
> it, since it isn't committed yet) and then the concurrent transaction
> commits, and then you scan the old row.

That is a bug and one we should fix. I supplied a patch for that,
written to Tom's idea for how to solve it.

I will apply that, unless there are objections.

> 2. Other backends may have data in the relcache or catcaches which
> won't get invalidated until they do AcceptInvalidationMessages().
> That will always happen no later than the next time they lock the
> relation, so if you are holding AccessExclusiveLock then you should be
> OK: no one else holds any lock, and they'll have to go get one before
> doing anything interesting.  But if you are holding any weaker lock,
> there's nothing to force AcceptInvalidationMessages to happen before
> you reuse those cache entries.

Yes, inconsistent cache entries will occur if we allow catalog updates
while the table is already locked by others.

The question is whether that is a problem in all cases.

With these ALTER TABLE subcommands, I don't see any problem with
different backends seeing different values.
            /*             * These subcommands affect general strategies for performance             * and maintenance,
thoughdon't change the semantic results             * from normal data reads and writes. Delaying an ALTER TABLE
    * behind currently active writes only delays the point where             * the new strategy begins to take effect,
sothere is no             * benefit in waiting. In this case the minimum restriction             * applies: we don't
currentlyallow concurrent catalog             * updates.             */        case AT_SetStatistics: 
// only used by ANALYZE, which is shut out by the ShareUpdateExclusiveLock
        case AT_ClusterOn:        case AT_DropCluster:
// only used by CLUSTER, which is shut out because it needs AccessExclusiveLock
        case AT_SetRelOptions:        case AT_ResetRelOptions:        case AT_SetOptions:        case AT_ResetOptions:
     case AT_SetStorage: 
// not critical
        case AT_ValidateConstraint:
// not a problem if some people think its invalid when it is valid

So ISTM that we can allow reduced lock levels for the above commands
if we fix SnapshotNow.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services