Re: Lock on ShmemVariableCache fields?

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.