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

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a11406c741c6c0cdc977eb86f29e9bce6b200110..e46ee553d65f9c9fde959a589117c4d6d76406ed 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10206,7 +10206,7 @@ XLogFileNameP(TimeLineID tli, XLogSegNo segno)
  */
 XLogRecPtr
 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,
                                   bool needtblspcmapfile)
 {
@@ -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));
        {
                bool            gotUniqueStartpoint = false;
+               DIR                *tblspcdir;
                struct dirent *de;
                tablespaceinfo *ti;
                int                     datadirpathlen;
@@ -10428,6 +10429,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
                datadirpathlen = strlen(DataDir);
 
                /* Collect information about all tablespaces */
+               tblspcdir = AllocateDir("pg_tblspc");
                while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
                {
                        char            fullpath[MAXPGPATH + 10];
@@ -10476,7 +10478,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
                                appendStringInfoChar(&buflinkpath, *s++);
                        }
 
-
                        /*
                         * Relpath holds the relative path of the tablespace directory
                         * 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,
                                         errmsg("tablespaces are not supported on this platform")));
 #endif
                }
+               FreeDir(tblspcdir);
 
                /*
                 * Construct backup label file

diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 48d85c1ce5d2fa3d98f1ed50e2a2e5486bf8acb6..c41428ea2a3848eaab3e36bb04fb6198fd0ae201 100644 (file)
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -75,7 +75,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
        bool            exclusive = PG_GETARG_BOOL(2);
        char       *backupidstr;
        XLogRecPtr      startpoint;
-       DIR                *dir;
        SessionBackupState status = get_backup_status();
 
        backupidstr = text_to_cstring(backupid);
@@ -85,18 +84,10 @@ pg_start_backup(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                 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)
        {
                startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
-                                                                               dir, NULL, NULL, false, true);
+                                                                               NULL, NULL, false, true);
        }
        else
        {
@@ -112,13 +103,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
                MemoryContextSwitchTo(oldcontext);
 
                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);
        }
 
-       FreeDir(dir);
-
        PG_RETURN_LSN(startpoint);
 }
 

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index b264b69aef694c66022ab05aa5860fd7aac8c631..cd7d391b2ffd59089e8a557d754796c437d7295a 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -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 SendBackupHeader(List *tablespaces);
 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 SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli);
 static int     compareWalFileNames(const void *a, const void *b);
@@ -188,7 +188,7 @@ base_backup_cleanup(int code, Datum arg)
  * clobbered by longjmp" from stupider versions of gcc.
  */
 static void
-perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
+perform_base_backup(basebackup_options *opt)
 {
        XLogRecPtr      startptr;
        TimeLineID      starttli;
@@ -207,7 +207,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
        tblspc_map_file = makeStringInfo();
 
        startptr = do_pg_start_backup(opt->label, opt->fastcheckpoint, &starttli,
-                                                                 labelfile, tblspcdir, &tablespaces,
+                                                                 labelfile, &tablespaces,
                                                                  tblspc_map_file,
                                                                  opt->progress, opt->sendtblspcmapfile);
 
@@ -690,7 +690,6 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 void
 SendBaseBackup(BaseBackupCmd *cmd)
 {
-       DIR                *dir;
        basebackup_options opt;
 
        parse_basebackup_options(cmd->options, &opt);
@@ -706,17 +705,7 @@ SendBaseBackup(BaseBackupCmd *cmd)
                set_ps_display(activitymsg, false);
        }
 
-       /* 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")));
-
-       perform_base_backup(&opt, dir);
-
-       FreeDir(dir);
+       perform_base_backup(&opt);
 }
 
 static void

diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 8fd6010ba04a73f9ddbbd40b6114477ab25b2b5c..dd7d8b5e40308defb3a888cce337a5bae904d283 100644 (file)
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -310,7 +310,7 @@ typedef enum SessionBackupState
 } SessionBackupState;
 
 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,
                                   bool needtblspcmapfile);
 extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,