Обсуждение: pgsql: Map basebackup tablespaces using a tablespace_map file
Map basebackup tablespaces using a tablespace_map file Windows can't reliably restore symbolic links from a tar format, so instead during backup start we create a tablespace_map file, which is used by the restoring postgres to create the correct links in pg_tblspc. The backup protocol also now has an option to request this file to be included in the backup stream, and this is used by pg_basebackup when operating in tar mode. This is done on all platforms, not just Windows. This means that pg_basebackup will not not work in tar mode against 9.4 and older servers, as this protocol option isn't implemented there. Amit Kapila, reviewed by Dilip Kumar, with a little editing from me. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/72d422a5227ef6f76f412486a395aba9f53bf3f0 Modified Files -------------- doc/src/sgml/backup.sgml | 32 +-- doc/src/sgml/func.sgml | 14 +- doc/src/sgml/protocol.sgml | 15 +- doc/src/sgml/ref/pg_basebackup.sgml | 14 +- src/backend/access/transam/xlog.c | 387 ++++++++++++++++++++++++++++++-- src/backend/access/transam/xlogfuncs.c | 12 +- src/backend/replication/basebackup.c | 138 +++++------- src/backend/replication/repl_gram.y | 16 +- src/backend/replication/repl_scanner.l | 1 + src/bin/pg_basebackup/pg_basebackup.c | 5 +- src/include/access/xlog.h | 9 +- src/include/replication/basebackup.h | 10 + 12 files changed, 519 insertions(+), 134 deletions(-)
On 05/12/2015 04:42 PM, Andrew Dunstan wrote:
> +
> + /*
> + * Remove the existing symlink if any and Create the symlink
> + * under PGDATA. We need to use rmtree instead of rmdir as
> + * the link location might contain directories or files
> + * corresponding to the actual path. Some tar utilities do
> + * things that way while extracting symlinks.
> + */
> + if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
> + {
> + if (!rmtree(linkloc,true))
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not remove directory \"%s\": %m",
> + linkloc)));
> + }
> + else
> + {
> + if (unlink(linkloc) < 0 && errno != ENOENT)
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not remove symbolic link \"%s\": %m",
> + linkloc)));
> + }
> +
That's scary. What tar utilitiy replaces the symlink with a non-empty
directory?
What if you call pg_start_backup() and take the backup with a utility
that follows symlinks? I wouldn't recommend using such a tool, but with
this patch, it will zap all the tablespaces. Before this, you at least
got a working database and could read out all the data or fix the
symlinks afterwards.
Why is the RelationCacheInitFileRemove() call moved?
> XLogRecPtr
> do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
> - char **labelfile)
> + char **labelfile, DIR *tblspcdir, List **tablespaces,
> + char **tblspcmapfile, bool infotbssize,
> + bool needtblspcmapfile)
Why does this need to take tblspcdir as argument? Wouldn't it make more
sense to open the directory inside do_pg_start_backup?
In the BASE_BACKUP command, is there any reason to not always include
the tablespace map? IOW, do we really need the TABLESPACE_MAP option?
- Heikki
On 05/12/2015 10:33 AM, Heikki Linnakangas wrote:
> On 05/12/2015 04:42 PM, Andrew Dunstan wrote:
>> +
>> + /*
>> + * Remove the existing symlink if any and Create the
>> symlink
>> + * under PGDATA. We need to use rmtree instead of
>> rmdir as
>> + * the link location might contain directories or files
>> + * corresponding to the actual path. Some tar
>> utilities do
>> + * things that way while extracting symlinks.
>> + */
>> + if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
>> + {
>> + if (!rmtree(linkloc,true))
>> + ereport(ERROR,
>> + (errcode_for_file_access(),
>> + errmsg("could not remove directory
>> \"%s\": %m",
>> + linkloc)));
>> + }
>> + else
>> + {
>> + if (unlink(linkloc) < 0 && errno != ENOENT)
>> + ereport(ERROR,
>> + (errcode_for_file_access(),
>> + errmsg("could not remove symbolic
>> link \"%s\": %m",
>> + linkloc)));
>> + }
>> +
>
> That's scary. What tar utilitiy replaces the symlink with a non-empty
> directory?
>
> What if you call pg_start_backup() and take the backup with a utility
> that follows symlinks? I wouldn't recommend using such a tool, but
> with this patch, it will zap all the tablespaces. Before this, you at
> least got a working database and could read out all the data or fix
> the symlinks afterwards.
>
> Why is the RelationCacheInitFileRemove() call moved?
I will let Amit comment on this.
>
>> XLogRecPtr
>> do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID
>> *starttli_p,
>> - char **labelfile)
>> + char **labelfile, DIR *tblspcdir, List **tablespaces,
>> + char **tblspcmapfile, bool infotbssize,
>> + bool needtblspcmapfile)
>
> Why does this need to take tblspcdir as argument? Wouldn't it make
> more sense to open the directory inside do_pg_start_backup?
Good point. It does look cleaner to do that.
>
> In the BASE_BACKUP command, is there any reason to not always include
> the tablespace map? IOW, do we really need the TABLESPACE_MAP option?
>
>
>
I assume it was done for minimal disturbance, i.e. that's where we need
it. Amit, comments?
cheers
andrew
On Tue, May 12, 2015 at 9:02 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
On 05/12/2015 10:33 AM, Heikki Linnakangas wrote:On 05/12/2015 04:42 PM, Andrew Dunstan wrote:+
+ /*
+ * Remove the existing symlink if any and Create the symlink
+ * under PGDATA. We need to use rmtree instead of rmdir as
+ * the link location might contain directories or files
+ * corresponding to the actual path. Some tar utilities do
+ * things that way while extracting symlinks.
+ */
+ if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
+ {
+ if (!rmtree(linkloc,true))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove directory \"%s\": %m",
+ linkloc)));
+ }
+ else
+ {
+ if (unlink(linkloc) < 0 && errno != ENOENT)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove symbolic link \"%s\": %m",
+ linkloc)));
+ }
+
That's scary. What tar utilitiy replaces the symlink with a non-empty directory?
IIRC, it was star utility by using --copysymlinks options.
It will actually copy the symlink data at appropriate location,
but will not maintain symlink after extraction.
I don't have a link handly for it, but can again search for
it and send you if you want to have a look.
What if you call pg_start_backup() and take the backup with a utility that follows symlinks? I wouldn't recommend using such a tool, but with this patch, it will zap all the tablespaces. Before this, you at least got a working database and could read out all the data or fix the symlinks afterwards.
Yeah, but I don't think user will do such a thing without
being aware of the same and if he is aware, he will either
restore the symlinks before starting the server or would
atleast keep a copy of such a backup until he is able to
restore the database completely.
Do you think adding a note in docs makes sense?
Why is the RelationCacheInitFileRemove() call moved?
I will let Amit comment on this.
Because it assumes that tablespace directory pg_tblspc is all in
place and it tries to remove the files by reading pg_tblspc
directory as well. Now as we setup the symlinks in pg_tblspc
after reading tablespace_map file, so we should remove relcache init
file once the symlinks are setup in pg_tblspc directory.
XLogRecPtr
do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
- char **labelfile)
+ char **labelfile, DIR *tblspcdir, List **tablespaces,
+ char **tblspcmapfile, bool infotbssize,
+ bool needtblspcmapfile)
Why does this need to take tblspcdir as argument?
It needs to be called from perform_base_backup() which already
has access to tblspcdir.
Wouldn't it make more sense to open the directory inside do_pg_start_backup?
Good point. It does look cleaner to do that.
If you prefer that way, I can look into rearranging the
code.
In the BASE_BACKUP command, is there any reason to not always include the tablespace map?
The main reason is that it is not required to restore symlinks
for plain format. Also I think it can cause a problem if user
has used --tablespace-mapping option in plain format as the
tablespace_map file doesn't contain information for same and recovery
will try to restore symlinks for wrong path based on tablespace_map
file.