Re: [PATCH] pg_stat_toast v0.4

Am 21.12.21 um 13:51 schrieb kuroda.hayato@fujitsu.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.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato