|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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 | www.2ndQuadrant.it
|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|