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

Commit795a916

Browse files
committed
Fix some issues with WAL segment opening for pg_receivewal --compress
The logic handling the opening of new WAL segments was fuzzy when using--compress if a partial, non-compressed, segment with the same base nameexisted in the repository storing those files. In this case, using--compress would cause the code to first check for the existence and thesize of a non-compressed segment, followed by the opening of a newcompressed, partial, segment. The code was accidentally workingcorrectly on most platforms as the buildfarm has proved, exceptbowerbird where gzflush() could fail in this code path. It is wronganyway to take the code path used pre-padding when creating a newpartial, non-compressed, segment, so let's fix it.Note that this issue exists when users mix successive runs ofpg_receivewal with or without compression, as discovered with the testsintroduced byffc9dda.While on it, this refactors the code so as code paths that need to knowabout the ".gz" suffix are down from four to one in walmethods.c, easinga bit the introduction of new compression methods. This addresses asecond issue where log messages generated for an unexpected failurewould not show the compressed segment name involved, which wasconfusing, printing instead the name of the non-compressed equivalent.Reported-by: Georgios KokolatosDiscussion:https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyzBackpatch-through: 10
1 parenteb158e7 commit795a916

File tree

3 files changed

+78
-23
lines changed

3 files changed

+78
-23
lines changed

