Re: block-level incremental backup

From: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: block-level incremental backup
Date: 2019-08-16 10:23:35
Message-ID: CAM2+6=Un9uV29uzTXMGgEG47dhy2m4y0Ff1nztahFu4Y16Gu=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 2, 2019 at 6:43 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:

> Some comments:
> 1) There will be some link files created for tablespace, we might
> require some special handling for it
>

Yep. I have that in my ToDo.
Will start working on that soon.

> 2)
> Retry functionality is hanlded only for copying of full files, should
> we handle retry for copying of partial files
> 3)
> we can have some range for maxretries similar to sleeptime
>

I took help from pg_standby code related to maxentries and sleeptime.

However, as we don't want to use system() call now, I have
removed all this kludge and just used fread/fwrite as discussed.

> 4)
> Should we check for malloc failure
>

Used pg_malloc() instead. Same is also suggested by Ibrar.

>
> 5) Should we add display of progress as backup may take some time,
> this can be added as enhancement. We can get other's opinion on this.
>

Can be done afterward once we have the functionality in place.

>
> 6)
> If the backup count increases providing the input may be difficult,
> Shall user provide all the incremental backups from a parent folder
> and can we handle the ordering of incremental backup internally
>

I am not sure of this yet. We need to provide the tablespace mapping too.
But thanks for putting a point here. Will keep that in mind when I revisit
this.

>
> 7)
> Add verbose for copying whole file
>
Done

>
> 8) We can also check if approximate space is available in disk before
> starting combine backup, this can be added as enhancement. We can get
> other's opinion on this.
>

Hmm... will leave it for now. User will get an error anyway.

>
> 9)
> Combine backup into directory can be combine backup directory
>
Done

>
> 10)
> MAX_INCR_BK_COUNT can be increased little, some applications use 1
> full backup at the beginning of the month and use 30 incremental
> backups rest of the days in the month
>

Yeah, agree. But using any number here is debatable.
Let's see others opinion too.

> Regards,
> Vignesh
> EnterpriseDB: http://www.enterprisedb.com
>

Attached new sets of patches with refactoring done separately.
Incremental backup patch became small now and hopefully more
readable than the first version.

--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Attachment Content-Type Size
0001-Add-support-for-command-line-option-to-pass-LSN.patch text/x-patch 6.7 KB
0004-Add-support-to-combine-files-using-pg_combinebackup.patch text/x-patch 32.2 KB
0003-Add-support-for-the-incremental-backup.patch text/x-patch 19.1 KB
0002-Refactor-code-in-basebackup.c.patch text/x-patch 10.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeevan Chalke 2019-08-16 10:42:52 Re: block-level incremental backup
Previous Message Dilip Kumar 2019-08-16 08:47:19 Re: POC: Cleaning up orphaned files using undo logs