On 05/12/2023 05:40, Richard Guo wrote:
On Tue, Dec 5, 2023 at 12:31 AM Tristan Partin <tris...@neon.tech> wrote:
On Mon Dec 4, 2023 at 6:49 AM CST, Heikki Linnakangas wrote:
> Here's a patch to allocate and initialize it with a pair of
ShmemSize
> and ShmemInit functions, like all other shared memory structs.
>
> + if (!IsUnderPostmaster)
> + {
> + Assert(!found);
> + memset(ShmemVariableCache, 0,
sizeof(VariableCacheData));
> + }
> + else
> + Assert(found);
Should the else branch instead be a fatal log?
The Assert here seems OK to me. We do the same when initializing
commitTsShared/MultiXactState. I think it would be preferable to adhere
to this convention.
Right. I'm not 100% happy with that pattern either, but better be
consistent.
There's a brief comment about this in CreateOrAttachShmemStructs():
* This is called by the postmaster or by a standalone backend.
* It is also called by a backend forked from the postmaster in the
* EXEC_BACKEND case. In the latter case, the shared memory segment
* already exists and has been physically attached to, but we have to
* initialize pointers in local memory that reference the shared structures,
* because we didn't inherit the correct pointer values from the postmaster
* as we do in the fork() scenario. The easiest way to do that is to run
* through the same code as before. (Note that the called routines mostly
* check IsUnderPostmaster, rather than EXEC_BACKEND, to detect this case.
* This is a bit code-wasteful and could be cleaned up.)
The last sentence refers to this pattern.
Patches look good to me.
Also +1 to the patches.
Committed, thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)
