Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 9 Aug 2023 16:56:27 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, tgl@sss.pgh.pa.us,
	pgsql-hackers@lists.postgresql.org
Subject: Re: WIP: new system catalog pg_wait_event
Message-ID: <ZNNGq7DTv9ur5113@paquier.xyz>
References: <3658241.1691161701@sss.pgh.pa.us>
 <0109438a-9672-322b-7638-04ad47c97d0f@gmail.com>
 <1dc98591-0d06-f0da-bc88-21f529d6fb8e@gmail.com>
 <20230808.115332.301701413576096139.horikyota.ntt@gmail.com>
 <ZNGw9T2t5z5Kg1xd@paquier.xyz>
 <b21e5706-b6dd-42f1-a744-3afffe87c2c9@gmail.com>
 <c915434a-3d3a-4135-8f3a-fca0cf22c6ae@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="BCXgd8Rdde1cIW1G"
Content-Disposition: inline
In-Reply-To: <c915434a-3d3a-4135-8f3a-fca0cf22c6ae@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZNNGq7DTv9ur5113%40paquier.xyz>
Precedence: bulk


--BCXgd8Rdde1cIW1G
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Aug 08, 2023 at 10:16:37AM +0200, Drouvot, Bertrand wrote:
> Please find attached v3 adding the wait event types.

+-- There will surely be at least 9 wait event types, 240 wait events and at
+-- least 27 related to WAL
+select count(distinct(wait_event_type)) > 8 as ok_type,
+       count(*) > 239 as ok,
+       count(*) FILTER (WHERE description like '%WAL%') > 26 AS ok_wal_desc
+from pg_wait_event;
The point is to check the execution of this function, so this could be
simpler, like that or a GROUP BY clause with the event type:
SELECT count(*) > 0 FROM pg_wait_event;
SELECT wait_event_type, count(*) > 0 AS has_data FROM pg_wait_event
  GROUP BY wait_event_type ORDER BY wait_event_type;

+           printf $ic "\tmemset(values, 0, sizeof(values));\n";
+           printf $ic "\tmemset(nulls, 0, sizeof(nulls));\n\n";
+           printf $ic "\tvalues[0] = CStringGetTextDatum(\"%s\");\n", $last;
+           printf $ic "\tvalues[1] = CStringGetTextDatum(\"%s\");\n", $wev->[1];
+           printf $ic "\tvalues[2] = CStringGetTextDatum(\"%s\");\n\n", $new_desc;

That's overcomplicated for some code generated.  Wouldn't it be
simpler to generate a list of elements, with the code inserting the
tuples materialized looping over it?

+           my $new_desc = substr $wev->[2], 1, -2;
+           $new_desc =~ s/'/\\'/g;
+           $new_desc =~ s/<.*>(.*?)<.*>/$1/g;
+           $new_desc =~ s/<xref linkend="guc-(.*?)"\/>/$1/g;
+           $new_desc =~ s/; see.*$//;
Better to document what this does, the contents produced look good.

+   rename($ictmp, "$output_path/pg_wait_event_insert.c")
+     || die "rename: $ictmp to $output_path/pg_wait_event_insert.c: $!";

 # seems nicer to not add that as an include path for the whole backend.
 waitevent_sources = files(
   'wait_event.c',
+  'pg_wait_event.c',
 )

This could use a name referring to SQL functions, say
wait_event_funcs.c, with a wait_event_data.c or a
wait_event_funcs_data.c?

+       # Don't generate .c (except pg_wait_event_insert.c) and .h files for
+       # Extension, LWLock and Lock, these are handled independently.
+       my $is_exception = $waitclass eq 'WaitEventExtension' ||
+                          $waitclass eq 'WaitEventLWLock' ||
+                          $waitclass eq 'WaitEventLock';
Perhaps it would be cleaner to use a separate loop?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmTTRqsACgkQnvQgOdby
QH2GaBAAmzRS+wZjjkrE+Mp1ZTOHUhKX/aofYy8uXw93Y6Hd9Wz4nUxqCAHaqkCr
Bnk/QnbCrLBR6oLR/Oo/uTTKnnnzHhBl9kr3+8+77oSs4esR3EwOSY4N30iB/bwm
eUlEd4Tcjy4WfZ0jtgSRd7y+ZKja8/sOp2+q4uEOaUOV6vNiPYqtHjqsoAh0Jaow
WH5Pv1WxHBooVCuNE1WZT+t5dfQy+h/UKUmJt4tkaixOVuT3m/8w/4CSsXhGivjJ
9ND8X5qZJrT7gLsrITo/YGNwvDtDUseANw083kp9udtN9F7w+s0FjZBhw1gp8kg5
4FzzT/D4PWWwx96jjaF2ClCI2v2OKaphvWdyuTU6+YDeeSzUSQJYaC5o63tjMw9B
DpRqSVP0PJ7/mhTnIVU+jCEtMApcklu8Ahh8AHbpNCWcgaurDaNiU2y3AcLM9nOd
P60raSwMzbymvF/82SdAa39t6/jxNTkYB9pndtjnHK06nhT8pWPOT/S2iYNdYYn/
h/5mr/2G6TkmO8Yt4rC9cfsv7ARWxj4VXlf/Ixj12DLopu7ZEPDXoHsZFk5F1OWI
4qrxVYpgwx57Mai2GGbkZOQUAFcUcqgYNgkLIPrH7MPHB0WPWXIjF91AHMel+M+/
YGlsEDbfayb9PQQUTtx+n8YTTd6OFttufNsqi0CFwiVcw7mXibM=
=OTHM
-----END PGP SIGNATURE-----

--BCXgd8Rdde1cIW1G--