[Anton A. Melnikov <a.melnikov@postgrespro.ru>]

On 20.09.2024 19:19, Fujii Masao wrote:
> I've attached the updated version (0001.patch). I made some cosmetic changes,
> including reverting the switch in the entries for pg_stat_get_checkpointer_write_time
> and pg_stat_get_checkpointer_sync_time in pg_proc.dat, as I didn’t think
> that change was necessary. Could you please review the latest version?

Thanks for corrections!
All looks good for me.
As for switching in the pg_proc.dat entries the idea was to put them in order
so that the pg_stat_get_checkpointer* functions were grouped together.
I don't know if this is the common and accepted practice. Simply i like it better this way.
Sure, if you think it's unnecessary, let it stay as is with minimal diff.


> After we commit 0001.patch, how about applying 0002.patch, which updates
> the documentation for the pg_stat_checkpointer view to clarify what types
> of checkpoints and restartpoints each counter tracks?

I liked that the short definitions of the counters are now separated from
the description of its work features which are combined into one paragraph.
It seems to me that is much more logical and easier to understand.

In addition, checkpoints may be skipped due to "checkpoints are occurring
too frequently" error. Not sure, but maybe add this information to
the new description?

> In 0002.patch, I also modified the description of num_requested from
> "Number of backend requested checkpoints" to remove "backend," as it can
> be confusing since num_requested includes requests from sources other than
> the backend. Thought?

Agreed. E.g. from xlog. Then maybe changed it also in the function
descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
and pg_stat_get_checkpointer_restartpoints_requested().


Also checked v4 with the travis patch-tester. All is ok.

With the best wishes!

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company