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

Commit2069e6f

Browse files
committed
Clean up assorted messiness around AllocateDir() usage.
This patch fixes a couple of low-probability bugs that could lead toreporting an irrelevant errno value (and hence possibly a wrong SQLSTATE)concerning directory-open or file-open failures. It also fixes placeswhere we took shortcuts in reporting such errors, either by using eloginstead of ereport or by using ereport but forgetting to specify anerrcode. And it eliminates a lot of just plain redundant error-handlingcode.In service of all this, export fd.c's formerly-static functionReadDirExtended, so that external callers can make use of the codingpatterndir = AllocateDir(path);while ((de = ReadDirExtended(dir, path, LOG)) != NULL)if they'd like to treat directory-open failures as mere LOG conditionsrather than errors. Also fix FreeDir to be a no-op if we reach itwith dir == NULL, as such a coding pattern would cause.Then, remove code at many call sites that was throwing an error or logmessage for AllocateDir failure, as ReadDir or ReadDirExtended can handlethat job just fine. Aside from being a net code savings, this gets rid ofa lot of not-quite-up-to-snuff reports, as mentioned above. (In someplaces these changes result in replacing a custom error message such as"could not open tablespace directory" with more generic wording "could notopen directory", but it was agreed that the custom wording buys little aslong as we report the directory name.) In some other call sites where wecan't just remove code, change the error reports to be fullyproject-style-compliant.Also reorder code in restoreTwoPhaseData that was acquiring a lockbetween AllocateDir and ReadDir; in the unlikely but surely notimpossible case that LWLockAcquire changes errno, AllocateDir failureswould be misreported. There is no great value in opening the directorybefore acquiring TwoPhaseStateLock, so just do it in the other order.Also fix CheckXLogRemoved to guarantee that it preserves errno,as quite a number of call sites are implicitly assuming. (Again,it's unlikely but I think not impossible that errno could changeduring a SpinLockAcquire. If so, this function was broken for itsown purposes as well as breaking callers.)And change a few places that were using not-per-project-style messages,such as "could not read directory" when "could not open directory" ismore correct.Back-patch the exporting of ReadDirExtended, in case we have occasionto back-patch some fix that makes use of it; it's not needed right nowbut surely making it global is pretty harmless. Also back-patch therestoreTwoPhaseData and CheckXLogRemoved fixes. The rest of this isessentially cosmetic and need not get back-patched.Michael Paquier, with a bit of additional work by meDiscussion:https://postgr.es/m/CAB7nPqRpOCxjiirHmebEFhXVTK7V5Jvw4bz82p7Oimtsm3TyZA@mail.gmail.com
1 parentab6eaee commit2069e6f

File tree

17 files changed

+102
-153
lines changed

17 files changed

+102
-153
lines changed

