Re: heap_sync seems rather oblivious to partitioned tables(wal_level=minimal)

On 09/28/2018 06:02 AM, Amit Langote wrote:
> On 2018/09/28 12:12, Michael Paquier wrote:
>> On Fri, Sep 28, 2018 at 02:46:30PM +1200, David Rowley wrote:
>>> I don't agree that we can skip explaining why one of the optimisations
>>> can't be applied just because we've explained why a similar
>>> optimisation cannot be applied somewhere close by.  I think that the
>>> WAL/FSM optimisation can fairly easily be improved on and probably
>>> fixed in PG12 as we can just lazily determine per-partition if it can
>>> be applied to that partition or not.
>>
>> Have you guys looked at what the following patch does for partitions and
>> how it interacts with it?
>> https://commitfest.postgresql.org/19/528/
> 
> Just looked at that patch and noticed that the following hunk won't cope
> if COPY's target table is partitioned:
> 
> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
> index 7674369613..7b9a7af2d2 100644
> --- a/src/backend/commands/copy.c
> +++ b/src/backend/commands/copy.c
> @@ -2416,10 +2416,8 @@ CopyFrom(CopyState cstate)
>      {
>          hi_options |= HEAP_INSERT_SKIP_FSM;
> 
> -        if (!XLogIsNeeded() &&
> -            cstate->rel->trigdesc == NULL &&
> -            RelationGetNumberOfBlocks(cstate->rel) == 0)
> -            hi_options |= HEAP_INSERT_SKIP_WAL;
> +        if (!XLogIsNeeded() && RelationGetNumberOfBlocks(cstate->rel) == 0)
> +             hi_options |= HEAP_INSERT_SKIP_WAL;
>      }
> 
>      /*
> 
> RelationGetNumberOfBlocks would blow up if passed a partitioned table to it.
> 
> Applying David's patch will take care of that though.
> 
>> The proposed patch is missing the point that documentation also mentions
>> the optimizations for COPY with wal_level = minimal:
>>    <para>
>>     <command>COPY</command> is fastest when used within the same
>>     transaction as an earlier <command>CREATE TABLE</command> or
>>     <command>TRUNCATE</command> command. In such cases no WAL
>>     needs to be written, because in case of an error, the files
>>     containing the newly loaded data will be removed anyway.
>>     However, this consideration only applies when
>>     <xref linkend="guc-wal-level"/> is <literal>minimal</literal> as all commands
>>     must write WAL otherwise.
>>    </para>
> 
> I might be wrong but I'm not sure if we should mention here that this
> optimization is not applied to partitioned tables due to what appears to
> be a implementation-level limitation?
> 

Aren't most limitations implementation-level? ;-)

IMHO it's entirely appropriate to mention this in user-level docs. Based
on the current wording, it's quite natural to assume the optimization
applies to both partitioned and non-partitioned tables. And in fact it
does not. It will still work for individual partitions, though.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services