Re: Introduce a new view for checkpointer related stats

On Fri, Feb 10, 2023 at 6:16 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> That's true for buffers_fsync_backend, but not true for
> buffers_backend/buffers_written_backend.
>
> That isn't tied to checkpointer.
>
> I think if we end up breaking compat, we should just drop that column. The
> pg_stat_io patch from Melanie, which I hope to finish committing by tomorrow,
> provides that in a more useful way, in a less confusing place.

On Fri, Feb 10, 2023 at 11:29 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Feb 09, 2023 at 04:46:04PM -0800, Andres Freund wrote:
> > I think if we end up breaking compat, we should just drop that
> > column.
>
> Indeed.

Yeah, pg_stat_io is a better place to track the backend IO stats. I
removed buffers_backend, please see the attached 0001 patch.

On Fri, Feb 10, 2023 at 6:16 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> I'm not sure it's worth having buffers_fsync_backend in pg_stat_checkpointer
> in that case. You can get nearly the same information from pg_stat_io as well
> (except fsyncs for SLRUs that couldn't be put into the queue, which you'd not
> see right now - hard to believe that ever happens at a relelvant frequency).

I think it'd be better to move the SLRU fsync stats during checkpoints
to the pg_stat_slru view, then it can be a one-stop view to track all
SLRU IO stats. This lets us remove buffers_fsync_backend too, please
see the attached 0002 patch. However, one metric we might miss is the
number of times checkpointer missed to absorb the fsync requests. If
needed, this metric can be added to the new pg_stat_checkpointer view.

> > Yep, and I think you are all wrong, and that this is just going to cause
> > unnecessary pain :). I'm not going to try to prevent the patch from going in
> > because of this, just to be clear.
>
> Catalog attributes have faced a lot of renames across the years, with
> the same potential of breakages for monitoring tools. I am not saying
> that all of them are justified, but we have usually done so because it
> makes sense to reshape things in the way they are now, thinking
> long-term. Splitting pg_stat_bgwriter into two views does not strike
> me as something that bad, TBH, because it becomes clearer which stats
> are attached to which process (bgwriter or checkpointer). (Note: I
> have not checked in details the stats switching to the new view and
> how pertinent each choice is.)

Thanks. FWIW, I've attached the patch introducing pg_stat_checkpointer
as 0003 here.

Please review the attached v7 patch set.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com