On Wed, Nov 16, 2022 at 1:35 AM Robert Haas <robertmh...@gmail.com> wrote: > > On Fri, Nov 4, 2022 at 4:27 AM Drouvot, Bertrand > <bertranddrouvot...@gmail.com> wrote: > > Please find attached a rebase in v7. > > I don't think it's a good thing that this patch is using the > progress-reporting machinery. The point of that machinery is that we > want any backend to be able to report progress for any command it > happens to be running, and we don't know which command that will be at > any given point in time, or how many backends will be running any > given command at once. So we need some generic set of counters that > can be repurposed for whatever any particular backend happens to be > doing right at the moment.
Hm. > But none of that applies to the checkpointer. Any information about > the checkpointer that we want to expose can just be advertised in a > dedicated chunk of shared memory, perhaps even by simply adding it to > CheckpointerShmemStruct. Then you can give the fields whatever names, > types, and sizes you like, and you don't have to do all of this stuff > with mapping down to integers and back. The only real disadvantage > that I can see is then you have to think a bit harder about what the > concurrency model is here, and maybe you end up reimplementing > something similar to what the progress-reporting stuff does for you, > and *maybe* that is a sufficient reason to do it this way. -1 for CheckpointerShmemStruct as it is being used for running checkpoints and I don't think adding stats to it is a great idea. Instead, extending PgStat_CheckpointerStats and using shared memory stats for reporting progress/last checkpoint related stats is a good idea IMO. 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. Also, removing CheckpointStatsData and moving all of those members to PgStat_CheckpointerStats, of course, by being careful about the amount of shared memory required, is also a good idea IMO. Going forward, PgStat_CheckpointerStats and pg_stat_checkpoint view can be a single point of location for all the checkpoint related stats. Thoughts? In fact, I was recently having an off-list chat with Bertrand Drouvot about the above idea. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
