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

On Wed, Mar 27, 2019 at 3:38 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Mar 21, 2019 at 10:40 PM Pavan Deolasee
> <pavan.deolasee@gmail.com> wrote:
> >
> > Hi,
> >
> > On Thu, Mar 21, 2019 at 3:10 AM Shaun Thomas <shaun.thomas@2ndquadrant.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
compressionanyways when  compress_tuple_target is higher than toast_tuple_target. May be some discussion in the
paragraphrelated 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.
>

Marked.

Regards,

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