On 2022-Nov-01, Michael Paquier wrote: > On Mon, Oct 31, 2022 at 03:14:54PM +0800, Japin Li wrote: > > For example, since SetCommitTsLimit() is only used in BootStrapXLog() and > > StartupXLOG(), we can safely remove the code of acquiring/releasing lock? > > Logically yes, I guess that you could go without the LWLock acquired > in this routine at this early stage of the game. Now, perhaps that's > not worth worrying, but removing these locks could impact any external > code relying on SetCommitTsLimit() to actually hold them.
My 0.02€: From an API point of view it makes no sense to remove acquisition of the lock in SetCommitTsLimit, particularly given that that function is not at all performance critical. I think the first question I would ask, when somebody proposes to make a change like that, is *why* do they want to make that change. Is it just because we *can*? That doesn't sound, to me, sufficient motivation. You actually introduce *more* complexity, because after such a change any future hacker would have to worry about whether their changes are still valid considering that these struct members are modified unlocked someplace. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Syntax error: function hell() needs an argument. Please choose what hell you want to involve.
