I wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> ISTM that this would change the "immediate shutdown" to not save stats
>> files anymore. So far, all the shutdown modes are equivalent in terms
>> of how they preserve data and system state. They differ only in when
>> the hard work happens. This would be a deviation from that principle.
> There is that. Up to now, an immediate shutdown request didn't cause
> any actual data loss, but now it would.
Oh, no, after rereading the thread I remember the point here: during
crash recovery, we intentionally zap the stats files on the grounds
that they might be broken, or might be out-of-sync with the recovery
stop point. See pgstat_reset_all().
Now, you could certainly argue that that's an idea we should rethink;
we're throwing away probably-useful data because it *might* be wrong,
which seems rather pointless when it's known to be approximate anyway.
But if that stays, there's no very good reason not to short-circuit
writing of the files during immediate shutdown.
On the other side of the coin: I note that in 9.4 and up, the postmaster
has a 5-second patience limit before SIGKILL'ing child processes during a
crash or immediate shutdown (cf commit 82233ce7e). That limit applies to
the stats collector too, and it seems to me that it addresses the real
problem here (ie, unbounded shutdown time) much better than the proposed
patch. Note that the complaint that started this thread was against 9.2.
My feeling is that 82233ce7e has obsoleted all of the proposals made so
far in this thread, and that we should reject them all. Instead we should
think about whether pgstat_reset_all should go away.
regards, tom lane