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 |
Message-ID: | 54DCAF9D.9040002@2ndquadrant.it |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(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
> 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(dot)nenciarini(at)2ndQuadrant(dot)it | www.2ndQuadrant.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 |
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 |