On Mon, 2025-05-19 at 14:07 -0400, Robert Haas wrote:
> I agree with that, but I think that it may also be error-prone to
> assume that it's OK to acquire heavyweight locks on other catalog
> objects at any place in the code where we record a dependency. I will
> not be surprised at all if that turns out to have some negative
> consequences. For example, I think it might result in acquiring the
> locks on those other objects at a subtly wrong time (leading to race
> conditions) or acquiring two locks on the same object with different
> lock modes where we should really only acquire one. I'm all in favor
> of solving this problem using heavyweight locks, but I think that
> implicitly acquiring them as a side effect of recording dependencies
> feels too surprising.
I see what you mean now, in the sense that other code that calls
LockDatabaseObject (and other variants of LockAcquire) are mostly
higher-level operations like AlterPublication(), and not side-effects
of something else.
But relation_open() is sort of an exception. There are lots of places
where that takes a lock because we happen to want something out of the
relcache, like generate_partition_qual() taking a lock on the parent or
CheckAttributeType() taking a lock on the typrelid. You could say those
are fairly obvious, but that's because we already know, and we could
make it more widely known that recording a dependency takes a lock.
One compromise might be to have recordDependencyOn() take a LOCKMODE
parameter, which would both inform the caller that a lock will be
taken, and allow the caller to do it their own way and specify NoLock
if necessary. That still results in a huge diff, but the end result
would not be any more complex than the current code.
Regards,
Jeff Davis
