Re: heap_sync seems rather oblivious to partitioned tables(wal_level=minimal) - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: heap_sync seems rather oblivious to partitioned tables(wal_level=minimal)
Date
Msg-id 20180919030647.GG1650@paquier.xyz
Whole thread Raw
In response to Re: heap_sync seems rather oblivious to partitioned tables (wal_level=minimal)  (David Rowley <david.rowley@2ndquadrant.com>)
Responses Re: heap_sync seems rather oblivious to partitioned tables(wal_level=minimal)  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Wed, Sep 19, 2018 at 01:14:10PM +1200, David Rowley wrote:
> On 19 September 2018 at 12:21, Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> So apparently CopyFrom() invokes heap_sync() on the partitioned
>> relation, which attempts to do mdimmedsync() only on the root. That
>> seems like a bug to me.

And the root should not have any physical storage either, so attempting
to sync it is wrong.

>> Obviously this only applies to wal_level=minimal. There are multiple
>> callers of heap_sync(), but only the COPY seems to be affected by this,
>> because the rest can't see partitioned tables.
>>
>> So it seems heap_sync() needs to be improved to sync all partitions.
>
> Wouldn't it be better to modify copy.c to just perform the heap_sync
> on just the partitions it touches?

Yeah, my gut is telling me that this would be the best approach for now,
still I am not sure that this is the best move in the long term.  All
the other callers of heap_sync don't care about partitioned tables, so
we could add an assertion on RELKIND_PARTITIONED_TABLE.  Still, one
crazy argument against that approach would be that we'd need to do the
same thing again if one day CTAS or matviews allow some sort of
automatic creation of partitions.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: pgsql: Allow concurrent-safe open() and fopen() in frontend codefor Wi
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: when set track_commit_timestamp on, database system abortstartup