Commit 066bc21c authored by Tom Lane's avatar Tom Lane

Simplify do_pg_start_backup's API by opening pg_tblspc internally.

do_pg_start_backup() expects its callers to pass in an open DIR pointer
for the pg_tblspc directory, but there's no apparent advantage in that.
It complicates the callers without adding any flexibility, and there's no
robustness advantage, since we surely have to be prepared for errors during
the scan of pg_tblspc anyway.  In fact, by holding an extra kernel resource
during operations like the preliminary checkpoint, we might be making
things a fraction more failure-prone not less.  Hence, remove that argument
and open the directory just for the duration of the actual scan.

Discussion: https://postgr.es/m/28752.1512413887@sss.pgh.pa.us
parent 561885db
...@@ -10206,7 +10206,7 @@ XLogFileNameP(TimeLineID tli, XLogSegNo segno) ...@@ -10206,7 +10206,7 @@ XLogFileNameP(TimeLineID tli, XLogSegNo segno)
*/ */
XLogRecPtr XLogRecPtr
do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
StringInfo labelfile, DIR *tblspcdir, List **tablespaces, StringInfo labelfile, List **tablespaces,
StringInfo tblspcmapfile, bool infotbssize, StringInfo tblspcmapfile, bool infotbssize,
bool needtblspcmapfile) bool needtblspcmapfile)
{ {
...@@ -10297,6 +10297,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ...@@ -10297,6 +10297,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive)); PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
{ {
bool gotUniqueStartpoint = false; bool gotUniqueStartpoint = false;
DIR *tblspcdir;
struct dirent *de; struct dirent *de;
tablespaceinfo *ti; tablespaceinfo *ti;
int datadirpathlen; int datadirpathlen;
...@@ -10428,6 +10429,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ...@@ -10428,6 +10429,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
datadirpathlen = strlen(DataDir); datadirpathlen = strlen(DataDir);
/* Collect information about all tablespaces */ /* Collect information about all tablespaces */
tblspcdir = AllocateDir("pg_tblspc");
while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL) while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
{ {
char fullpath[MAXPGPATH + 10]; char fullpath[MAXPGPATH + 10];
...@@ -10476,7 +10478,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ...@@ -10476,7 +10478,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
appendStringInfoChar(&buflinkpath, *s++); appendStringInfoChar(&buflinkpath, *s++);
} }
/* /*
* Relpath holds the relative path of the tablespace directory * Relpath holds the relative path of the tablespace directory
* when it's located within PGDATA, or NULL if it's located * when it's located within PGDATA, or NULL if it's located
...@@ -10511,6 +10512,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ...@@ -10511,6 +10512,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errmsg("tablespaces are not supported on this platform"))); errmsg("tablespaces are not supported on this platform")));
#endif #endif
} }
FreeDir(tblspcdir);
/* /*
* Construct backup label file * Construct backup label file
......
...@@ -75,7 +75,6 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -75,7 +75,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
bool exclusive = PG_GETARG_BOOL(2); bool exclusive = PG_GETARG_BOOL(2);
char *backupidstr; char *backupidstr;
XLogRecPtr startpoint; XLogRecPtr startpoint;
DIR *dir;
SessionBackupState status = get_backup_status(); SessionBackupState status = get_backup_status();
backupidstr = text_to_cstring(backupid); backupidstr = text_to_cstring(backupid);
...@@ -85,18 +84,10 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -85,18 +84,10 @@ pg_start_backup(PG_FUNCTION_ARGS)
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("a backup is already in progress in this session"))); errmsg("a backup is already in progress in this session")));
/* Make sure we can open the directory with tablespaces in it */
dir = AllocateDir("pg_tblspc");
if (!dir)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open directory \"%s\": %m",
"pg_tblspc")));
if (exclusive) if (exclusive)
{ {
startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL, startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
dir, NULL, NULL, false, true); NULL, NULL, false, true);
} }
else else
{ {
...@@ -112,13 +103,11 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -112,13 +103,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
MemoryContextSwitchTo(oldcontext); MemoryContextSwitchTo(oldcontext);
startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file, startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
dir, NULL, tblspc_map_file, false, true); NULL, tblspc_map_file, false, true);
before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0); before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
} }
FreeDir(dir);
PG_RETURN_LSN(startpoint); PG_RETURN_LSN(startpoint);
} }
......
...@@ -64,7 +64,7 @@ static int64 _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *sta ...@@ -64,7 +64,7 @@ static int64 _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *sta
static void send_int8_string(StringInfoData *buf, int64 intval); static void send_int8_string(StringInfoData *buf, int64 intval);
static void SendBackupHeader(List *tablespaces); static void SendBackupHeader(List *tablespaces);
static void base_backup_cleanup(int code, Datum arg); static void base_backup_cleanup(int code, Datum arg);
static void perform_base_backup(basebackup_options *opt, DIR *tblspcdir); static void perform_base_backup(basebackup_options *opt);
static void parse_basebackup_options(List *options, basebackup_options *opt); static void parse_basebackup_options(List *options, basebackup_options *opt);
static void SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli); static void SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli);
static int compareWalFileNames(const void *a, const void *b); static int compareWalFileNames(const void *a, const void *b);
...@@ -188,7 +188,7 @@ base_backup_cleanup(int code, Datum arg) ...@@ -188,7 +188,7 @@ base_backup_cleanup(int code, Datum arg)
* clobbered by longjmp" from stupider versions of gcc. * clobbered by longjmp" from stupider versions of gcc.
*/ */
static void static void
perform_base_backup(basebackup_options *opt, DIR *tblspcdir) perform_base_backup(basebackup_options *opt)
{ {
XLogRecPtr startptr; XLogRecPtr startptr;
TimeLineID starttli; TimeLineID starttli;
...@@ -207,7 +207,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir) ...@@ -207,7 +207,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
tblspc_map_file = makeStringInfo(); tblspc_map_file = makeStringInfo();
startptr = do_pg_start_backup(opt->label, opt->fastcheckpoint, &starttli, startptr = do_pg_start_backup(opt->label, opt->fastcheckpoint, &starttli,
labelfile, tblspcdir, &tablespaces, labelfile, &tablespaces,
tblspc_map_file, tblspc_map_file,
opt->progress, opt->sendtblspcmapfile); opt->progress, opt->sendtblspcmapfile);
...@@ -690,7 +690,6 @@ parse_basebackup_options(List *options, basebackup_options *opt) ...@@ -690,7 +690,6 @@ parse_basebackup_options(List *options, basebackup_options *opt)
void void
SendBaseBackup(BaseBackupCmd *cmd) SendBaseBackup(BaseBackupCmd *cmd)
{ {
DIR *dir;
basebackup_options opt; basebackup_options opt;
parse_basebackup_options(cmd->options, &opt); parse_basebackup_options(cmd->options, &opt);
...@@ -706,17 +705,7 @@ SendBaseBackup(BaseBackupCmd *cmd) ...@@ -706,17 +705,7 @@ SendBaseBackup(BaseBackupCmd *cmd)
set_ps_display(activitymsg, false); set_ps_display(activitymsg, false);
} }
/* Make sure we can open the directory with tablespaces in it */ perform_base_backup(&opt);
dir = AllocateDir("pg_tblspc");
if (!dir)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open directory \"%s\": %m",
"pg_tblspc")));
perform_base_backup(&opt, dir);
FreeDir(dir);
} }
static void static void
......
...@@ -310,7 +310,7 @@ typedef enum SessionBackupState ...@@ -310,7 +310,7 @@ typedef enum SessionBackupState
} SessionBackupState; } SessionBackupState;
extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast, extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir, TimeLineID *starttli_p, StringInfo labelfile,
List **tablespaces, StringInfo tblspcmapfile, bool infotbssize, List **tablespaces, StringInfo tblspcmapfile, bool infotbssize,
bool needtblspcmapfile); bool needtblspcmapfile);
extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive, extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment