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

Commit7fbe0c8

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 parent01c3adc commit7fbe0c8

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
@@ -88,26 +88,29 @@ static bool
8888
open_walfile(StreamCtl*stream,XLogRecPtrstartpoint)
8989
{
9090
Walfile*f;
91-
charfn[MAXPGPATH];
91+
char*fn;
9292
ssize_tsize;
9393
XLogSegNosegno;
9494

9595
XLByteToSeg(startpoint,segno,WalSegSz);
9696
XLogFileName(current_walfile_name,stream->timeline,segno,WalSegSz);
9797

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

101102
/*
102103
* When streaming to files, if an existing file exists we verify that it's
103104
* either empty (just created), or a complete WalSegSz segment (in which
104105
* case it has been created and padded). Anything else indicates a corrupt
105-
* file.
106+
* file. Compressed files have no need for padding, so just ignore this
107+
* case.
106108
*
107109
* When streaming to tar, no file with this name will exist before, so we
108110
* never have to verify a size.
109111
*/
110-
if (stream->walmethod->existsfile(fn))
112+
if (stream->walmethod->compression()==0&&
113+
stream->walmethod->existsfile(fn))
111114
{
112115
size=stream->walmethod->get_file_size(fn);
113116
if (size<0)

‎src/bin/pg_basebackup/walmethods.c

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

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

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

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

333+
staticint
334+
dir_compression(void)
335+
{
336+
returndir_data->compression;
337+
}
338+
316339
staticbool
317340
dir_existsfile(constchar*pathname)
318341
{
@@ -355,6 +378,8 @@ CreateWalDirectoryMethod(const char *basedir, int compression, bool sync)
355378
method->write=dir_write;
356379
method->get_current_pos=dir_get_current_pos;
357380
method->get_file_size=dir_get_file_size;
381+
method->get_file_name=dir_get_file_name;
382+
method->compression=dir_compression;
358383
method->close=dir_close;
359384
method->sync=dir_sync;
360385
method->existsfile=dir_existsfile;
@@ -527,11 +552,22 @@ tar_write_padding_data(TarMethodFile *f, size_t bytes)
527552
return true;
528553
}
529554

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

536572
tar_clear_error();
537573

@@ -583,8 +619,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
583619

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

586-
snprintf(tmppath,sizeof(tmppath),"%s%s",
587-
pathname,temp_suffix ?temp_suffix :"");
622+
tmppath=tar_get_file_name(pathname,temp_suffix);
588623

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

727+
staticint
728+
tar_compression(void)
729+
{
730+
returntar_data->compression;
731+
}
732+
692733
staticoff_t
693734
tar_get_current_pos(Walfilef)
694735
{
@@ -994,6 +1035,8 @@ CreateWalTarMethod(const char *tarbase, int compression, bool sync)
9941035
method->write=tar_write;
9951036
method->get_current_pos=tar_get_current_pos;
9961037
method->get_file_size=tar_get_file_size;
1038+
method->get_file_name=tar_get_file_name;
1039+
method->compression=tar_compression;
9971040
method->close=tar_close;
9981041
method->sync=tar_sync;
9991042
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