On Thu, Mar 21, 2019 at 10:40 PM Pavan Deolasee <pavan.deola...@gmail.com> wrote: > > Hi, > > On Thu, Mar 21, 2019 at 3:10 AM Shaun Thomas <shaun.tho...@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 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
