Commit
c6f2f01611d4f2c412e92eb7893f76fa590818e8 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
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))
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)));
- continue;
+ 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 (rllen >= sizeof(linkpath))
+ else if (de_type == PGFILETYPE_DIR)
{
- ereport(WARNING,
- (errmsg("symbolic link \"%s\" target is too long",
- fullpath)));
- continue;
+ /*
+ * 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);
}
- 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++)
+ else
{
- if (*s == '\n' || *s == '\r' || *s == '\\')
- appendStringInfoChar(&escapedpath, '\\');
- appendStringInfoChar(&escapedpath, *s);
+ /* Skip any other file type that appears here. */
+ continue;
}
- /*
- * 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);
}
}
+/*
+ * 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))
* 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);
*/
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);
}
--- /dev/null
+# 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();
projects / users / rhaas / postgres.git / commitdiff