Skip to content

Commit 066bc21

Browse files
tglsfdctglsfdc
committed
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
1 parent 561885d commit 066bc21

File tree

4 files changed

+11
-31
lines changed

4 files changed

+11
-31
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10206,7 +10206,7 @@ XLogFileNameP(TimeLineID tli, XLogSegNo segno)
1020610206
*/
1020710207
XLogRecPtr
1020810208
do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
10209-
StringInfo labelfile, DIR *tblspcdir, List **tablespaces,
10209+
StringInfo labelfile, List **tablespaces,
1021010210
StringInfo tblspcmapfile, bool infotbssize,
1021110211
bool needtblspcmapfile)
1021210212
{
@@ -10297,6 +10297,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1029710297
PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
1029810298
{
1029910299
bool gotUniqueStartpoint = false;
10300+
DIR *tblspcdir;
1030010301
struct dirent *de;
1030110302
tablespaceinfo *ti;
1030210303
int datadirpathlen;
@@ -10428,6 +10429,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1042810429
datadirpathlen = strlen(DataDir);
1042910430

1043010431
/* Collect information about all tablespaces */
10432+
tblspcdir = AllocateDir("pg_tblspc");
1043110433
while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
1043210434
{
1043310435
char fullpath[MAXPGPATH + 10];
@@ -10476,7 +10478,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1047610478
appendStringInfoChar(&buflinkpath, *s++);
1047710479
}
1047810480

10479-
1048010481
/*
1048110482
* Relpath holds the relative path of the tablespace directory
1048210483
* 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,
1051110512
errmsg("tablespaces are not supported on this platform")));
1051210513
#endif
1051310514
}
10515+
FreeDir(tblspcdir);
1051410516

1051510517
/*
1051610518
* Construct backup label file

‎src/backend/access/transam/xlogfuncs.c

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
7575
bool exclusive = PG_GETARG_BOOL(2);
7676
char *backupidstr;
7777
XLogRecPtr startpoint;
78-
DIR *dir;
7978
SessionBackupState status = get_backup_status();
8079

8180
backupidstr = text_to_cstring(backupid);
@@ -85,18 +84,10 @@ pg_start_backup(PG_FUNCTION_ARGS)
8584
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
8685
errmsg("a backup is already in progress in this session")));
8786

88-
/* Make sure we can open the directory with tablespaces in it */
89-
dir = AllocateDir("pg_tblspc");
90-
if (!dir)
91-
ereport(ERROR,
92-
(errcode_for_file_access(),
93-
errmsg("could not open directory \"%s\": %m",
94-
"pg_tblspc")));
95-
9687
if (exclusive)
9788
{
9889
startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
99-
dir, NULL, NULL, false, true);
90+
NULL, NULL, false, true);
10091
}
10192
else
10293
{
@@ -112,13 +103,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
112103
MemoryContextSwitchTo(oldcontext);
113104

114105
startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
115-
dir, NULL, tblspc_map_file, false, true);
106+
NULL, tblspc_map_file, false, true);
116107

117108
before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
118109
}
119110

120-
FreeDir(dir);
121-
122111
PG_RETURN_LSN(startpoint);
123112
}
124113

‎src/backend/replication/basebackup.c

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ static int64 _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *sta
6464
static void send_int8_string(StringInfoData *buf, int64 intval);
6565
static void SendBackupHeader(List *tablespaces);
6666
static void base_backup_cleanup(int code, Datum arg);
67-
static void perform_base_backup(basebackup_options *opt, DIR *tblspcdir);
67+
static void perform_base_backup(basebackup_options *opt);
6868
static void parse_basebackup_options(List *options, basebackup_options *opt);
6969
static void SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli);
7070
static int compareWalFileNames(const void *a, const void *b);
@@ -188,7 +188,7 @@ base_backup_cleanup(int code, Datum arg)
188188
* clobbered by longjmp" from stupider versions of gcc.
189189
*/
190190
static void
191-
perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
191+
perform_base_backup(basebackup_options *opt)
192192
{
193193
XLogRecPtr startptr;
194194
TimeLineID starttli;
@@ -207,7 +207,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
207207
tblspc_map_file = makeStringInfo();
208208

209209
startptr = do_pg_start_backup(opt->label, opt->fastcheckpoint, &starttli,
210-
labelfile, tblspcdir, &tablespaces,
210+
labelfile, &tablespaces,
211211
tblspc_map_file,
212212
opt->progress, opt->sendtblspcmapfile);
213213

@@ -690,7 +690,6 @@ parse_basebackup_options(List *options, basebackup_options *opt)
690690
void
691691
SendBaseBackup(BaseBackupCmd *cmd)
692692
{
693-
DIR *dir;
694693
basebackup_options opt;
695694

696695
parse_basebackup_options(cmd->options, &opt);
@@ -706,17 +705,7 @@ SendBaseBackup(BaseBackupCmd *cmd)
706705
set_ps_display(activitymsg, false);
707706
}
708707

709-
/* Make sure we can open the directory with tablespaces in it */
710-
dir = AllocateDir("pg_tblspc");
711-
if (!dir)
712-
ereport(ERROR,
713-
(errcode_for_file_access(),
714-
errmsg("could not open directory \"%s\": %m",
715-
"pg_tblspc")));
716-
717-
perform_base_backup(&opt, dir);
718-
719-
FreeDir(dir);
708+
perform_base_backup(&opt);
720709
}
721710

722711
static void

‎src/include/access/xlog.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ typedef enum SessionBackupState
310310
} SessionBackupState;
311311

312312
extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
313-
TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir,
313+
TimeLineID *starttli_p, StringInfo labelfile,
314314
List **tablespaces, StringInfo tblspcmapfile, bool infotbssize,
315315
bool needtblspcmapfile);
316316
extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,

0 commit comments

Comments
 (0)