‎contrib/adminpack/adminpack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ pg_logdir_ls(PG_FUNCTION_ARGS)
319319
if (!fctx->dirdesc)
320320
ereport(ERROR,
321321
(errcode_for_file_access(),
322-
errmsg("could notread directory \"%s\": %m",
322+
errmsg("could notopen directory \"%s\": %m",
323323
fctx->location)));
324324

325325
funcctx->user_fctx=fctx;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,8 +1735,8 @@ restoreTwoPhaseData(void)
17351735
DIR*cldir;
17361736
structdirent*clde;
17371737

1738-
cldir=AllocateDir(TWOPHASE_DIR);
17391738
LWLockAcquire(TwoPhaseStateLock,LW_EXCLUSIVE);
1739+
cldir=AllocateDir(TWOPHASE_DIR);
17401740
while ((clde=ReadDir(cldir,TWOPHASE_DIR))!=NULL)
17411741
{
17421742
if (strlen(clde->d_name)==8&&

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

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3764,10 +3764,16 @@ PreallocXlogFiles(XLogRecPtr endptr)
37643764
* existed while the server has been running, as this function always
37653765
* succeeds if no WAL segments have been removed since startup.
37663766
* 'tli' is only used in the error message.
3767+
*
3768+
* Note: this function guarantees to keep errno unchanged on return.
3769+
* This supports callers that use this to possibly deliver a better
3770+
* error message about a missing file, while still being able to throw
3771+
* a normal file-access error afterwards, if this does return.
37673772
*/
37683773
void
37693774
CheckXLogRemoved(XLogSegNosegno,TimeLineIDtli)
37703775
{
3776+
intsave_errno=errno;
37713777
XLogSegNolastRemovedSegNo;
37723778

37733779
SpinLockAcquire(&XLogCtl->info_lck);
@@ -3779,11 +3785,13 @@ CheckXLogRemoved(XLogSegNo segno, TimeLineID tli)
37793785
charfilename[MAXFNAMELEN];
37803786

37813787
XLogFileName(filename,tli,segno,wal_segment_size);
3788+
errno=save_errno;
37823789
ereport(ERROR,
37833790
(errcode_for_file_access(),
37843791
errmsg("requested WAL segment %s has already been removed",
37853792
filename)));
37863793
}
3794+
errno=save_errno;
37873795
}
37883796

37893797
/*
@@ -3837,13 +3845,6 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr)
38373845
structdirent*xlde;
38383846
charlastoff[MAXFNAMELEN];
38393847

3840-
xldir=AllocateDir(XLOGDIR);
3841-
if (xldir==NULL)
3842-
ereport(ERROR,
3843-
(errcode_for_file_access(),
3844-
errmsg("could not open write-ahead log directory \"%s\": %m",
3845-
XLOGDIR)));
3846-
38473848
/*
38483849
* Construct a filename of the last segment to be kept. The timeline ID
38493850
* doesn't matter, we ignore that in the comparison. (During recovery,
@@ -3854,6 +3855,8 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr)
38543855
elog(DEBUG2,"attempting to remove WAL segments older than log file %s",
38553856
lastoff);
38563857

3858+
xldir=AllocateDir(XLOGDIR);
3859+
38573860
while ((xlde=ReadDir(xldir,XLOGDIR))!=NULL)
38583861
{
38593862
/* Ignore files that are not XLOG segments */
@@ -3912,13 +3915,6 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI)
39123915

39133916
XLByteToPrevSeg(switchpoint,endLogSegNo,wal_segment_size);
39143917

3915-
xldir=AllocateDir(XLOGDIR);
3916-
if (xldir==NULL)
3917-
ereport(ERROR,
3918-
(errcode_for_file_access(),
3919-
errmsg("could not open write-ahead log directory \"%s\": %m",
3920-
XLOGDIR)));
3921-
39223918
/*
39233919
* Construct a filename of the last segment to be kept.
39243920
*/
@@ -3927,6 +3923,8 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI)
39273923
elog(DEBUG2,"attempting to remove WAL segments newer than log file %s",
39283924
switchseg);
39293925

