Re: Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up

On 2015-02-03 14:18:02 +0900, Michael Paquier wrote:
> - "RecoveryLockList contains entry for lock no longer recorded by
> lock manager: xid %u database %u relation %u",
> - lock->xid, lock->dbOid, lock->relOid);
> + "RecoveryLockList contains entry for lock no longer recorded by
> lock manager: xid %u",
> + lock->xid);
> This patch is making the information provided less verbose, and I
> think that it is useful to have some information not only about the
> lock held, but as well about the database and the relation.

It's debug4 or impossible stuff that lock.c already warned about - I
doubt anybody has ever actually looked at it in a long while, if
ever. If we really want to provide something more we can use something
like LOCK_PRINT() - but I really doubt it's worth neither the
notational, nor the verbosity overhead.

> Also, ISTM that StandbyAcquireLock should still use a database OID and
> a relation OID instead of a only LOCKTAG, and SET_LOCKTAG_RELATION
> should be set in StandbyAcquireLock while
> ResolveRecoveryConflictWithLock is extended only with the lock mode as
> new argument. (Patch 2 adds many calls to SET_LOCKTAG_RELATION btw
> justidying to keep he API changes minimal).

But there's now callers acquiring other locks than relation locks, like
dbase_redo() acquiring a object lock. And we need to acquire those via
the standby mechanism to avoid races around release. We could add a
separate wrapper for relation locks, but imo the locktag move to the
callers saved about as many lines in some places as it cost in others.

> In patch 2, isn't it necessary to bump XLOG_PAGE_MAGIC?

I don't think so, there's no incompatible change.

Thanks for having a look!

Andres

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