rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix pg_basebackup with in-place tablespaces some more. 

Commit c6f2f01611 purported to make
this work, but problems remained. In a plain-format backup, the
files from an in-place tablespace got included in the tar file for
the main tablespace, which is wrong but it's not clear that it
has any user-visible consequences. In a tar-format backup, the
TABLESPACE_MAP option is used, and so we never iterated over
pg_tblspc and thus never backed up the in-place tablespaces
anywhere at all.

To fix this, reverse the changes in that commit, so that when we scan
pg_tblspc during a backup, we create tablespaceinfo objects even for
in-place tablespaces. We set the field that would normally contain the
absolute pathname to the relative path pg_tblspc/${TSOID}, and that's
good enough to make basebackup.c happy without any further changes.

However, pg_basebackup needs a couple of adjustments to make it work.
First, it needs to understand that a relative path for a tablespace
means it's an in-place tablespace.  Second, it needs to tolerate the
situation where restoring the main tablespace tries to create
pg_tblspc or a subdirectory and finds that it already exists, because
we restore user-defined tablespaces before the main tablespace.

Since in-place tablespaces are only intended for use in development
and testing, no back-patch.

Patch by me, reviewed by Thomas Munro and Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmobwvbEp+fLq2PykMYzizcvuNv0a7gPMJtxOTMOuuRLMHg@mail.gmail.com
This commit is contained in:
Robert Haas 2023-04-18 11:23:34 -04:00
parent f180290847
commit 363e8f9115
4 changed files with 162 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
src/backend/access/transam/xlog.c
View File

@ -8454,9 +8454,8 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
char fullpath[MAXPGPATH + 10];
char linkpath[MAXPGPATH];
char *relpath = NULL;
int rllen;
StringInfoData escapedpath;
char *s;
PGFileType de_type;
/* Skip anything that doesn't look like a tablespace */
if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
@ -8464,66 +8463,83 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
/*
* Skip anything that isn't a symlink/junction. For testing only,
* we sometimes use allow_in_place_tablespaces to create
* directories directly under pg_tblspc, which would fail below.
*/
if (get_dirent_type(fullpath, de, false, ERROR) != PGFILETYPE_LNK)
continue;
de_type = get_dirent_type(fullpath, de, false, ERROR);
rllen = readlink(fullpath, linkpath, sizeof(linkpath));
if (rllen < 0)
if (de_type == PGFILETYPE_LNK)
{
ereport(WARNING,
(errmsg("could not read symbolic link \"%s\": %m",
fullpath)));
StringInfoData escapedpath;
int rllen;
rllen = readlink(fullpath, linkpath, sizeof(linkpath));
if (rllen < 0)
{
ereport(WARNING,
(errmsg("could not read symbolic link \"%s\": %m",
fullpath)));
continue;
}
else if (rllen >= sizeof(linkpath))
{
ereport(WARNING,
(errmsg("symbolic link \"%s\" target is too long",
fullpath)));
continue;
}
linkpath[rllen] = '\0';
/*
* Relpath holds the relative path of the tablespace directory
* when it's located within PGDATA, or NULL if it's located
* elsewhere.
*/
if (rllen > datadirpathlen &&
strncmp(linkpath, DataDir, datadirpathlen) == 0 &&
IS_DIR_SEP(linkpath[datadirpathlen]))
relpath = pstrdup(linkpath + datadirpathlen + 1);
/*
* Add a backslash-escaped version of the link path to the
* tablespace map file.
*/
initStringInfo(&escapedpath);
for (s = linkpath; *s; s++)
{
if (*s == '\n' || *s == '\r' || *s == '\\')
appendStringInfoChar(&escapedpath, '\\');
appendStringInfoChar(&escapedpath, *s);
}
appendStringInfo(tblspcmapfile, "%s %s\n",
de->d_name, escapedpath.data);
pfree(escapedpath.data);
}
else if (de_type == PGFILETYPE_DIR)
{
/*
* It's possible to use allow_in_place_tablespaces to create
* directories directly under pg_tblspc, for testing purposes
* only.
*
* In this case, we store a relative path rather than an
* absolute path into the tablespaceinfo.
*/
snprintf(linkpath, sizeof(linkpath), "pg_tblspc/%s",
de->d_name);
relpath = pstrdup(linkpath);
}
else
{
/* Skip any other file type that appears here. */
continue;
}
else if (rllen >= sizeof(linkpath))
{
ereport(WARNING,
(errmsg("symbolic link \"%s\" target is too long",
fullpath)));
continue;
}
linkpath[rllen] = '\0';
/*
* Build a backslash-escaped version of the link path to include
* in the tablespace map file.
*/
initStringInfo(&escapedpath);
for (s = linkpath; *s; s++)
{
if (*s == '\n' || *s == '\r' || *s == '\\')
appendStringInfoChar(&escapedpath, '\\');
appendStringInfoChar(&escapedpath, *s);
}
/*
* Relpath holds the relative path of the tablespace directory
* when it's located within PGDATA, or NULL if it's located
* elsewhere.
*/
if (rllen > datadirpathlen &&
strncmp(linkpath, DataDir, datadirpathlen) == 0 &&
IS_DIR_SEP(linkpath[datadirpathlen]))
relpath = linkpath + datadirpathlen + 1;
ti = palloc(sizeof(tablespaceinfo));
ti->oid = pstrdup(de->d_name);
ti->path = pstrdup(linkpath);
ti->rpath = relpath ? pstrdup(relpath) : NULL;
ti->rpath = relpath;
ti->size = -1;
if (tablespaces)
*tablespaces = lappend(*tablespaces, ti);
appendStringInfo(tblspcmapfile, "%s %s\n",
ti->oid, escapedpath.data);
pfree(escapedpath.data);
}
FreeDir(tblspcdir);

