Re: File based Incremental backup v8

From: Marco Nenciarini <marco(dot)nenciarini(at)2ndquadrant(dot)it>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Gabriele Bartolini <gabriele(dot)bartolini(at)2ndquadrant(dot)it>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: File based Incremental backup v8
Date: 2015-02-12 13:50:21
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


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(dot)nenciarini(at)2ndquadrant(dot)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
> - 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


Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco(dot)nenciarini(at)2ndQuadrant(dot)it |

Attachment Content-Type Size
0001-public-parse_filename_for_nontemp_relation.patch text/plain 4.4 KB
0002-file-based-incremental-backup-v9.patch text/plain 47.0 KB
0003-copydir-LSN-v3.patch text/plain 16.5 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-02-12 13:52:56 Re: Manipulating complex types as non-contiguous structures in-memory
Previous Message Heikki Linnakangas 2015-02-12 13:35:11 Re: SSL renegotiation and other related woes