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

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2021-08-06 21:49:52 -0700, Andres Freund wrote:
>> Not yet really sure what the best way to deal with this is. Presumably this
>> issue would be fixed if AtProcExit_Files()/CleanupTempFiles() were scheduled
>> via before_shmem_exit(). And perhaps it's not too off to schedule
>> CleanupTempFiles() there - but it doesn't quite seem entirely right either.

> Huh. I just noticed that AtProcExit_Files() is not even scheduled via
> on_shmem_exit() but on_proc_exit(). That means that even before fb2c5028e63
> we sent pgstat messages *well* after pgstat_shutdown_hook() already
> ran. Crufty.

> Just hacking in an earlier CleanupTempFiles() does "fix" the OSX issue:
> https://cirrus-ci.com/task/5941265494704128?logs=macos_basebackup#L4

So if I have the lay of the land correctly:

1. Somebody decided it'd be a great idea for temp file cleanup to send
stats collector messages.

2. Temp file cleanup happens after shmem disconnection.

3. This accidentally worked, up to now, because stats transmission happens
via a separate socket not shared memory.

4. We can't keep both #1 and #2 if we'd like to switch to shmem-based
stats collection.

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.

regards, tom lane