53
src/bin/pg_basebackup/bbstreamer_file.c
View File

@ -276,28 +276,49 @@ bbstreamer_extractor_content(bbstreamer *streamer, bbstreamer_member *member,
}
}
/*
* Should we tolerate an already-existing directory?
*
* When streaming WAL, pg_wal (or pg_xlog for pre-9.6 clusters) will have been
* created by the wal receiver process. Also, when the WAL directory location
* was specified, pg_wal (or pg_xlog) has already been created as a symbolic
* link before starting the actual backup. So just ignore creation failures
* on related directories.
*
* If in-place tablespaces are used, pg_tblspc and subdirectories may already
* exist when we get here. So tolerate that case, too.
*/
static bool
should_allow_existing_directory(const char *pathname)
{
const char *filename = last_dir_separator(pathname) + 1;
if (strcmp(filename, "pg_wal") == 0 ||
strcmp(filename, "pg_xlog") == 0 ||
strcmp(filename, "archive_status") == 0 ||
strcmp(filename, "pg_tblspc") == 0)
return true;
if (strspn(filename, "0123456789") == strlen(filename))
{
const char *pg_tblspc = strstr(pathname, "/pg_tblspc/");
return pg_tblspc != NULL && pg_tblspc + 11 == filename;
}
return false;
}
/*
* Create a directory.
*/
static void
extract_directory(const char *filename, mode_t mode)
{
if (mkdir(filename, pg_dir_create_mode) != 0)
{
/*
* When streaming WAL, pg_wal (or pg_xlog for pre-9.6 clusters) will
* have been created by the wal receiver process. Also, when the WAL
* directory location was specified, pg_wal (or pg_xlog) has already
* been created as a symbolic link before starting the actual backup.
* So just ignore creation failures on related directories.
*/
if (!((pg_str_endswith(filename, "/pg_wal") ||
pg_str_endswith(filename, "/pg_xlog") ||
pg_str_endswith(filename, "/archive_status")) &&
errno == EEXIST))
pg_fatal("could not create directory \"%s\": %m",
filename);
}
if (mkdir(filename, pg_dir_create_mode) != 0 &&
(errno != EEXIST || !should_allow_existing_directory(filename)))
pg_fatal("could not create directory \"%s\": %m",
filename);
#ifndef WIN32
if (chmod(filename, mode))

22
src/bin/pg_basebackup/pg_basebackup.c
View File

@ -1122,9 +1122,17 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
* other tablespaces will be written to the directory where they're
* located on the server, after applying any user-specified tablespace
* mappings.
*
* In the case of an in-place tablespace, spclocation will be a
* relative path. We just convert it to an absolute path by prepending
* basedir.
*/
directory = spclocation == NULL ? basedir
: get_tablespace_mapping(spclocation);
if (spclocation == NULL)
directory = basedir;
else if (!is_absolute_path(spclocation))
directory = psprintf("%s/%s", basedir, spclocation);
else
directory = get_tablespace_mapping(spclocation);
streamer = bbstreamer_extractor_new(directory,
get_tablespace_mapping,
progress_update_filename);
@ -1955,7 +1963,15 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
*/
if (backup_target == NULL && format == 'p' && !PQgetisnull(res, i, 1))
{
char *path = unconstify(char *, get_tablespace_mapping(PQgetvalue(res, i, 1)));
char *path = PQgetvalue(res, i, 1);
if (is_absolute_path(path))
path = unconstify(char *, get_tablespace_mapping(path));
else
{
/* This is an in-place tablespace, so prepend basedir. */
path = psprintf("%s/%s", basedir, path);
}
verify_dir_is_empty_or_create(path, &made_tablespace_dirs, &found_tablespace_dirs);
}

40
src/bin/pg_basebackup/t/011_in_place_tablespace.pl Normal file
View File

@ -0,0 +1,40 @@
# Copyright (c) 2021-2023, PostgreSQL Global Development Group
use strict;
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
# For nearly all pg_basebackup invocations some options should be specified,
# to keep test times reasonable. Using @pg_basebackup_defs as the first
# element of the array passed to IPC::Run interpolate the array (as it is
# not a reference to an array)...
my @pg_basebackup_defs = ('pg_basebackup', '--no-sync', '-cfast');
# Set up an instance.
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init('allows_streaming' => 1);
$node->start();
# Create an in-place tablespace.
$node->safe_psql('postgres', <<EOM);
SET allow_in_place_tablespaces = on;
CREATE TABLESPACE inplace LOCATION '';
EOM
# Back it up.
my $backupdir = $tempdir . '/backup';
$node->command_ok(
[ @pg_basebackup_defs, '-D', $backupdir, '-Ft', '-X', 'none' ],
'pg_basebackup runs');
# Make sure we got base.tar and one tablespace.
ok(-f "$backupdir/base.tar", 'backup tar was created');
my @tblspc_tars = glob "$backupdir/[0-9]*.tar";
is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
# All good.
done_testing();