Re: WIP: new system catalog pg_wait_event - Mailing list pgsql-hackers
From | Drouvot, Bertrand |
---|---|
Subject | Re: WIP: new system catalog pg_wait_event |
Date | |
Msg-id | b71d1977-1a8b-4129-8c03-533031dbd004@gmail.com Whole thread Raw |
In response to | Re: WIP: new system catalog pg_wait_event (Michael Paquier <michael@paquier.xyz>) |
Responses |
Re: WIP: new system catalog pg_wait_event
|
List | pgsql-hackers |
Hi, On 8/9/23 9:56 AM, Michael Paquier wrote: > 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; > Thanks for looking at it! Right, so v4 attached is just testing "SELECT count(*) > 0 FROM pg_wait_event;", that does look enough to test. > + 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? > Yeah, agree thanks! In v4, the perl script now appends the wait events in a List that way: " printf $ic "\telement = (wait_event_element *) palloc(sizeof(wait_event_element));\n"; printf $ic "\telement->wait_event_type = \"%s\";\n", $last; printf $ic "\telement->wait_event_name = \"%s\";\n", $wev->[1]; printf $ic "\telement->wait_event_description = \"%s\";\n\n", $new_desc; printf $ic "\twait_event = lappend(wait_event, element);\n\n"; " And the C function pg_get_wait_events() now iterates over this List. > + 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, good idea... I had to turn them "on" one by one to recall why they are there...;-) Done in v4. > the contents produced look good. yeap > > + 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? That sounds better indeed, thanks! v4 is using wait_event_funcs.c and 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? Agree that's worth it given the fact that iterating one more time is not that costly here. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
pgsql-hackers by date: