On Fri, Aug 13, 2021 at 3:08 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2021-08-11 16:11:34 -0400, Melanie Plageman wrote:
> > On Tue, Aug 3, 2021 at 2:13 PM Andres Freund <andres@anarazel.de> wrote:
> > > > Also, I'm unsure how writing the buffer action stats out in
> > > > pgstat_write_statsfiles() will work, since I think that backends can
> > > > update their buffer action stats after we would have already persisted
> > > > the data from the BufferActionStatsArray -- causing us to lose those
> > > > updates.
> > >
> > > I was thinking it'd work differently. Whenever a connection ends, it reports
> > > its data up to pgstats.c (otherwise we'd loose those stats). By the time
> > > shutdown happens, they all need to have already have reported their stats - so
> > > we don't need to do anything to get the data to pgstats.c during shutdown
> > > time.
> > >
> >
> > When you say "whenever a connection ends", what part of the code are you
> > referring to specifically?
>
> pgstat_beshutdown_hook()
>
>
> > Also, when you say "shutdown", do you mean a backend shutting down or
> > all backends shutting down (including postmaster) -- like pg_ctl stop?
>
> Admittedly our language is very imprecise around this :(. What I meant
> is that backends would report their own stats up to the stats collector
> when the connection ends (in pgstat_beshutdown_hook()). That means that
> when the whole server (pgstat and then postmaster, potentially via
> pg_ctl stop) shuts down, all the per-connection stats have already been
> reported up to pgstat.
>
So, I realized that the patch has a problem. I added the code to send
buffer actions stats to the stats collector
(pgstat_send_buffer_actions()) to pgstat_report_stat() and this isn't
getting called when all types of backends exit.
I originally thought to add pgstat_send_buffer_actions() to
pgstat_beshutdown_hook() (as suggested), but, this is called after
pgstat_shutdown_hook(), so, we aren't able to send stats to the stats
collector at that time. (pgstat_shutdown_hook() sets pgstat_is_shutdown
to true and then in pgstat_beshutdown_hook() (called after), if we call
pgstat_send_buffer_actions(), it calls pgstat_send() which calls
pgstat_assert_is_up() which trips when pgstat_is_shutdown is true.)
After calling pgstat_send_buffer_actions() from pgstat_report_stat(), it
seems to miss checkpointer stats entirely. I did find that if I
sprinkled pgstat_send_buffer_actions() around in the various places that
pgstat_send_checkpointer() is called, I could get checkpointer stats
(see attached patch, capture_checkpointer_buffer_actions.patch), but,
that seems a little bit haphazard since pgstat_send_buffer_actions() is
supposed to capture stats for all backend types. Is there somewhere else
I can call it that is exercised by all backend types before
pgstat_shutdown_hook() is called but after they would have finished any
relevant buffer actions?
- Melanie
