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

Hi,

On 2021-08-07 13:37:16 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2021-08-07 13:06:47 -0400, Tom Lane wrote:
> >> Fair.  But I suggest that the first cut should look more like what
> >> I suggest above, ie just be willing to lose events during shutdown.
> >> The downsides of that are not so enormous that we should be willing
> >> to undertake major klugery to avoid it before we've even got a
> >> semi-working system.
> 
> > I think that's more likely to hide bugs unfortunately. Consider fa91d4c91f2 -
> > I might not have found that if we had just ignored "too late" pgstats activity
> > in pgstats.c or fd.c, and that's not an edge case.
> 
> Depends what you want to define as a bug.  What I am not happy about
> is the prospect of random assertion failures for the next six months
> while you finish redesigning half of the system.  The rest of us
> have work we want to get done, too.  I don't object to the idea of
> making no-lost-events an end goal, but we are clearly not ready
> for that today.

I don't know what to do about that. How would we even find these cases if they
aren't hit during regression tests on my machine (nor on a lot of others)?
Obviously I had ran the regression tests many times before pushing the earlier
changes.

The check for pgstat being up is in one central place and thus easily can be
turned into a warning if problems around the shutdown sequence become a
frequent issue on HEAD.  If you think it's better to turn that into a WARNING
now and then arm into an assert later, I can live with that as well, but I
don't think it'll lead to a better outcome.

The shared memory stats stuff isn't my personal project - it's
Horiguchi-san's. I picked it up because it seems like an important thing to
address and because it'd been in maybe a dozen CFs without a lot of
progress. It just turns out that there's a lot of prerequisite changes :(.

Greetings,

Andres Freund