Re: [PATCH] pg_stat_toast v0.4

Am 21.12.21 um 13:51 schrieb kuroda(dot)hayato(at)fujitsu(dot)com:
> Dear Gunnar,

Hayato-san, all,

thanks for the feedback!

>> * gathering timing information
>
> Here is a minor comment:
> I'm not sure when we should start to measure time.
> If you want to record time spent for compressing, toast_compress_datum() should be
> sandwiched by INSTR_TIME_SET_CURRENT() and caclurate the time duration.
> Currently time_spent is calcurated in the pgstat_report_toast_activity(),
> but this have a systematic error.
> If you want to record time spent for inserting/updating, however,
> I think we should start measuring at the upper layer, maybe heap_toast_insert_or_update().

Yes, both toast_compress_datum() and toast_save_datum() are sandwiched
the way you mentioned, as that's exactly what we want to measure (time
spent doing compression and/or externalizing data).

Implementation-wise, I (again) took "track_functions" as a template
there, assuming that jumping into pgstat_report_toast_activity() and
only then checking if "track_toast = on" isn't too costly (we call
pgstat_init_function_usage() and pgstat_end_function_usage() a _lot_).

I did miss though that
INSTR_TIME_SET_CURRENT(time_spent);
should be called right after entering pgstat_report_toast_activity(), as
that might need additional clock ticks for setting up the hash etc.
That's fixed now.

What I can't assess is the cost of the unconditional call to
INSTR_TIME_SET_CURRENT(start_time) in both toast_tuple_try_compression()
and toast_tuple_externalize().

Would it be wise (cheaper) to add a check like
if (pgstat_track_toast)
before querying the system clock?

>> Any hints on how to write meaningful tests would be much appreciated!
>> I.e., will a test
>
> I searched hints from PG sources, and I thought that modules/ subdirectory can be used.
> Following is the example:
> https://github.com/postgres/postgres/tree/master/src/test/modules/commit_ts
>
> I attached a patch to create a new test. Could you rewrite the sql and the output file?

Thanks for the kickstart ;-)

Some tests (as meaningful as they may get, I'm afraid) are now in
src/test/modules/track_toast/.
"make check-world" executes them successfully, although only after I
introduced a "SELECT pg_sleep(1);" to them.

pg_stat_toast_v0.4.patch attached.

Best regards,
--
Gunnar "Nick" Bluth

Eimermacherweg 106
D-48159 Münster

Mobil +49 172 8853339
Email: gunnar(dot)bluth(at)pro-open(dot)de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato