On 2022/02/10 20:28, Masahiko Sawada wrote:
While I understand that these are theoretically valid concerns, it’s unclear to me how much the current code leads to bad effects in practice. There are other low-level codes/paths that call palloc() while holding LWLocks, and I’m not sure that the overheads result in visible negative performance impact particularly because the calling to LockBufferForCleanup() is likely to accompany wait in the first place.
I'm also not sure that. palloc() for PS display, copy of process title, GetCurrentTimestamp() and ereport() of recovery conflict happen when we fail to acquire the lock. In this case we also call ResolveRecoveryConflictWithBufferPin() and wait to be signaled there. So compared to the wait time or overhead caused in ResolveRecoveryConflictWithBufferPin(), ISTM that the overhead caused by them would be ralatively small. Also ereport() of recovery conflict can happen only when log_recovery_conflict_waits parameter is enabled and the deadlock timeout happens during the lock wait.
BTW I think calling to LockBufferForCleanup() in a critical section is a bad idea for sure since it becomes uninterruptible.
Yes. And as far as I tested, currently LockBufferForCleanup() is not called in critical section. Right? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
