Feedback-ID: id4a34324:Fastmail
Date: Wed, 16 Nov 2022 10:14:33 -0800
From: Andres Freund <andres@anarazel.de>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	"Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>,
	Nitin Jadhav <nitinjadhavpostgres@gmail.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Ashutosh Sharma <ashu.coek88@gmail.com>,
	Julien Rouhaud <rjuju123@gmail.com>,
	Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>,
	Magnus Hagander <magnus@hagander.net>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint
 (was: Report checkpoint progress in server logs)
Message-ID: <20221116181433.y2hq2pirtbqmmndt@awork3.anarazel.de>
References: 
 <CALj2ACXsmvEDsHtADoQeDE-NVyBh4A-D80NmNanTUc37g=FQ-Q@mail.gmail.com>
 <CAMm1aWabDx3V9Hw5L2eB+6EOtr60QWDybZShQsYLCCMNF-es_A@mail.gmail.com>
 <CAMm1aWaFmbc8QCbcPkG7ROV7gjshamS=sJRJd=oghLJM9uOnpQ@mail.gmail.com>
 <20220319001556.atuqbmafvc73pa2k@alap3.anarazel.de>
 <CAMm1aWa_3N1Z2-_YwdNxfogs_rxzpu90VZeKjqwjerCVq9rQAw@mail.gmail.com>
 <20220707000446.hefgyu5xikxwt4md@alap3.anarazel.de>
 <CAMm1aWbfFwT+Cf1eSrXtF2R8ffxXTbH=un8unQ8WbcpcbymnYw@mail.gmail.com>
 <d9546a13-dd86-57d8-3bb4-32136fbb3673@gmail.com>
 <CA+TgmoaJEDirAMq_9q3B8PrXDf1h72S7hhPr4aotakYe9HLoLw@mail.gmail.com>
 <CALj2ACX5xK_X=MSBh_Qe518O7V5D2phnpXuSD9=-zAkGnyo9vA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CALj2ACX5xK_X=MSBh_Qe518O7V5D2phnpXuSD9=-zAkGnyo9vA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20221116181433.y2hq2pirtbqmmndt%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2022-11-16 16:01:55 +0530, Bharath Rupireddy wrote:
> -1 for CheckpointerShmemStruct as it is being used for running
> checkpoints and I don't think adding stats to it is a great idea.

Why?  Imo the data needed for progress reporting aren't really "stats". We'd
not accumulate counters over time, just for the current checkpoint.

I think it might even be useful for other parts of the system to know what the
checkpointer is doing, e.g. bgwriter or autovacuum could adapt the behaviour
if checkpointer can't keep up. Somehow it'd feel wrong to use the stats system
as the source of such adjustments - but perhaps my gut feeling on that isn't
right.

The best argument for combining progress reporting with accumulating stats is
that we could likely share some of the code. Having accumulated stats for all
the checkpoint phases would e.g. be quite valuable.


> Instead, extending PgStat_CheckpointerStats and using shared memory
> stats for reporting progress/last checkpoint related stats is a good
> idea IMO

There's certainly some potential for deduplicating state and to make stats
updated more frequently. But that doesn't necessarily mean that putting the
checkpoint progress into PgStat_CheckpointerStats is a good idea (nor the
opposite).


> I also think that a new pg_stat_checkpoint view is needed
> because, right now, the PgStat_CheckpointerStats stats are exposed via
> the pg_stat_bgwriter view, having a separate view for checkpoint stats
> is good here.

I agree that we should do that, but largely independent of the architectural
question at hand.

Greetings,

Andres Freund