Re: Misc. consequences of backend memory management changes

Karel Zak <zakkr(at)zf(dot)jcu(dot)cz> writes:
>>>> Or skip for this specific variant context type independent
>>>> MemoryContextCreate and init this common part itself? - (I vote for
>>>> this)
>>
>> No, certainly not. Just pass a NULL for parent if you don't want to
>> connect it up to the local context tree.

> Well, the query cache shared memory pool and qCache_TopContext (parent of
> all cache entry) are init during postmarter start up. In this time not exist
> TopMemoryContext and 'node' for this context will alloc via malloc().

Look again ;-). TopMemoryContext is created as the first or second step
in postmaster startup, long before we attach to shared mem.

> *But* if I add entry to query cache I create for each entry separate memory
> context and it must be in shared memory, but in shared memory must be not
> only data allocated via this context, in shmem must be _all_ for context
> relevant data --- so, context independent data too (bacause context must
> overlive backend end). A problem is that common MemoryContextCreate() expects
> that context 'node' is allocated in the TopMemoryContext or by force of
> malloc(). Not is possible allocate context node in a parent context.

Good point.

> I read your comment "since the node must survive resets of its parent
> context", but how resolve this?

Well, there's a fundamental problem with keeping memory contexts that
meet this API in shared memory anyway: the contexts have to contain
parent/child pointers pointing at other contexts, and the contexts
have to contain pointers to context-management function pointer structs.
Neither of these are very safe for shared memory.

You've probably noticed that we keep cross-pointers in shared memory in
the form of offsets from the start of the shmem block, not as absolute
pointers. This is to support the possibility that the shmem block is
mapped at different addresses in different backends. (It's possible
that we could give up that flexibility now that all the backends are
forked from a postmaster that's already attached the shmem block.
But I'd rather not wire such an assumption into the contents of shmem.)

Similarly, it would be risky to assume that the functions all appear
at the same address in every backend connected to the shmem block.

What I think you probably want to do is create "placeholder" context
nodes in backend local memory to represent the shared-memory contexts.
The placeholder would contain the MemoryContext fields and a shmem
offset to a struct in shared memory that is the "real" control info
for the context.

If every backend has a placeholder node for every context in shmem
then you have a bit of a problem deleting shmem contexts, but I
think you probably don't need to do that. There should be a permanent
placeholder for the top-level shmem context (create this during
backend startup), but placeholder context nodes for per-plan contexts
only need to live for as long as you are copying nodetrees into the
context. They don't have to stick around longer than that.

A few minutes later:

Now that I think about it, this whole notion of copying nodetrees
into shared memory has got a pointer problem, because the constructed
tree is going to be full of pointers that will only be valid for a
backend that has shmem attached at the same address that the creating
backend had.

Do we really want to give up flexibility of shmem attach address in
order to have a shared-memory plan cache? I've been dubious about
the notion of a shared plan cache from day one (I think the locking
overhead is going to be intolerable) and this may be the problem
that kills it completely.

Before you say "we will never again care about flexibility of attach
address", consider these two scenarios:
1. Examining shmem using a debugging process started independently
of the postmaster.
2. "Hot swapping" a new compilation of the postmaster/backend without
service interruption. This is possible (for changes that don't
change the layout of shmem, of course) by killing and restarting
the postmaster without killing extant backends. But you couldn't
assume that the new incarnation of the postmaster would attach to
the existing shmem blocks at the exact same addresses the previous
incarnation had been assigned.
We don't have either of these today but we might want to do them
at some point in the future. There may be other reasons that I
didn't come up with in a moment's thought.

>>>> Plan you some changes in SPI? I have new code for SPI save plan
>>>> (per-context and via query cache).
>>
>> I have been wondering about that. I don't like SPI saving plans in
>> TopMemoryContext because I don't think it knows how to get rid of them;
>> a plan saved there is effectively a permanent memory leak. Not sure
>> what to do about it though.

> We already told about it with Hiroshi. I rewrite SPI_sevaplan(), now it
> save each plan to separate context and is possible use SPI_freeplan().
> Also I add 'by_key' interface to SPI that allows save planns to the query
> cache under some key (string or binary key). My SPI *not* use
> TopMemoryContext.

That all sounds good. Plan cache in local memory seems reasonable to
me, I'm just worried about the shared-mem aspect of it.

regards, tom lane