Re: PATCH: Exclude additional directories in pg_basebackup

From: David Steele <david(at)pgmasters(dot)net>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Subject: Re: PATCH: Exclude additional directories in pg_basebackup
Date: 2016-09-23 18:26:42
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 9/12/16 2:50 PM, Peter Eisentraut wrote:
> The comments on excludeDirContents are somewhat imprecise. For
> example, none of those directories are actually removed or recreated
> on startup, only the contents are.


> The comment for pg_replslot is incorrect. I think you can copy
> replication slots just fine, but you usually don't want to.


> What is the 2 for in this code?
> if (strcmp(pathbuf + 2, excludeFile[excludeIdx]) == 0)

This should be basepathlen + 1 (i.e., $PGDATA/). Fixed.

> I would keep the signature of _tarWriteDir() similar to
> _tarWriteHeader(). So don't pass sizeonly in there, or do pass in
> sizeonly but do the same with _tarWriteHeader().

I'm not sure how much more similar they can be, but I do agree that
sizeonly logic should be wrapped in _tarWriteHeader(). Done.

> The documentation in pg_basebackup.sgml and protocol.sgml should be
> updated.

Done. I moved a few things to the protocol docs to avoid repetition.

> Add some tests. At least test that one entry from the directory list
> and one entry from the files list is not contained in the backup
> result.

Done for all files and directories.

> Testing the symlink handling would also be good.

Done using pg_replslot for the test.

> (The
> pg_basebackup tests claim that Windows doesn't support symlinks and
> therefore skip all the symlink tests. That seems a bit at odds with
> your code handling symlinks on Windows.)

Hopefully Michael's response down-thread answered your question.


Attachment Content-Type Size
basebackup-exclusions-v4.patch text/plain 20.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-09-23 19:10:20 Re: Rename max_parallel_degree?
Previous Message David Steele 2016-09-23 18:19:23 Re: pg_basebackup creates a corrupt file for pg_stat_tmp and pg_replslot on a backup location