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.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato
