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

On Wed, Mar 27, 2019 at 3:38 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> 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.
>

Marked.

Regards,

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