Re: Rename ShmemVariableCache and initialize it in more standard way

On 05/12/2023 05:40, Richard Guo wrote:
> On Tue, Dec 5, 2023 at 12:31 AM Tristan Partin <tristan(at)neon(dot)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)