Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4fff78f

Browse files
committed
Restructure pg_upgrade output directories for better idempotence
38bfae3 has moved the contents written to files by pg_upgrade under anew directory called pg_upgrade_output.d/ located in the new cluster'sdata folder, and it used a simple structure made of two subdirectoriesleading to a fixed structure: log/ and dump/. This design has madeweaker pg_upgrade on repeated calls, as we could get failures whencreating one or more of those directories, while potentially losing thelogs of a previous run (logs are retained automatically on failure, andcleaned up on success unless --retain is specified). So a user wouldneed to clean up pg_upgrade_output.d/ as an extra step for any repeatedcalls of pg_upgrade. The most common scenario here is --check followedby the actual upgrade, but one could see a failure when specifying anincorrect input argument value. Removing entirely the logs would havethe disadvantage of removing all the past information, even if --retainwas specified at some past step.This result is annoying for a lot of users and automated upgrade flows.So, rather than requiring a manual removal of pg_upgrade_output.d/, thisredesigns the set of output directories in a more dynamic way, based ona suggestion from Tom Lane and Daniel Gustafsson. pg_upgrade_output.d/is still the base path, but a second directory level is added, mostlynamed after an ISO-8601-formatted timestamp (in short human-readable,with milliseconds appended to the name to avoid any conflicts). Thelogs and dumps are saved within the same subdirectories as previously,as of log/ and dump/, but these are located inside the subdirectorynamed after the timestamp.The logs of a given run are removed only after a successful run if--retain is not used, and pg_upgrade_output.d/ is kept if there are anylogs from a previous run. Note that previously, pg_upgrade would havekept the logs even after a successful --check but that was inconsistentcompared to the case without --check when using --retain. The code incharge of the removal of the output directories is now refactored into asingle routine.Two TAP tests are added with some --check commands (one failure case andone success case), to look after the issue fixed here. Note that thetests had to be tweaked a bit to fit with the new directory structure soas it can find any logs generated on failure. This is still going torequire a change in the buildfarm client for the case where pg_upgradeis tested without the TAP test, though, but I'll tackle that with aseparate patch where needed.Reported-by: Tushar AhujaAuthor: Michael PaquierReviewed-by: Daniel Gustafsson, Justin PryzbyDiscussion:https://postgr.es/m/77e6ecaa-2785-97aa-f229-4b6e047cbd2b@enterprisedb.com
1 parentfa5185b commit4fff78f

File tree

6 files changed

+150
-31
lines changed

6 files changed

+150
-31
lines changed

‎doc/src/sgml/ref/pgupgrade.sgml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,10 @@ psql --username=postgres --file=script.sql postgres
768768
<para>
769769
<application>pg_upgrade</application> creates various working files, such
770770
as schema dumps, stored within <literal>pg_upgrade_output.d</literal> in
771-
the directory of the new cluster.
771+
the directory of the new cluster. Each run creates a new subdirectory named
772+
with a timestamp formatted as per ISO 8601
773+
(<literal>%Y%m%dT%H%M%S</literal>), where all the generated files are
774+
stored.
772775
</para>
773776

774777
<para>

‎src/bin/pg_upgrade/check.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ report_clusters_compatible(void)
210210
pg_log(PG_REPORT,"\n*Clusters are compatible*\n");
211211
/* stops new cluster */
212212
stop_postmaster(false);
213+
214+
cleanup_output_dirs();
213215
exit(0);
214216
}
215217

‎src/bin/pg_upgrade/pg_upgrade.c

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ static void copy_xact_xlog_xid(void);
5858
staticvoidset_frozenxids(boolminmxid_only);
5959
staticvoidmake_outputdirs(char*pgdata);
6060
staticvoidsetup(char*argv0,bool*live_check);
61-
staticvoidcleanup(void);
6261

6362
ClusterInfoold_cluster,
6463
new_cluster;
@@ -204,7 +203,7 @@ main(int argc, char **argv)
204203

