Re: File based Incremental backup v8

Il 29/01/15 18:57, Robert Haas ha scritto:
> On Thu, Jan 29, 2015 at 9:47 AM, Marco Nenciarini
> <marco.nenciarini@2ndquadrant.it> wrote:
>> The current implementation of copydir function is incompatible with LSN
>> based incremental backups. The problem is that new files are created,
>> but their blocks are still with the old LSN, so they will not be backed
>> up because they are looking old enough.
>
> I think this is trying to pollute what's supposed to be a pure
> fs-level operation ("copy a directory") into something that is aware
> of specific details like the PostgreSQL page format.  I really think
> that nothing in storage/file should know about the page format.  If we
> need a function that copies a file while replacing the LSNs, I think
> it should be a new function living somewhere else.

Given that the copydir function is used only during CREATE DATABASE and
ALTER DATABASE SET TABLESPACE, we could move it/renaming it to a better
place that clearly mark it as "knowing about page format". I'm open to
suggestions on where to place it an on what should be the correct name.

However the whole copydir patch here should be treated as a "temporary"
thing. It is necessary until a proper WAL logging of CREATE DATABASE and
ALTER DATABASE SET TABLESPACE will be implemented to support any form of
LSN based incremental backup.

>
> A bigger problem is that you are proposing to stamp those files with
> LSNs that are, for lack of a better word, fake.  I would expect that
> this would completely break if checksums are enabled.

I'm sorry I completely ignored checksums in previous patch. The attached
one works with checksums enabled.

> Also, unlogged relations typically have an LSN of 0; this would
> change that in some cases, and I don't know whether that's OK.
>

It shouldn't be a problem because all the code that uses unlogged
relations normally skip all the WAL related operations. From the point
of view of an incremental backup it is also not a problem, because
restoring the backup the unlogged tables will get reinitialized because
of crash recovery procedure. However if you think it is worth the
effort, I can rewrite the copydir as a two pass operation detecting the
unlogged tables on the first pass and avoiding the LSN update on
unlogged tables. I personally think that it doesn't wort the effort
unless someone identify a real path where settins LSNs in unlogged
relations leads to an issue.

> The issues here are similar to those in
> http://www.postgresql.org/message-id/20150120152819.GC24381@alap3.anarazel.de
> - basically, I think we need to make CREATE DATABASE and ALTER
> DATABASE .. SET TABLESPACE fully WAL-logged operations, or this is
> never going to work right.  If we're not going to allow that, we need
> to disallow hot backups while those operations are in progress.
>

This is right, but the problem Andres reported is orthogonal with the
one I'm addressing here. Without this copydir patch (or without a proper
WAL logging of copydir operations), you cannot take an incremental
backup after a CREATE DATABASE or ALTER DATABASE SET TABLESPACE until
you get a full backup and use it as base.

Regards,
Marco

--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciarini@2ndQuadrant.it | www.2ndQuadrant.it