Skip to content

Commit 7d947ec

Browse files
bmomjianbmomjian
committed
Fix pg_upgrade file share violation on Windows created by the commit
4741e9a. This was done by adding an optional second log file parameter to exec_prog(), and closing and reopening the log file between system() calls. Backpatch to 9.2.
1 parent 4d06811 commit 7d947ec

File tree

6 files changed

+41
-25
lines changed

6 files changed

+41
-25
lines changed

‎contrib/pg_upgrade/check.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ issue_warnings(char *sequence_script_file_name)
183183
if (sequence_script_file_name)
184184
{
185185
prep_status("Adjusting sequences");
186-
exec_prog(true, true, UTILITY_LOG_FILE,
186+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
187187
SYSTEMQUOTE "\"%s/psql\" --echo-queries "
188188
"--set ON_ERROR_STOP=on "
189189
"--no-psqlrc --port %d --username \"%s\" "

‎contrib/pg_upgrade/dump.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ generate_old_dump(void)
2323
* --binary-upgrade records the width of dropped columns in pg_class, and
2424
* restores the frozenid's for databases and relations.
2525
*/
26-
exec_prog(true, true, UTILITY_LOG_FILE,
26+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
2727
SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username \"%s\" "
2828
"--schema-only --binary-upgrade %s > \"%s\" 2>> \"%s\""
2929
SYSTEMQUOTE, new_cluster.bindir, old_cluster.port, os_info.user,

‎contrib/pg_upgrade/exec.c

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,19 @@ static int win32_check_directory_write_permissions(void);
3333
* line to be executed is saved to the specified log file.
3434
*
3535
* If throw_error is TRUE, this function will throw a PG_FATAL error
36-
* instead of returning should an error occur.
36+
* instead of returning should an error occur. The command it appended
37+
* to log_file; opt_log_file is used in error messages.
3738
*/
3839
int
39-
exec_prog(bool throw_error, bool is_priv,
40-
const char *log_file, const char *fmt,...)
40+
exec_prog(bool throw_error, bool is_priv, const char *log_file,
41+
const char *opt_log_file, const char *fmt,...)
4142
{
4243
va_list args;
4344
int result;
4445
int retval;
4546
char cmd[MAXPGPATH];
4647
mode_t old_umask = 0;
47-
FILE *log = fopen(log_file, "a+");
48+
FILE *log;
4849

4950
if (is_priv)
5051
old_umask = umask(S_IRWXG | S_IRWXO);
@@ -53,9 +54,15 @@ exec_prog(bool throw_error, bool is_priv,
5354
vsnprintf(cmd, MAXPGPATH, fmt, args);
5455
va_end(args);
5556

57+
if ((log = fopen_priv(log_file, "a+")) == NULL)
58+
pg_log(PG_FATAL, "cannot write to log file %s\n", log_file);
5659
pg_log(PG_VERBOSE, "%s\n", cmd);
5760
fprintf(log, "command: %s\n", cmd);
58-
fflush(log);
61+
/*
62+
* In Windows, we must close then reopen the log file so the file is
63+
* not open while the command is running, or we get a share violation.
64+
*/
65+
fclose(log);
5966

6067
result = system(cmd);
6168

@@ -64,18 +71,28 @@ exec_prog(bool throw_error, bool is_priv,
6471

6572
if (result != 0)
6673
{
74+
char opt_string[MAXPGPATH];
75+
76+
/* Create string for optional second log file */
77+
if (opt_log_file)
78+
snprintf(opt_string, sizeof(opt_string), " or \"%s\"", opt_log_file);
79+
else
80+
opt_string[0] = '\0';
81+
6782
report_status(PG_REPORT, "*failure*");
6883
fflush(stdout);
6984
pg_log(PG_VERBOSE, "There were problems executing \"%s\"\n", cmd);
7085
pg_log(throw_error ? PG_FATAL : PG_REPORT,
71-
"Consult the last few lines of \"%s\" for\n"
86+
"Consult the last few lines of \"%s\"%s for\n"
7287
"the probable cause of the failure.\n",
73-
log_file);
88+
log_file, opt_string);
7489
retval = 1;
7590
}
7691
else
7792
retval = 0;
7893

94+
if ((log = fopen_priv(log_file, "a+")) == NULL)
95+
pg_log(PG_FATAL, "cannot write to log file %s\n", log_file);
7996
fprintf(log, "\n\n");
8097
fclose(log);
8198

‎contrib/pg_upgrade/pg_upgrade.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ main(int argc, char **argv)
140140
* because there is no need to have the schema load use new oids.
141141
*/
142142
prep_status("Setting next OID for new cluster");
143-
exec_prog(true, true, UTILITY_LOG_FILE,
143+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
144144
SYSTEMQUOTE "\"%s/pg_resetxlog\" -o %u \"%s\" >> \"%s\" 2>&1"
145145
SYSTEMQUOTE,
146146
new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid,
@@ -211,7 +211,7 @@ prepare_new_cluster(void)
211211
* --analyze so autovacuum doesn't update statistics later
212212
*/
213213
prep_status("Analyzing all rows in the new cluster");
214-
exec_prog(true, true, UTILITY_LOG_FILE,
214+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
215215
SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" "
216216
"--all --analyze %s >> \"%s\" 2>&1" SYSTEMQUOTE,
217217
new_cluster.bindir, new_cluster.port, os_info.user,
@@ -225,7 +225,7 @@ prepare_new_cluster(void)
225225
* later.
226226
*/
227227
prep_status("Freezing all rows on the new cluster");
228-
exec_prog(true, true, UTILITY_LOG_FILE,
228+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
229229
SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" "
230230
"--all --freeze %s >> \"%s\" 2>&1" SYSTEMQUOTE,
231231
new_cluster.bindir, new_cluster.port, os_info.user,
@@ -263,7 +263,7 @@ prepare_new_databases(void)
263263
* support functions in template1 but pg_dumpall creates database using
264264
* the template0 template.
265265
*/
266-
exec_prog(true, true, RESTORE_LOG_FILE,
266+
exec_prog(true, true, RESTORE_LOG_FILE, NULL,
267267
SYSTEMQUOTE "\"%s/psql\" --echo-queries "
268268
"--set ON_ERROR_STOP=on "
269269
/* --no-psqlrc prevents AUTOCOMMIT=off */
@@ -296,7 +296,7 @@ create_new_objects(void)
296296
check_ok();
297297

298298
prep_status("Restoring database schema to new cluster");
299-
exec_prog(true, true, RESTORE_LOG_FILE,
299+
exec_prog(true, true, RESTORE_LOG_FILE, NULL,
300300
SYSTEMQUOTE "\"%s/psql\" --echo-queries "
301301
"--set ON_ERROR_STOP=on "
302302
"--no-psqlrc --port %d --username \"%s\" "
@@ -328,7 +328,7 @@ copy_clog_xlog_xid(void)
328328
check_ok();
329329

330330
prep_status("Copying old commit clogs to new server");
331-
exec_prog(true, false, UTILITY_LOG_FILE,
331+
exec_prog(true, false, UTILITY_LOG_FILE, NULL,
332332
#ifndef WIN32
333333
SYSTEMQUOTE "%s \"%s\" \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE,
334334
"cp -Rf",
@@ -342,7 +342,7 @@ copy_clog_xlog_xid(void)
342342

343343
/* set the next transaction id of the new cluster */
344344
prep_status("Setting next transaction ID for new cluster");
345-
exec_prog(true, true, UTILITY_LOG_FILE,
345+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
346346
SYSTEMQUOTE
347347
"\"%s/pg_resetxlog\" -f -x %u \"%s\" >> \"%s\" 2>&1"
348348
SYSTEMQUOTE, new_cluster.bindir,
@@ -352,7 +352,7 @@ copy_clog_xlog_xid(void)
352352

353353
/* now reset the wal archives in the new cluster */
354354
prep_status("Resetting WAL archives");
355-
exec_prog(true, true, UTILITY_LOG_FILE,
355+
exec_prog(true, true, UTILITY_LOG_FILE, NULL,
356356
SYSTEMQUOTE
357357
"\"%s/pg_resetxlog\" -l %u,%u,%u \"%s\" >> \"%s\" 2>&1"
358358
SYSTEMQUOTE, new_cluster.bindir,

‎contrib/pg_upgrade/pg_upgrade.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,9 @@ void split_old_dump(void);
318318
/* exec.c */
319319

320320
int
321-
exec_prog(bool throw_error, bool is_priv,
322-
const char *log_file, const char *cmd,...)
323-
__attribute__((format(PG_PRINTF_ATTRIBUTE, 4, 5)));
321+
exec_prog(bool throw_error, bool is_priv, const char *log_file,
322+
const char *opt_log_file, const char *cmd,...)
323+
__attribute__((format(PG_PRINTF_ATTRIBUTE, 5, 6)));
324324
void verify_directories(void);
325325
bool is_server_running(const char *datadir);
326326

‎contrib/pg_upgrade/server.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,10 @@ start_postmaster(ClusterInfo *cluster)
171171
* Don't throw an error right away, let connecting throw the error because
172172
* it might supply a reason for the failure.
173173
*/
174-
pg_ctl_return = exec_prog(false, true,
174+
pg_ctl_return = exec_prog(false, true, SERVER_START_LOG_FILE,
175175
/* pass both file names if the differ */
176-
(strcmp(SERVER_LOG_FILE, SERVER_START_LOG_FILE) == 0) ?
177-
SERVER_LOG_FILE :
178-
SERVER_LOG_FILE " or " SERVER_START_LOG_FILE,
176+
(strcmp(SERVER_LOG_FILE, SERVER_START_LOG_FILE) != 0) ?
177+
SERVER_LOG_FILE : NULL,
179178
"%s", cmd);
180179

181180
/* Check to see if we can connect to the server; if not, report it. */
@@ -220,7 +219,7 @@ stop_postmaster(bool fast)
220219
cluster->pgopts ? cluster->pgopts : "",
221220
fast ? "-m fast" : "", SERVER_STOP_LOG_FILE);
222221

223-
exec_prog(fast ? false : true, true, SERVER_STOP_LOG_FILE, "%s", cmd);
222+
exec_prog(fast ? false : true, true, SERVER_STOP_LOG_FILE, NULL, "%s", cmd);
224223

225224
os_info.running_cluster = NULL;
226225
}

0 commit comments

Comments
 (0)