205204
pg_free(deletion_script_file_name);
206205

207-
cleanup();
206+
cleanup_output_dirs();
208207

209208
return0;
210209
}
@@ -221,19 +220,54 @@ make_outputdirs(char *pgdata)
221220
char**filename;
222221
time_trun_time=time(NULL);
223222
charfilename_path[MAXPGPATH];
223+
chartimebuf[128];
224+
structtimevaltime;
225+
time_ttt;
226+
intlen;
227+
228+
log_opts.rootdir= (char*)pg_malloc0(MAXPGPATH);
229+
len=snprintf(log_opts.rootdir,MAXPGPATH,"%s/%s",pgdata,BASE_OUTPUTDIR);
230+
if (len >=MAXPGPATH)
231+
pg_fatal("buffer for root directory too small");
232+
233+
/* BASE_OUTPUTDIR/$timestamp/ */
234+
gettimeofday(&time,NULL);
235+
tt= (time_t)time.tv_sec;
236+
strftime(timebuf,sizeof(timebuf),"%Y%m%dT%H%M%S",localtime(&tt));
237+
/* append milliseconds */
238+
snprintf(timebuf+strlen(timebuf),sizeof(timebuf)-strlen(timebuf),
239+
".%03d", (int) (time.tv_usec /1000));
240+
log_opts.basedir= (char*)pg_malloc0(MAXPGPATH);
241+
len=snprintf(log_opts.basedir,MAXPGPATH,"%s/%s",log_opts.rootdir,
242+
timebuf);
243+
if (len >=MAXPGPATH)
244+
pg_fatal("buffer for base directory too small");
245+
246+
/* BASE_OUTPUTDIR/$timestamp/dump/ */
247+
log_opts.dumpdir= (char*)pg_malloc0(MAXPGPATH);
248+
len=snprintf(log_opts.dumpdir,MAXPGPATH,"%s/%s/%s",log_opts.rootdir,
249+
timebuf,DUMP_OUTPUTDIR);
250+
if (len >=MAXPGPATH)
251+
pg_fatal("buffer for dump directory too small");
252+
253+
/* BASE_OUTPUTDIR/$timestamp/log/ */
254+
log_opts.logdir= (char*)pg_malloc0(MAXPGPATH);
255+
len=snprintf(log_opts.logdir,MAXPGPATH,"%s/%s/%s",log_opts.rootdir,
256+
timebuf,LOG_OUTPUTDIR);
257+
if (len >=MAXPGPATH)
258+
pg_fatal("buffer for log directory too small");
224259

225-
log_opts.basedir= (char*)pg_malloc(MAXPGPATH);
226-
snprintf(log_opts.basedir,MAXPGPATH,"%s/%s",pgdata,BASE_OUTPUTDIR);
227-
log_opts.dumpdir= (char*)pg_malloc(MAXPGPATH);
228-
snprintf(log_opts.dumpdir,MAXPGPATH,"%s/%s",pgdata,DUMP_OUTPUTDIR);
229-
log_opts.logdir= (char*)pg_malloc(MAXPGPATH);
230-
snprintf(log_opts.logdir,MAXPGPATH,"%s/%s",pgdata,LOG_OUTPUTDIR);
231-
232-
if (mkdir(log_opts.basedir,pg_dir_create_mode))
260+
/*
261+
* Ignore the error case where the root path exists, as it is kept the
262+
* same across runs.
263+
*/
264+
if (mkdir(log_opts.rootdir,pg_dir_create_mode)<0&&errno!=EEXIST)
265+
pg_fatal("could not create directory \"%s\": %m\n",log_opts.rootdir);
266+
if (mkdir(log_opts.basedir,pg_dir_create_mode)<0)
233267
pg_fatal("could not create directory \"%s\": %m\n",log_opts.basedir);
234-
if (mkdir(log_opts.dumpdir,pg_dir_create_mode))
268+
if (mkdir(log_opts.dumpdir,pg_dir_create_mode)<0)
235269
pg_fatal("could not create directory \"%s\": %m\n",log_opts.dumpdir);
236-
if (mkdir(log_opts.logdir,pg_dir_create_mode))
270+
if (mkdir(log_opts.logdir,pg_dir_create_mode)<0)
237271
pg_fatal("could not create directory \"%s\": %m\n",log_opts.logdir);
238272