‎src/bin/pg_basebackup/receivelog.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,26 +91,29 @@ static bool
9191
open_walfile(StreamCtl*stream,XLogRecPtrstartpoint)
9292
{
9393
Walfile*f;
94-
charfn[MAXPGPATH];
94+
char*fn;
9595
ssize_tsize;
9696
XLogSegNosegno;
9797

9898
XLByteToSeg(startpoint,segno,WalSegSz);
9999
XLogFileName(current_walfile_name,stream->timeline,segno,WalSegSz);
100100

101-
snprintf(fn,sizeof(fn),"%s%s",current_walfile_name,
102-
stream->partial_suffix ?stream->partial_suffix :"");
101+
/* Note that this considers the compression used if necessary */
102+
fn=stream->walmethod->get_file_name(current_walfile_name,
103+
stream->partial_suffix);
103104

104105
/*
105106
* When streaming to files, if an existing file exists we verify that it's
106107
* either empty (just created), or a complete WalSegSz segment (in which
107108
* case it has been created and padded). Anything else indicates a corrupt
108-
* file.
109+
* file. Compressed files have no need for padding, so just ignore this
110+
* case.
109111
*
110112
* When streaming to tar, no file with this name will exist before, so we
111113
* never have to verify a size.
112114
*/
113-
if (stream->walmethod->existsfile(fn))
115+
if (stream->walmethod->compression()==0&&
116+
stream->walmethod->existsfile(fn))
114117
{
115118
size=stream->walmethod->get_file_size(fn);
116119
if (size<0)

‎src/bin/pg_basebackup/walmethods.c

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,20 +69,32 @@ dir_getlasterror(void)
6969
returnstrerror(errno);
7070
}
7171

72+
staticchar*
73+
dir_get_file_name(constchar*pathname,constchar*temp_suffix)
74+
{
75+
char*filename=pg_malloc0(MAXPGPATH*sizeof(char));
76+
77+
snprintf(filename,MAXPGPATH,"%s%s%s",
78+
pathname,dir_data->compression>0 ?".gz" :"",
79+
temp_suffix ?temp_suffix :"");
80+
81+
returnfilename;
82+
}
83+
7284
staticWalfile
7385
dir_open_for_write(constchar*pathname,constchar*temp_suffix,size_tpad_to_size)
7486
{
7587
staticchartmppath[MAXPGPATH];
88+
char*filename;
7689
intfd;
7790
DirectoryMethodFile*f;
7891
#ifdefHAVE_LIBZ
7992
gzFilegzfp=NULL;
8093
#endif
8194

82-
snprintf(tmppath,sizeof(tmppath),"%s/%s%s%s",
83-
dir_data->basedir,pathname,
84-
dir_data->compression>0 ?".gz" :"",
85-
temp_suffix ?temp_suffix :"");
95+
filename=dir_get_file_name(pathname,temp_suffix);
96+
snprintf(tmppath,sizeof(tmppath),"%s/%s",
97+
dir_data->basedir,filename);
8698

8799
/*
88100
* Open a file for non-compressed as well as compressed files. Tracking
@@ -233,26 +245,31 @@ dir_close(Walfile f, WalCloseMethod method)
233245
/* Build path to the current version of the file */
234246
if (method==CLOSE_NORMAL&&df->temp_suffix)
235247
{
248+
char*filename;
249+
char*filename2;
250+
236251
/*
237252
* If we have a temp prefix, normal operation is to rename the
238253
* file.
239254
*/
240-
snprintf(tmppath,sizeof(tmppath),"%s/%s%s%s",
241-
dir_data->basedir,df->pathname,
242-
dir_data->compression>0 ?".gz" :"",
243-
df->temp_suffix);
244-
snprintf(tmppath2,sizeof(tmppath2),"%s/%s%s",
245-
dir_data->basedir,df->pathname,
246-
dir_data->compression>0 ?".gz" :"");
255+
filename=dir_get_file_name(df->pathname,df->temp_suffix);
256+
snprintf(tmppath,sizeof(tmppath),"%s/%s",
257+
dir_data->basedir,filename);
258+
259+
/* permanent name, so no need for the prefix */
260+
filename2=dir_get_file_name(df->pathname,NULL);
261+
snprintf(tmppath2,sizeof(tmppath2),"%s/%s",
262+
dir_data->basedir,filename2);
247263
r=durable_rename(tmppath,tmppath2,progname);
248264
}
249265
elseif (method==CLOSE_UNLINK)
250266
{
267+
char*filename;
268+
251269
/* Unlink the file once it's closed */
252-
snprintf(tmppath,sizeof(tmppath),"%s/%s%s%s",
253-
dir_data->basedir,df->pathname,
254-
dir_data->compression>0 ?".gz" :"",
255-
df->temp_suffix ?df->temp_suffix :"");
270+
filename=dir_get_file_name(df->pathname,df->temp_suffix);
271+
snprintf(tmppath,sizeof(tmppath),"%s/%s",
272+
dir_data->basedir,filename);
256273
r=unlink(tmppath);
257274
}
258275
else
@@ -314,6 +331,12 @@ dir_get_file_size(const char *pathname)
314331
returnstatbuf.st_size;
315332
}
316333

334+
staticint
335+
dir_compression(void)
336+
{
337+
returndir_data->compression;
338+
}
339+
317340
staticbool
318341
dir_existsfile(constchar*pathname)
319342
{
@@ -356,6 +379,8 @@ CreateWalDirectoryMethod(const char *basedir, int compression, bool sync)
356379
method->write=dir_write;
357380
method->get_current_pos=dir_get_current_pos;
358381
method->get_file_size=dir_get_file_size;
382+
method->get_file_name=dir_get_file_name;
383+
method->compression=dir_compression;
359384
method->close=dir_close;
360385
method->sync=dir_sync;
361386
method->existsfile=dir_existsfile;
@@ -528,11 +553,22 @@ tar_write_padding_data(TarMethodFile *f, size_t bytes)
528553
return true;
529554
}
530555

556+
staticchar*
557+
tar_get_file_name(constchar*pathname,constchar*temp_suffix)
558+
{
559+
char*filename=pg_malloc0(MAXPGPATH*sizeof(char));
560+
561+
snprintf(filename,MAXPGPATH,"%s%s",
562+
pathname,temp_suffix ?temp_suffix :"");
563+
564+
returnfilename;
565+
}
566+
531567
staticWalfile
532568
tar_open_for_write(constchar*pathname,constchar*temp_suffix,size_tpad_to_size)
533569
{
534570
intsave_errno;
535-
staticchartmppath[MAXPGPATH];
571+
char*tmppath;
536572

537573
tar_clear_error();
538574

@@ -584,8 +620,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
584620

585621
tar_data->currentfile=pg_malloc0(sizeof(TarMethodFile));
586622

587-
snprintf(tmppath,sizeof(tmppath),"%s%s",
588-
pathname,temp_suffix ?temp_suffix :"");
623+
tmppath=tar_get_file_name(pathname,temp_suffix);
589624

590625
/* Create a header with size set to 0 - we will fill out the size on close */
591626
if (tarCreateHeader(tar_data->currentfile->header,tmppath,NULL,0,S_IRUSR |S_IWUSR,0,0,time(NULL))!=TAR_OK)
@@ -686,6 +721,12 @@ tar_get_file_size(const char *pathname)
686721
return-1;
687722
}
688723

724+
staticint
725+
tar_compression(void)
726+
{
727+
returntar_data->compression;
728+
}
729+
689730
staticoff_t
690731
tar_get_current_pos(Walfilef)
691732
{
@@ -990,6 +1031,8 @@ CreateWalTarMethod(const char *tarbase, int compression, bool sync)
9901031
method->write=tar_write;
9911032
method->get_current_pos=tar_get_current_pos;
9921033
method->get_file_size=tar_get_file_size;
1034+
method->get_file_name=tar_get_file_name;
1035+
method->compression=tar_compression;
9931036
method->close=tar_close;
9941037
method->sync=tar_sync;
9951038
method->existsfile=tar_existsfile;

‎src/bin/pg_basebackup/walmethods.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ struct WalWriteMethod
5252
/* Return the size of a file, or -1 on failure. */
5353
ssize_t(*get_file_size) (constchar*pathname);
5454

55+
/*
56+
* Return the name of the current file to work on, without the base
57+
* directory. This is useful for logging.
58+
*/
59+
char*(*get_file_name) (constchar*pathname,constchar*temp_suffix);
60+
61+
/* Return the level of compression */
62+
int(*compression) (void);
63+
5564
/*
5665
* Write count number of bytes to the file, and return the number of bytes
5766
* actually written or -1 for error.

0 commit comments

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