On 2023-11-12 16:46, Michael Paquier wrote:
On Fri, Nov 10, 2023 at 01:15:50PM +0900, Michael Paquier wrote:
The comments added could be better grammatically, but basically LGTM.
I'll take care of that if there are no objections.
The documentation also needed a few tweaks (for DEFAULT and the
argument name), so I have fixed the whole and adapted the new part of
the docs to that, with few little tweaks.
Thanks!
I assume you have already taken this into account, but I think we should
add the same documentation to the below patch for pg_stat_reset_slru():
https://www.postgresql.org/message-id/CALj2ACW4Fqc_m%2BOaavrOMEivZ5aBa24pVKvoXRTmuFECsNBfAg%40mail.gmail.com
On 2023-11-12 16:54, Michael Paquier wrote:
On Fri, Nov 10, 2023 at 08:32:34PM +0900, torikoshia wrote:
On 2023-11-10 13:18, Andres Freund wrote:
I see no reason to not include slrus. We should never have added the
ability to reset them individually, particularly not without a use
case - I couldn't find one skimming some discussion. And what's the
point in not allowing to reset them via pg_stat_reset_shared()?
When including SLRUs, do you think it's better to add 'slrus' argument
which
enables pg_stat_reset_shared() to reset all SLRUs?
I understand that Andres says that he'd be OK with a addition of a
'slru' option in pg_stat_reset_shared(), as well as including SLRUs in
the resets if everything should be wiped.
Thanks, I'll make the patch.
28cac71bd368 is around since 13~, so changing pg_stat_reset_slru() or
removing it could impact existing applications, so there's little
benefit in changing it at this stage. Let it be itself.
+1.
As described above, since SLRUs cannot be reset by
pg_stat_reset_shared(), I
feel a bit uncomfortable to delete it all together.
That would be only effective if NULL is given to the function to reset
everything, which is OK IMO, because this is a shared stats.
--
Michael
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
