Re: Summary and Plan for Hot Standby

On Thu, 2009-11-19 at 10:13 +0200, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > Recovery does *not* take the same locks as the original statements on
> > the master took. For example, the WAL record for an INSERT just makes
> > its changes without acquiring locks. This is OK as long as we only allow
> > read-only users to acquire AccessShareLocks. If we allowed higher locks
> > we might need to do deadlock detection, which would add more complexity.
>
> But we *do* allow higher locks than AccessShareLocks, as Tatsuo-sans
> example shows. Is that a bug?
>
> > The above restrictions are limited to LOCKTAG_OBJECT so that advisory
> > locks work as advertised. So advisory locks can take both shared and
> > exclusive locks. This never conflicts with recovery because advisory
> > locks are not WAL logged.
>
> So we allow any lock on anything *except* LOCKTAG_OBJECT. That includes
> advisory locks, but also relation locks, tuple locks and page locks.
>
> Looking at the lock types in detail:
>
> LOCKTAG_RELATION
>
> Any lock level is allowed. We have other defenses against actually
> modifying a relation, but it feels a bit fragile and I got the
> impression from your comments that it's not intentional.

Possibly fragile, will look further. LOCKTAG_OBJECT was the important
one in testing.

> LOCKTAG_RELATION_EXTEND
>
> Any lock level is allowed. Again, we have other defenses against
> modifying relations, but feels fragile.

This only ever happens after xid is assigned, which can never happen.
Happy to add protection if you think so.

> LOCKTAG_PAGE
>
> Any lock level is allowed. Page locks are only used when extending a
> hash index, so it seems irrelevant what we do. I think we should
> disallow page locks in standby altogether.

As above, but OK.

> LOCKTAG_TUPLE,
>
> Any lock level is allowed. Only used when locking a tuple for update. We
> forbid locking tuples by the general "is the transaction read-only?"
> check in executor, and if you manage to bypass that, you will fail to
> get an XID to set to xmax. Nevertheless, seems we shouldn't allow tuple
> locks.

Specifically disallowed earlier when row marks queries are requested.

> LOCKTAG_TRANSACTION,
>
> Any lock level is allowed. Acquired in AssignTransactionId, to allow
> others to wait for the transaction to finish. We don't allow
> AssignTransactionId() during recovery, but could someone want to wait
> for a transaction to finish? All the current callers of
> XactLockTableWait() seem to be from operations that are not allowed in
> recovery. Should we take a conservative stance and disallow taking
> transaction-locks?

Only used after xid assignment, which is disallowed.

> LOCKTAG_VIRTUALTRANSACTION
>
> Any lock level is allowed. Similar to transaction locks, but virtual
> transaction locks are held by read-only transactions as well. Also
> during recovery, and we rely on it in the code to wait for a conflicting
> transaction to finish. But we don't acquire locks to represent
> transactions in master.

Only ever requested as exclusive.

> LOCKTAG_OBJECT,
>
> Anything higher than AccessShareLock is disallowed. Used by dependency
> walking in pg_depend.c. Also used as interlock between database start
> and DROP/CREATE DATABASE. At backend start, we normally take
> RowExclusiveLock on the database in postinit.c, but you had to modify to
> acquire AccessShareLock instead in standby mode.

Yes

> LOCKTAG_USERLOCK
> LOCKTAG_ADVISORY
>
> Any lock level is allowed. As documented, advisory locks are per-server,
> so a lock taken in master doesn't conflict with one taken in slave.

Yes

--
Simon Riggs www.2ndQuadrant.com