From: | "Gunnar \"Nick\" Bluth" <gunnar(dot)bluth(at)pro-open(dot)de> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, kuroda(dot)hayato(at)fujitsu(dot)com |
Subject: | Re: [PATCH] pg_stat_toast v6 |
Date: | 2022-01-04 11:11:23 |
Message-ID: | acf5f31f-e76b-693e-d8ab-b51897c0c6ba@pro-open.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Am 03.01.22 um 22:03 schrieb Justin Pryzby:
>> +pgstat_report_toast_activity(Oid relid, int attr,
>> + bool externalized,
>> + bool compressed,
>> + int32 old_size,
>> + int32 new_size,
> ...
>> + if (new_size)
>> + {
>> + htabent->t_counts.t_size_orig+=old_size;
>> + if (new_size)
>> + {
>
> I guess one of these is supposed to say old_size?
Didn't make a difference, tbth, as they'd both be 0 or have a value.
Streamlined the whole block now.
>> +CREATE TABLE toast_test (cola TEXT, colb TEXT COMPRESSION lz4, colc TEXT , cold TEXT, cole TEXT);
>
> Is there a reason this uses lz4 ?
I thought it might help later on, but alas! the LZ4 column mainly broke
things, so I removed it for the time being.
> If that's needed for stable results, I think you should use pglz, since that's
> what's guaranteed to exist. I imagine LZ4 won't be required any time soon,
> seeing as zlib has never been required.
Yeah. It didn't prove anything whatsoever.
>> + Be aware that this feature, depending on the amount of TOASTable columns in
>> + your databases, may significantly increase the size of the statistics files
>> + and the workload of the statistics collector. It is recommended to only
>> + temporarily activate this to assess the right compression and storage method
>> + for (a) column(s).
>
> saying "a column" is fine
Changed.
>
>> + <structfield>schemaname</structfield> <type>name</type>
>> + Attribute (column) number in the relation
>> + <structfield>relname</structfield> <type>name</type>
>
>> + <entry role="catalog_table_entry"><para role="column_definition">
>> + <structfield>compressmethod</structfield> <type>char</type>
>> + </para>
>> + <para>
>> + Compression method of the attribute (empty means default)
>
> One thing to keep in mind is that the current compression method is only used
> for *new* data - old data can still use the old compression method. It
> probably doesn't need to be said here, but maybe you can refer to the docs
> about that in alter_table.
>
>> + Number of times the compression was successful (gained a size reduction)
>
> It's more clear to say "was reduced in size"
Changed the wording a bit, I guess it is clear enough now.
The question is if the column should be there at all, as it's simply
fetched from pg_attribute...
>
>> + /* we assume this inits to all zeroes: */
>> + static const PgStat_ToastCounts all_zeroes;
>
> You don't have to assume; static/global allocations are always zero unless
> otherwise specified.
Copy-pasta ;-)
Removed.
Thx for looking into this!
Patch v7 will be in the next mail.
--
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
From | Date | Subject | |
---|---|---|---|
Next Message | Gunnar "Nick" Bluth | 2022-01-04 11:29:09 | Re: [PATCH] pg_stat_toast v6 |
Previous Message | Andrey V. Lepikhov | 2022-01-04 10:44:53 | Re: Clarify planner_hook calling convention |