On Wed, Aug 21, 2019 at 6:38 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > FWIW, although I also thought of doing what you are describing here, I > > think Andres's proposal is probably preferable, because it's simpler. > > There's not really any reason why we can't take the buffer locks from > > within the critical section, and that way callers don't have to deal > > with the extra step. > > IIRC, the reason this was done before starting critical section was > because of coding convention mentioned in src/access/transam/README > (Section: Write-Ahead Log Coding). It says first pin and exclusive > lock the shared buffers and then start critical section. It might be > that we can bypass that convention here, but I guess it is mainly to > avoid any error in the critical section. I have checked the > LWLockAcquire path and there doesn't seem to be any reason that it > will throw error except when the caller has acquired many locks at the > same time which is not the case here.
Yeah, I think it's fine to deviate from that convention in this respect. We treat LWLockAcquire() as a no-fail operation in many places; in my opinion, that elog(ERROR) that we have for too many LWLocks should be changed to elog(PANIC) precisely because we do treat LWLockAcquire() as no-fail in lots of places in the code, but I think I suggested that once and got slapped down, and I haven't had the energy to fight about it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company