3926+
xldir=AllocateDir(XLOGDIR);
3927+
39303928
while ((xlde=ReadDir(xldir,XLOGDIR))!=NULL)
39313929
{
39323930
/* Ignore files that are not XLOG segments */
@@ -4108,11 +4106,6 @@ CleanupBackupHistory(void)
41084106
charpath[MAXPGPATH+sizeof(XLOGDIR)];
41094107

41104108
xldir=AllocateDir(XLOGDIR);
4111-
if (xldir==NULL)
4112-
ereport(ERROR,
4113-
(errcode_for_file_access(),
4114-
errmsg("could not open write-ahead log directory \"%s\": %m",
4115-
XLOGDIR)));
41164109

41174110
while ((xlde=ReadDir(xldir,XLOGDIR))!=NULL)
41184111
{

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ pg_start_backup(PG_FUNCTION_ARGS)
8989
dir=AllocateDir("pg_tblspc");
9090
if (!dir)
9191
ereport(ERROR,
92-
(errmsg("could not open directory \"%s\": %m","pg_tblspc")));
92+
(errcode_for_file_access(),
93+
errmsg("could not open directory \"%s\": %m",
94+
"pg_tblspc")));
9395

9496
if (exclusive)
9597
{

‎src/backend/postmaster/pgarch.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -673,11 +673,6 @@ pgarch_readyXlog(char *xlog)
673673

674674
snprintf(XLogArchiveStatusDir,MAXPGPATH,XLOGDIR"/archive_status");
675675
rldir=AllocateDir(XLogArchiveStatusDir);
676-
if (rldir==NULL)
677-
ereport(ERROR,
678-
(errcode_for_file_access(),
679-
errmsg("could not open archive status directory \"%s\": %m",
680-
XLogArchiveStatusDir)));
681676

682677
while ((rlde=ReadDir(rldir,XLogArchiveStatusDir))!=NULL)
683678
{

‎src/backend/replication/basebackup.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,6 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
367367
XLogFileName(lastoff,ThisTimeLineID,endsegno,wal_segment_size);
368368

369369
dir=AllocateDir("pg_wal");
370-
if (!dir)
371-
ereport(ERROR,
372-
(errmsg("could not open directory \"%s\": %m","pg_wal")));
373370
while ((de=ReadDir(dir,"pg_wal"))!=NULL)
374371
{
375372
/* Does it look like a WAL segment, and is it in the range? */
@@ -713,7 +710,9 @@ SendBaseBackup(BaseBackupCmd *cmd)
713710
dir=AllocateDir("pg_tblspc");
714711
if (!dir)
715712
ereport(ERROR,
716-
(errmsg("could not open directory \"%s\": %m","pg_tblspc")));
713+
(errcode_for_file_access(),
714+
errmsg("could not open directory \"%s\": %m",
715+
"pg_tblspc")));
717716

718717
perform_base_backup(&opt,dir);
719718

‎src/backend/storage/file/copydir.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ copydir(char *fromdir, char *todir, bool recurse)
4747
errmsg("could not create directory \"%s\": %m",todir)));
4848

4949
xldir=AllocateDir(fromdir);
50-
if (xldir==NULL)
51-
ereport(ERROR,
52-
(errcode_for_file_access(),
53-
errmsg("could not open directory \"%s\": %m",fromdir)));
5450

5551
while ((xlde=ReadDir(xldir,fromdir))!=NULL)
5652
{
@@ -90,10 +86,6 @@ copydir(char *fromdir, char *todir, bool recurse)
9086
return;
9187

9288
xldir=AllocateDir(todir);
93-
if (xldir==NULL)
94-
ereport(ERROR,
95-
(errcode_for_file_access(),
96-
errmsg("could not open directory \"%s\": %m",todir)));
9789

9890
while ((xlde=ReadDir(xldir,todir))!=NULL)
9991
{

‎src/backend/storage/file/fd.c

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ static intFileAccess(File file);
318318
staticFileOpenTemporaryFileInTablespace(OidtblspcOid,boolrejectError);
319319
staticboolreserveAllocatedDesc(void);
320320
staticintFreeDesc(AllocateDesc*desc);
321-
staticstructdirent*ReadDirExtended(DIR*dir,constchar*dirname,intelevel);
322321

323322
staticvoidAtProcExit_Files(intcode,Datumarg);
324323
staticvoidCleanupTempFiles(boolisProcExit);
@@ -2587,6 +2586,10 @@ CloseTransientFile(int fd)
25872586
* necessary to open the directory, and with closing it after an elog.
25882587
* When done, call FreeDir rather than closedir.
25892588
*
2589+
* Returns NULL, with errno set, on failure. Note that failure detection
2590+
* is commonly left to the following call of ReadDir or ReadDirExtended;
2591+
* see the comments for ReadDir.
2592+
*
25902593
* Ideally this should be the *only* direct call of opendir() in the backend.
25912594
*/
25922595
DIR*
@@ -2649,8 +2652,8 @@ AllocateDir(const char *dirname)
26492652
*FreeDir(dir);
26502653
*
26512654
* since a NULL dir parameter is taken as indicating AllocateDir failed.
2652-
* (Make sure errnohasn'tbeenchangedsince AllocateDirif you use this
2653-
* shortcut.)
2655+
* (Make sure errnoisn't changedbetween AllocateDirand ReadDir if you
2656+
*use thisshortcut.)
26542657
*
26552658
* The pathname passed to AllocateDir must be passed to this routine too,
26562659
* but it is only used for error reporting.
@@ -2662,10 +2665,15 @@ ReadDir(DIR *dir, const char *dirname)
26622665
}
26632666

26642667
/*
2665-
* Alternate version that allows caller to specify the elevel for any
2666-
* error report. If elevel < ERROR, returns NULL on any error.
2668+
* Alternate version of ReadDir that allows caller to specify the elevel
2669+
* for any error report (whether it's reporting an initial failure of
2670+
* AllocateDir or a subsequent directory read failure).
2671+
*
2672+
* If elevel < ERROR, returns NULL after any error. With the normal coding
2673+
* pattern, this will result in falling out of the loop immediately as
2674+
* though the directory contained no (more) entries.
26672675
*/
2668-
staticstructdirent*
2676+
structdirent*
26692677
ReadDirExtended(DIR*dir,constchar*dirname,intelevel)
26702678
{
26712679
structdirent*dent;
@@ -2695,14 +2703,22 @@ ReadDirExtended(DIR *dir, const char *dirname, int elevel)
26952703
/*
26962704
* Close a directory opened with AllocateDir.
26972705
*
2698-
* Note we do not check closedir's return value --- it is up to the caller
2699-
* to handle close errors.
2706+
* Returns closedir's return value (with errno set if it's not 0).
2707+
* Note we do not check the return value --- it is up to the caller
2708+
* to handle close errors if wanted.
2709+
*
2710+
* Does nothing if dir == NULL; we assume that directory open failure was
2711+
* already reported if desired.
27002712
*/
27012713
int
27022714
FreeDir(DIR*dir)
27032715
{
27042716
inti;
27052717

2718+
/* Nothing to do if AllocateDir failed */
2719+
if (dir==NULL)
2720+
return0;
2721+
27062722
DO_DB(elog(LOG,"FreeDir: Allocated %d",numAllocatedDescs));
27072723

27082724
/* Remove dir from list of allocated dirs, if it's present */
@@ -3043,9 +3059,10 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool unlink_all)
30433059
{
30443060
/* anything except ENOENT is fishy */
30453061
if (errno!=ENOENT)
3046-
elog(LOG,
3047-
"could not open temporary-files directory \"%s\": %m",
3048-
tmpdirname);
3062+
ereport(LOG,
3063+
(errcode_for_file_access(),
3064+
errmsg("could not open directory \"%s\": %m",
3065+
tmpdirname)));
30493066
return;
30503067
}
30513068

@@ -3099,9 +3116,10 @@ RemovePgTempRelationFiles(const char *tsdirname)
30993116
{
31003117
/* anything except ENOENT is fishy */
31013118
if (errno!=ENOENT)
3102-
elog(LOG,
3103-
"could not open tablespace directory \"%s\": %m",
3104-
tsdirname);
3119+
ereport(LOG,
3120+
(errcode_for_file_access(),
3121+
errmsg("could not open directory \"%s\": %m",
3122+
tsdirname)));
31053123
return;
31063124
}
31073125

@@ -3136,16 +3154,8 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
31363154
charrm_path[MAXPGPATH*2];
31373155

31383156
dbspace_dir=AllocateDir(dbspacedirname);
3139-
if (dbspace_dir==NULL)
3140-
{
3141-
/* we just saw this directory, so it really ought to be there */
3142-
elog(LOG,
3143-
"could not open dbspace directory \"%s\": %m",
3144-
dbspacedirname);
3145-
return;
3146-
}
31473157

3148-
while ((de=ReadDir(dbspace_dir,dbspacedirname))!=NULL)
3158+
while ((de=ReadDirExtended(dbspace_dir,dbspacedirname,LOG))!=NULL)
31493159
{
31503160
if (!looks_like_temp_rel_name(de->d_name))
31513161
continue;
@@ -3310,13 +3320,6 @@ walkdir(const char *path,
33103320
structdirent*de;
33113321

33123322
dir=AllocateDir(path);
3313-
if (dir==NULL)
3314-
{
3315-
ereport(elevel,
3316-
(errcode_for_file_access(),
3317-
errmsg("could not open directory \"%s\": %m",path)));
3318-
return;
3319-
}
33203323

33213324
while ((de=ReadDirExtended(dir,path,elevel))!=NULL)
33223325
{
@@ -3356,9 +3359,11 @@ walkdir(const char *path,
33563359
/*
33573360
* It's important to fsync the destination directory itself as individual
33583361
* file fsyncs don't guarantee that the directory entry for the file is
3359-
* synced.
3362+
* synced. However, skip this if AllocateDir failed; the action function
3363+
* might not be robust against that.
33603364
*/
3361-
(*action) (path, true,elevel);
3365+
if (dir)
3366+
(*action) (path, true,elevel);
33623367
}
33633368

33643369

‎src/backend/storage/file/reinit.c

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op)
111111
{
112112
/* anything except ENOENT is fishy */
113113
if (errno!=ENOENT)
114-
elog(LOG,
115-
"could not open tablespace directory \"%s\": %m",
116-
tsdirname);
114+
ereport(LOG,
115+
(errcode_for_file_access(),
116+
errmsg("could not open directory \"%s\": %m",
117+
tsdirname)));
117118
return;
118119
}
119120

@@ -164,9 +165,10 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
164165
dbspace_dir=AllocateDir(dbspacedirname);
165166
if (dbspace_dir==NULL)
166167
{
167-
elog(LOG,
168-
"could not open dbspace directory \"%s\": %m",
169-
dbspacedirname);
168+
ereport(LOG,
169+
(errcode_for_file_access(),
170+
errmsg("could not open directory \"%s\": %m",
171+
dbspacedirname)));
170172
return;
171173
}
172174

@@ -226,9 +228,10 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
226228
dbspace_dir=AllocateDir(dbspacedirname);
227229
if (dbspace_dir==NULL)
228230
{
229-
elog(LOG,
230-
"could not open dbspace directory \"%s\": %m",
231-
dbspacedirname);
231+
ereport(LOG,
232+
(errcode_for_file_access(),
233+
errmsg("could not open directory \"%s\": %m",
234+
dbspacedirname)));
232235
hash_destroy(hash);
233236
return;
234237
}
@@ -296,9 +299,10 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
296299
if (dbspace_dir==NULL)
297300
{
298301
/* we just saw this directory, so it really ought to be there */
299-
elog(LOG,
300-
"could not open dbspace directory \"%s\": %m",
301-
dbspacedirname);
302+
ereport(LOG,
303+
(errcode_for_file_access(),
304+
errmsg("could not open directory \"%s\": %m",
305+
dbspacedirname)));
302306
return;
303307
}
304308

@@ -349,9 +353,10 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op)
349353
if (dbspace_dir==NULL)
350354
{
351355
/* we just saw this directory, so it really ought to be there */
352-
elog(LOG,
353-
"could not open dbspace directory \"%s\": %m",
354-
dbspacedirname);
356+
ereport(LOG,
357+
(errcode_for_file_access(),
358+
errmsg("could not open directory \"%s\": %m",
359+
dbspacedirname)));
355360
return;
356361
}
357362

0 commit comments

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