Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 27 Dec 2024 13:00:37 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tomas Vondra <tomas@vondra.me>
Cc: wenhui qiu <qiuwenhuifx@gmail.com>, Robert Treat <rob@xzilla.net>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: PoC: history of recent vacuum/checkpoint runs (using new hooks)
Message-ID: <Z24mZZAPKkw02oy2@paquier.xyz>
References: <b68ab452-c41f-4d04-893f-eaab84f1855b@vondra.me>
 <CAGjGUAJSg3tJGxjeyU6T98ZYCXcAiGpq0eitAUQVaOa+nULDuA@mail.gmail.com>
 <8df7cee1-31aa-4db3-bbb7-83157ca139da@vondra.me>
 <CABV9wwPcnoS+=27o=pHinCy7fegTTfB=yECyK5XyGecCFfrFHg@mail.gmail.com>
 <CAGjGUAKXjFw+zq+9GZ+y5p360pkO7UuTojkKuB6k3MtdDCEf6w@mail.gmail.com>
 <a40fc9ea-ec9a-4f34-a5d5-42629c76fbb2@vondra.me>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="rAVWtuanTv+xcdc0"
Content-Disposition: inline
In-Reply-To: <a40fc9ea-ec9a-4f34-a5d5-42629c76fbb2@vondra.me>
Archived-At: <https://www.postgresql.org/message-id/Z24mZZAPKkw02oy2%40paquier.xyz>
Precedence: bulk


--rAVWtuanTv+xcdc0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Dec 26, 2024 at 06:58:11PM +0100, Tomas Vondra wrote:
> If 128MB is insufficient, why would 256MB be OK? A factor of 2x does not
> make a fundamental difference ...
>=20
> Anyway, the 128MB value is rather arbitrary. I don't mind increasing the
> limit, or possibly removing it entirely (and accepting anything the
> system can handle).

+    DefineCustomIntVariable("stats_history.size",
+                            "Sets the amount of memory available for past =
events.",

How about some time-based retention?  Data size can be hard to think
about for the end user, while there would be a set of users that would
want to retain data for the past week, month, etc?  If both size and
time upper-bound are define, then entries that match one condition or
the other are removed.

+        checkpoint_log_hook(
+          CheckpointStats.ckpt_start_t,            /* start_time */
+          CheckpointStats.ckpt_end_t,                /* end_time */
+          (flags & CHECKPOINT_IS_SHUTDOWN),        /* is_shutdown */
+          (flags & CHECKPOINT_END_OF_RECOVERY),    /* is_end_of_recovery */
+          (flags & CHECKPOINT_IMMEDIATE),            /* is_immediate */
+          (flags & CHECKPOINT_FORCE),                /* is_force */
+          (flags & CHECKPOINT_WAIT),                /* is_wait */
+          (flags & CHECKPOINT_CAUSE_XLOG),        /* is_wal */
+          (flags & CHECKPOINT_CAUSE_TIME),        /* is_time */
+          (flags & CHECKPOINT_FLUSH_ALL),            /* is_flush_all */
+          CheckpointStats.ckpt_bufs_written,        /* buffers_written */
+          CheckpointStats.ckpt_slru_written,        /* slru_written */
+          CheckpointStats.ckpt_segs_added,        /* segs_added */
+          CheckpointStats.ckpt_segs_removed,        /* segs_removed */
+          CheckpointStats.ckpt_segs_recycled,        /* segs_recycled */

That's a lot of arguments.  CheckpointStatsData and the various
CHECKPOINT_* flags are exposed, why not just send these values to the
hook?

For v1-0001 as well, I'd suggest some grouping with existing
structures, or expose these structures so as they can be reused for
out-of-core code via the proposed hook.  More arguments lead to more
mistakes that could be easily avoided.
--
Michael

--rAVWtuanTv+xcdc0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmduJmUACgkQnvQgOdby
QH04RA//Sbq3PwunCpCyzkTb31PRfEpXDryyW6zpBlryJ+fNj62qhQQk61IH1mRf
8WkJBFlfrifNWZaaxzd+rQj34dpX/0R1GZm+IohJhgiaVNaDSb4z3LAwLFqACNBS
s5Y8laahhbAjui4L1KNoU7bV6wN8gnDgjeZXjtddtjyAwwpKtHajoqJw02MTjuxG
7CfI7wco3vdoBtz5URqthGqtkiuP90jN1sc/VjS7+HX3emkugAc0VBD7t+Ns4Hxe
i432nA29H6C2ZWFRM/v1LNt5QAdt69vnfqNN2eANK0CrH3kLQjTr1FZB9WZiDZd1
UzEdpD7rd94B0I1iyIEdoTHH0ud6FWCmEi7NkbA1HquOMzvxRINgHy8fgFsewBgu
3a63Fe+aMq4C7f6sZIM+upAHTWgbQl6LS7r3ZvsQwcWOBQOEk/mkS8BhbqtnhjHm
UzZTau8M7RVpX2fxijRAPESLuiQR7eFOepW83qjX84TMmMpxdgEqxAlgwyIFVd9N
qBrn7ZUpUiADhpp7sbpzsLk62FhLIFD30jOQsylvQNgTI5VxjbhJcSHbhc6ZQrOw
MfpYfMx2qy94Szcah5YrMs3tLdGuuwaHzrNHay7HnlbIThh22c5TaV8idl0K6lrE
7YJuoPupbusr5nCoiy1Ppr3OOAd7rmjx7ZKdXYXJP2A3/egWrZo=
=ZpxG
-----END PGP SIGNATURE-----

--rAVWtuanTv+xcdc0--