Re: Re: A separate table level option to control compression

On Thu, Mar 21, 2019 at 10:40 PM Pavan Deolasee
<pavan(dot)deolasee(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Mar 21, 2019 at 3:10 AM Shaun Thomas <shaun(dot)thomas(at)2ndquadrant(dot)com> wrote:
>>
>>
>> I can't really speak for the discussion related to `storage.sgml`, but
>> I based my investigation on the existing patch to `create_table.sgml`.
>> About the only thing I would suggest there is to possibly tweak the
>> wording.
>>
>> * "The compress_tuple_target ... " for example should probably read
>> "The toast_tuple_target parameter ...".
>> * "the (blocksize - header)" can drop "the".
>> * "If the value is set to a value" redundant wording should be rephrased;
>> "If the specified value is greater than toast_tuple_target, then
>> we will substitute the current setting of toast_tuple_target instead."
>> would work.
>
>
> Thanks Shaun. Attached patch makes these adjustments.
>
>>
>> * I'd recommend a short discussion on what negative consequences can be
>> expected by playing with this value. As an example in my tests, setting
>> it very high may result in extremely sparse pages that could have an
>> adverse impact on HOT updates.
>
>
> Setting compress_tuple_target to a higher value won't be negative because the toast_tuple_target is used for compression anyways when compress_tuple_target is higher than toast_tuple_target. May be some discussion in the paragraph related to toast_tuple_target can be added to explain the negative impact of the high value.
>
> I added a small discussion about negative effects of setting compress_tuple_target lower though, per your suggestion.
>
> Also added some details in storage.sgml as recommended by Sawada-san. Hope this helps.
>

Thank you for updating the patch!

The patch looks good to me but the <para> of the following change
seems to break indents a bit. Please check it.

+ <term><literal>compress_tuple_target</literal>
(<type>integer</type>)</term>
+ <listitem>
+ <para>
+ The compress_tuple_target parameter specifies the minimum tuple
length required before
+ we try to compress columns marked as Extended or Main and applies only to
+ new tuples - there is no effect on existing rows.
+ By default this parameter is set to allow at least 4 tuples per block,
+ which with the default blocksize will be 2040 bytes. Valid values are
+ between 128 bytes and (blocksize - header), by default 8160 bytes.
+ If the specified value is greater than
+ <literal>toast_tuple_target</literal>, then
+ we will use the current setting of <literal>toast_tuple_target</literal>
+ for <literal>compress_tuple_target</literal>.
+ Note that the default setting is often close to optimal. If the value is
+ set too low then the <acronym>TOAST</acronym> may get invoked too often.
+ If the compressibility of
+ the field values is not good, then compression and decompression can add
+ significant computation overhead without corresponding savings in storage
+ consumption.
+ </para>
+ <para>
+ This parameter cannot be set for TOAST tables.
+ </para>
+ </listitem>
+ </varlistentry>

If there is no comment from other reviews, I'll mark this patch as
Ready for Committer.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center