Re: [PATCH] pg_stat_toast v8

Am 04.01.22 um 12:29 schrieb Gunnar "Nick" Bluth:
> Am 03.01.22 um 22:23 schrieb Alvaro Herrera:
>> Overall I think this is a good feature to have; assessing the need for
>> compression is important for tuning, so +1 for the goal of the patch.
>
> Much appreciated!
>
>
>> I didn't look into the patch carefully, but here are some minor
>> comments:
>>
>> On 2022-Jan-03, Gunnar "Nick" Bluth wrote:
>>
>>> @@ -229,7 +230,9 @@ toast_tuple_try_compression(ToastTupleContext
>>> *ttc, int attribute)
>>>       Datum       *value = &ttc->ttc_values[attribute];
>>>       Datum        new_value;
>>>       ToastAttrInfo *attr = &ttc->ttc_attr[attribute];
>>> +    instr_time    start_time;
>>>   +    INSTR_TIME_SET_CURRENT(start_time);
>>>       new_value = toast_compress_datum(*value, attr->tai_compression);
>>>         if (DatumGetPointer(new_value) != NULL)
>>
>> Don't INSTR_TIME_SET_CURRENT unconditionally; in some systems it's an
>> expensive syscall.  Find a way to only do it if the feature is enabled.
>
> Yeah, I was worried about that (and asking if it would be required)
> already.
> Adding the check was easier than I expected, though I'm absolutely
> clueless if I did it right!
>
> #include "pgstat.h"
> extern PGDLLIMPORT bool pgstat_track_toast;
>

As it works and nobody objected, it seems to be the right way...

v8 (applies cleanly to today's HEAD/master) attached.

Any takers?

--
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