On Thu, Mar 9, 2023 at 9:24 PM Drouvot, Bertrand <[email protected]> wrote: > > Hi, > > On 2/28/23 4:30 PM, Bharath Rupireddy wrote: > > Hi, > > > > Most of the multiplexed SIGUSR1 handlers are setting latch explicitly > > when the procsignal_sigusr1_handler() can do that for them at the end. > > Right. > > > These multiplexed handlers are currently being used as SIGUSR1 > > handlers, not as independent handlers, so no problem if SetLatch() is > > removed from them. > > Agree, they are only used in procsignal_sigusr1_handler(). > > > A few others do it right by saying /* latch will be > > set by procsignal_sigusr1_handler */. > > Yeap, so do HandleProcSignalBarrierInterrupt() and > HandleLogMemoryContextInterrupt(). > > > Although, calling SetLatch() in > > quick succession does no harm (it just returns if the latch was > > previously set), it seems unnecessary. > > > > Agree.
While reviewing the patch in [1], I noticed this issue and ended up here. I agree with the approach and have attached a revised version of the patch. > > I'm attaching a patch that avoids multiple SetLatch() calls. > > > > Thoughts? > > > > I agree with the idea behind the patch. The thing > that worry me a bit is that the changed functions are defined > as external and so may produce an impact outside of core pg and I'm > not sure that's worth it. I understand the concern. There's no guarantee that PostgreSQL functions behave identically across major versions, so removing redundant SetLatch() calls is generally fine. However, as you are concerned, extensions might call these functions and implicitly rely on the extra SetLatch(). Since the patch doesn't change the API, such behavioral changes may be hard for extension authors to notice. Also they will be not in release notes. In practice, they would probably catch this during testing against a new major version, though. I guess similar situations have come up in past major upgrades, so perhaps I'm overthinking this?? Regards, [1] https://postgr.es/m/CABdArM6pmn5yFqiU33KTYBXYM=vny2ulnjy_gqfbsmedt+1...@mail.gmail.com -- Fujii Masao
v2-0001-Remove-redundant-SetLatch-calls-in-interrupt-hand.patch
Description: Binary data
