"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>> OK case by case. I will be glad to check them one by one.
> I'm checking them for AccessExclusiveLock now.
> As for RowExclusiveLock,it would be much effective to remove
> the release of lock unconditionally.
> RowExclusiveLock isn't so intensive a lock. For example it
> doesn't conflict relatively. Therefore it would be hard to find
> and resolve the bugs which are caused by the early release
> of RowExclusiveLock,deadlock etc ...
> Holding the lock a little longer won't be so harmful inversely.
We could try it and see, certainly. You are probably right that it
would not be harmful to hold it.
> As for AccessExclusiveLock I found followings now.
> 3) commands/dbcommands.c(DROP DATABASE)
> elog(ERROR) follows immediately. The release should
> be removed.
> 5) commands/vacuum.c(VACUUM)
> The release is caused by new security check. Probably
> the check could be done before acquiring AccessExcl-
> usiveLock.
In both of these cases, we are closing the system table unmodified,
and AFAICT the point is just to release the lock a tad sooner than
we otherwise would. (I coded the VACUUM code just like code I'd seen
elsewhere.) It's probably harmless either way.
> 6) commands/commands.c(ALTER TABLE)
> ALTER TABLE doesn't release AccessExclusiveLock till
> end of transaction. I couldn't find any reason to allow
> the release for inheritors of a relation class. The release
> should be removed.
Yes, the recursive subroutine will grab the same lock and not release
it. I'm not sure why it's coded the way it is, but certainly there's
no benefit to releasing the lock earlier at the outer level.
> 7) commands/async.c(LISTEN/UNLISTEN)
> This case seems dangerous too but unfortunately I couldn't
> point out a concrete flaw now.
Holding the lock on pg_listener longer than absolutely necessary strikes
me as very risky, since any other backend might need to grab the lock
before it can complete its own transaction (in order to send or receive
notifies). Deadlock could ensue depending on what other locks are held.
I think the locking logic for pg_listener was last revised for 6.4 (by me).
It seems to work fine as-is, but it doesn't know anything about MVCC.
It is possible that we could downgrade the locks to RowExclusiveLock and
rely on MVCC semantics instead of a hard lock. This would require
careful study however, in particular to be sure that cross-backend
notifies couldn't be missed because of not-yet-committed tuples.
I haven't had time to think about it...
regards, tom lane