On Tue, Dec 3, 2024 at 2:12 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > On 2024-Dec-02, Amit Kapila wrote: > > > Even, if we want a new context for some localized handling, we should > > add that in PGOutputData rather than a local context as the proposed > > patch is doing at the very least for HEAD. > > I don't necessarily agree, given that this context is not needed > anywhere else. >
But that suits the current design more. We allocate PGOutputData and other contexts in that structure in a "Logical decoding context". A few of its members (publications, publication_names) residing in totally unrelated contexts sounds odd. In the first place, we don't need to allocate publications under CacheMemoryContext, they should be allocated in PGOutputData->cachectx. However, because we need to free those entirely at one-shot during invalidation processing, we could use a new context as a child context of PGOutputData->cachectx. Unless I am missing something, the current memory context usage appears more like a coding convenience than a thoughtful design decision. -- With Regards, Amit Kapila.
