Re: File based Incremental backup v8

Hi,

I've attached an updated version of the patch. This fixes the issue on
checksum calculation for segments after the first one.

To solve it I've added an optional uint32 *segno argument to
parse_filename_for_nontemp_relation, so I can know the segment number
and calculate the block number correctly.

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.
>

I've named it copydir_set_lsn and placed it as static function in
dbcommands.c. This lefts the copydir and copy_file functions in copydir.c
untouched. The copydir function in copydir.c is now unused, while the copy_file
function is still used during unlogged tables reinit.

> 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.  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.
>

I've investigate a bit and I have not been able to find any problem here.

> 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.
>

As already said the copydir-LSN patch should be treated as a "temporary"
until a proper WAL logging of CREATE DATABASE and ALTER DATABASE SET
TABLESPACE will be implemented. At that time we could probably get rid
of the whole copydir.[ch] file moving the copy_file function inside reinit.c

Regards,
Marco

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