Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Sat, Aug 7, 2021 at 11:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Intuitively it seems like temp file management should be a low-level,
> backend-local function and therefore should be okay to run after
> shmem disconnection.  I do not have a warm feeling about reversing that
> module layering --- what's to stop someone from breaking things by
> trying to use a temp file in their on_proc_exit or on_shmem_exit hook?
> Maybe what needs to go overboard is point 1.
>
> More generally, this points up the fact that we don't have a well-defined
> module hierarchy that would help us understand what code can safely do which.
> I'm not volunteering to design that, but maybe it needs to happen soon.

Yeah, I was quite surprised when I saw this commit, because my first
reaction was - why in the world would temporary file shutdown properly
precede DSM shutdown, given that temporary files are a low-level
mechanism? The explanation that we're trying to send statistics at
that point makes sense as far as it goes, but it seems to me that we
cannot be far from having a circular dependency. All we need is to
have DSM require the use of temporary files, and we'll end up needing
DSM shutdown to happen both before and after temporary file cleanup.

/me wonders idly about dynamic_shared_memory_type=file

I think that subsystems like "memory" and "files" really ought to be
the lowest-level things we have, and should be shut down last. Stuff
like "send a message to the stats collector" seems like a higher level
thing that may require those lower-level facilities in order to
operate, and must therefore be shut down first. Maybe some subsystems
need to be divided into upper and lower levels to make this work, or,
well, I don't know, something else. But I'm deeply suspicious that
lifting stuff like this to the front of the shutdown sequence is just
papering over the problem, and not actually solving it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com