239273
snprintf(filename_path,sizeof(filename_path),"%s/%s",log_opts.logdir,
@@ -745,14 +779,3 @@ set_frozenxids(bool minmxid_only)
745779

746780
check_ok();
747781
}
748-
749-
750-
staticvoid
751-
cleanup(void)
752-
{
753-
fclose(log_opts.internal);
754-
755-
/* Remove dump and log files? */
756-
if (!log_opts.retain)
757-
(void)rmtree(log_opts.basedir, true);
758-
}

‎src/bin/pg_upgrade/pg_upgrade.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@
3030
#defineDB_DUMP_FILE_MASK"pg_upgrade_dump_%u.custom"
3131

3232
/*
33-
* Base directories that include all the files generated internally,
34-
* from the root path of the new cluster.
33+
* Base directories that include all the files generated internally, from the
34+
* root path of the new cluster. The paths are dynamically built as of
35+
* BASE_OUTPUTDIR/$timestamp/{LOG_OUTPUTDIR,DUMP_OUTPUTDIR} to ensure their
36+
* uniqueness in each run.
3537
*/
3638
#defineBASE_OUTPUTDIR"pg_upgrade_output.d"
37-
#defineLOG_OUTPUTDIRBASE_OUTPUTDIR "/log"
38-
#defineDUMP_OUTPUTDIRBASE_OUTPUTDIR "/dump"
39+
#defineLOG_OUTPUTDIR "log"
40+
#defineDUMP_OUTPUTDIR "dump"
3941

