Date: Tue, 5 Apr 2022 18:45:44 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@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: <YkwPyAoTq/KmTX2B@paquier.xyz>
References: 
 <CAE9k0Png9tjW5M7wjMKzEoOqkHS-VToP9yC2pr=HX7aQ0yQRNA@mail.gmail.com>
 <CAMm1aWa8p=o60y1Z71bjAjWH8XH2Y+kZ6cNjQ21uK2_i65soOQ@mail.gmail.com>
 <CALj2ACUNnqY7oc5C5yGS1ZbHV1RtQgPGiMkPTm989czCDwag6g@mail.gmail.com>
 <CAEze2Wh_+P9Hs4RQ=tc=70rdB5h0omGx5Kz-BLhsJPRnKQ+cWw@mail.gmail.com>
 <CAMm1aWbuJLEnAjT4_1s8eArmZq=H7oi0Jv=J2GnRpBhkbC_PVw@mail.gmail.com>
 <CAMm1aWYWmqys2O7UekQ0=ZSWSpG9h2qoEW7w46yiWM0eO1Lu7g@mail.gmail.com>
 <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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="VMa5z6qkzTF7AJUE"
Content-Disposition: inline
In-Reply-To: <20220319001556.atuqbmafvc73pa2k@alap3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/YkwPyAoTq/KmTX2B%40paquier.xyz>
Precedence: bulk


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

On Fri, Mar 18, 2022 at 05:15:56PM -0700, Andres Freund wrote:
> Have you measured the performance effects of this? On fast storage with l=
arge
> shared_buffers I've seen these loops in profiles. It's probably fine, but=
 it'd
> be good to verify that.

I am wondering if we could make the function inlined at some point.
We could also play it safe and only update the counters every N loops
instead.

> This view is depressingly complicated. Added up the view definitions for
> the already existing pg_stat_progress* views add up to a measurable part =
of
> the size of an empty database:

Yeah.  I think that what's proposed could be simplified, and we had
better remove the fields that are not that useful.  First, do we have=20
any need for next_flags?  Second, is the start LSN really necessary
for monitoring purposes?  Not all the information in the first
parameter is useful, as well.  For example "shutdown" will never be=20
seen as it is not possible to use a session at this stage, no?  There
is also no gain in having "immediate", "flush-all", "force" and "wait"
(for this one if the checkpoint is requested the session doing the
work knows this information already).

A last thing is that we may gain in visibility by having more
attributes as an effect of splitting param2.  On thing that would make
sense is to track the reason why the checkpoint was triggered
separately (aka wal and time).  Should we use a text[] instead to list
all the parameters instead?  Using a space-separated list of items is
not intuitive IMO, and callers of this routine will likely parse
that.

Shouldn't we also track the number of files flushed in each sub-step?
In some deployments you could have a large number of 2PC files and
such.  We may want more information on such matters.

+                      WHEN 3 THEN 'checkpointing replication slots'
+                      WHEN 4 THEN 'checkpointing logical replication snaps=
hot files'
+                      WHEN 5 THEN 'checkpointing logical rewrite mapping f=
iles'
+                      WHEN 6 THEN 'checkpointing replication origin'
+                      WHEN 7 THEN 'checkpointing commit log pages'
+                      WHEN 8 THEN 'checkpointing commit time stamp pages'
There is a lot of "checkpointing" here.  All those terms could be
shorter without losing their meaning.

This patch still needs some work, so I am marking it as RwF for now.
--
Michael

--VMa5z6qkzTF7AJUE
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmJMD8gACgkQnvQgOdby
QH0iQg//SLGakAR6mRMXn3gkF4dXGh1OgXCWVM2Kk6Ro5gPpNUB7CCJsRsEdsHr4
DLks3GK7oQPwDh8hOyeXpfoMyfp8b1PfB+E/oMBRMQioObRi9Ja3ZY1xJg6iTSiq
FoHe1ZxfiLUwfotcrsZlj002rPzn4Dat1Rlffc7EUC/4FvP8dbMAheJWydTL/WFt
J5tOQIXbUqqRYUrj3W3X1ZoxnqaIMEHZvWWo5R3wKIOEWMV41sucPTQ0z1ph+9T2
aBShX+RxBk6Kt+F5+XoV1ppuD/5m/FTohIb1jBO8rc5c7BAA1O5Qt4BYtQ9NYP3M
NKBkiTY5qOtyF/h/MoroDWVTKuLdAQsMZeRfvAcURwtLPSG7Ha3giolrYs/Jziz1
gAXZuRyEsu/1rdtx3iZZDT5WygRx1vs0S/g6vBmGhOtk5L72cxBCOwGSpG/wlfQO
Rghu0qYz4d6nCeRXdloHCUL8K3Yb3Z46+280jIM5lsl8u+DLHMvQGWhJJhTJL2yM
PLD/w7r4oKoxxBs7PeDfOTMOHGrPjhrXpO3eurjZOArXzUZvlZhIj5jONWxMBAsZ
gyv5QjJgKeIR5G6DeKM1y0cIBHNJ83ZJhu7eQjZ7z1B2pEVaKxxuVHcyndKJj8Mu
8nE64dHW7f3Ey+QnArxI/vCVfr+boOOWecOEvct3cTxeNEMUC6I=
=gyq2
-----END PGP SIGNATURE-----

--VMa5z6qkzTF7AJUE--