4042
#defineDB_DUMP_LOG_FILE_MASK"pg_upgrade_dump_%u.log"
4143
#defineSERVER_LOG_FILE"pg_upgrade_server.log"
@@ -276,7 +278,8 @@ typedef struct
276278
boolverbose;/* true -> be verbose in messages */
277279
boolretain;/* retain log files on success */
278280
/* Set of internal directories for output files */
279-
char*basedir;/* Base output directory */
281+
char*rootdir;/* Root directory, aka pg_upgrade_output.d */
282+
char*basedir;/* Base output directory, with timestamp */
280283
char*dumpdir;/* Dumps */
281284
char*logdir;/* Log files */
282285
boolisatty;/* is stdout a tty */
@@ -432,6 +435,7 @@ voidreport_status(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3
432435
voidpg_log(eLogTypetype,constchar*fmt,...)pg_attribute_printf(2,3);
433436
voidpg_fatal(constchar*fmt,...)pg_attribute_printf(1,2)pg_attribute_noreturn();
434437
voidend_progress_output(void);
438+
voidcleanup_output_dirs(void);
435439
voidprep_status(constchar*fmt,...)pg_attribute_printf(1,2);
436440
voidprep_status_progress(constchar*fmt,...)pg_attribute_printf(1,2);
437441
unsignedintstr2uint(constchar*str);

‎src/bin/pg_upgrade/t/002_pg_upgrade.pl

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use Cwdqw(abs_path);
66
use File::Basenameqw(dirname);
77
use File::Compare;
8+
use File::Findqw(find);
9+
use File::Pathqw(rmtree);
810

911
use PostgreSQL::Test::Cluster;
1012
use PostgreSQL::Test::Utils;
@@ -213,6 +215,39 @@ sub generate_db
213215

214216
# Upgrade the instance.
215217
$oldnode->stop;
218+
219+
# Cause a failure at the start of pg_upgrade, this should create the logging
220+
# directory pg_upgrade_output.d but leave it around. Keep --check for an
221+
# early exit.
222+
command_fails(
223+
[
224+
'pg_upgrade','--no-sync',
225+
'-d',$oldnode->data_dir,
226+
'-D',$newnode->data_dir,
227+
'-b',$oldbindir .'/does/not/exist/',
228+
'-B',$newbindir,
229+
'-p',$oldnode->port,
230+
'-P',$newnode->port,
231+
'--check'
232+
],
233+
'run of pg_upgrade --check for new instance with incorrect binary path');
234+
ok(-d$newnode->data_dir ."/pg_upgrade_output.d",
235+
"pg_upgrade_output.d/ not removed after pg_upgrade failure");
236+
rmtree($newnode->data_dir ."/pg_upgrade_output.d");
237+
238+
# --check command works here, cleans up pg_upgrade_output.d.
239+
command_ok(
240+
[
241+
'pg_upgrade','--no-sync','-d',$oldnode->data_dir,
242+
'-D',$newnode->data_dir,'-b',$oldbindir,
243+
'-B',$newbindir,'-p',$oldnode->port,
244+
'-P',$newnode->port,'--check'
245+
],
246+
'run of pg_upgrade --check for new instance');
247+
ok( !-d$newnode->data_dir ."/pg_upgrade_output.d",
248+
"pg_upgrade_output.d/ not removed after pg_upgrade --check success");
249+
250+
# Actual run, pg_upgrade_output.d is removed at the end.
216251
command_ok(
217252
[
218253
'pg_upgrade','--no-sync','-d',$oldnode->data_dir,
@@ -221,14 +256,24 @@ sub generate_db
221256
'-P',$newnode->port
222257
],
223258
'run of pg_upgrade for new instance');
259+
ok( !-d$newnode->data_dir ."/pg_upgrade_output.d",
260+
"pg_upgrade_output.d/ removed after pg_upgrade success");
261+
224262
$newnode->start;
225263

226264
# Check if there are any logs coming from pg_upgrade, that would only be
227265
# retained on failure.
228-
my$log_path =$newnode->data_dir ."/pg_upgrade_output.d/log";
266+
my$log_path =$newnode->data_dir ."/pg_upgrade_output.d";
229267
if (-d$log_path)
230268
{
231-
foreachmy$log (glob("$log_path/*"))
269+
my@log_files;
270+
find(
271+
sub {
272+
push@log_files,$File::Find::name
273+
if$File::Find::name =~m/.*\.log/;
274+
},
275+
$newnode->data_dir ."/pg_upgrade_output.d");
276+
foreachmy$log (@log_files)
232277
{
233278
note"=== contents of$log ===\n";
234279
print slurp_file($log);

‎src/bin/pg_upgrade/util.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,48 @@ end_progress_output(void)
5555
pg_log(PG_REPORT,"%-*s",MESSAGE_WIDTH,"");
5656
}
5757

58+
/*
59+
* Remove any logs generated internally. To be used once when exiting.
60+
*/
61+
void
62+
cleanup_output_dirs(void)
63+
{
64+
fclose(log_opts.internal);
65+
66+
/* Remove dump and log files? */
67+
if (log_opts.retain)
68+
return;
69+
70+
(void)rmtree(log_opts.basedir, true);
71+
72+
/* Remove pg_upgrade_output.d only if empty */
73+
switch (pg_check_dir(log_opts.rootdir))
74+
{
75+
case0:/* non-existent */
76+
case3:/* exists and contains a mount point */
77+
Assert(false);
78+
break;
79+
80+
case1:/* exists and empty */
81+
case2:/* exists and contains only dot files */
82+
(void)rmtree(log_opts.rootdir, true);
83+
break;
84+
85+
case4:/* exists */
86+
87+
/*
88+
* Keep the root directory as this includes some past log
89+
* activity.
90+
*/
91+
break;
92+
93+
default:
94+
/* different failure, just report it */
95+
pg_log(PG_WARNING,"could not access directory \"%s\": %m",
96+
log_opts.rootdir);
97+
break;
98+
}
99+
}
58100

59101
/*
60102
* prep_status

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp