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

Commit5eefe74

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 parent93acb89 commit5eefe74

File tree

3 files changed

+34
-8
lines changed

3 files changed

+34
-8
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2844,10 +2844,16 @@ PreallocXlogFiles(XLogRecPtr endptr)
28442844
* existed while the server has been running, as this function always
28452845
* succeeds if no WAL segments have been removed since startup.
28462846
* 'tli' is only used in the error message.
2847+
*
2848+
* Note: this function guarantees to keep errno unchanged on return.
2849+
* This supports callers that use this to possibly deliver a better
2850+
* error message about a missing file, while still being able to throw
2851+
* a normal file-access error afterwards, if this does return.
28472852
*/
28482853
void
28492854
CheckXLogRemoved(XLogSegNosegno,TimeLineIDtli)
28502855
{
2856+
intsave_errno=errno;
28512857
/* use volatile pointer to prevent code rearrangement */
28522858
volatileXLogCtlData*xlogctl=XLogCtl;
28532859
XLogSegNolastRemovedSegNo;
@@ -2861,11 +2867,13 @@ CheckXLogRemoved(XLogSegNo segno, TimeLineID tli)
28612867
charfilename[MAXFNAMELEN];
28622868

28632869
XLogFileName(filename,tli,segno);
2870+
errno=save_errno;
28642871
ereport(ERROR,
28652872
(errcode_for_file_access(),
28662873
errmsg("requested WAL segment %s has already been removed",
28672874
filename)));
28682875
}
2876+
errno=save_errno;
28692877
}
28702878

28712879
/*

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,6 @@ static intFileAccess(File file);
297297
staticFileOpenTemporaryFileInTablespace(OidtblspcOid,boolrejectError);
298298
staticboolreserveAllocatedDesc(void);
299299
staticintFreeDesc(AllocateDesc*desc);
300-
staticstructdirent*ReadDirExtended(DIR*dir,constchar*dirname,intelevel);
301300

302301
staticvoidAtProcExit_Files(intcode,Datumarg);
303302
staticvoidCleanupTempFiles(boolisProcExit);
@@ -2084,6 +2083,10 @@ CloseTransientFile(int fd)
20842083
* necessary to open the directory, and with closing it after an elog.
20852084
* When done, call FreeDir rather than closedir.
20862085
*
2086+
* Returns NULL, with errno set, on failure. Note that failure detection
2087+
* is commonly left to the following call of ReadDir or ReadDirExtended;
2088+
* see the comments for ReadDir.
2089+
*
20872090
* Ideally this should be the *only* direct call of opendir() in the backend.
20882091
*/
20892092
DIR*
@@ -2146,8 +2149,8 @@ AllocateDir(const char *dirname)
21462149
*FreeDir(dir);
21472150
*
21482151
* since a NULL dir parameter is taken as indicating AllocateDir failed.
2149-
* (Make sure errnohasn'tbeenchangedsince AllocateDirif you use this
2150-
* shortcut.)
2152+
* (Make sure errnoisn't changedbetween AllocateDirand ReadDir if you
2153+
*use thisshortcut.)
21512154
*
21522155
* The pathname passed to AllocateDir must be passed to this routine too,
21532156
* but it is only used for error reporting.
@@ -2159,10 +2162,15 @@ ReadDir(DIR *dir, const char *dirname)
21592162
}
21602163

21612164
/*
2162-
* Alternate version that allows caller to specify the elevel for any
2163-
* error report. If elevel < ERROR, returns NULL on any error.
2165+
* Alternate version of ReadDir that allows caller to specify the elevel
2166+
* for any error report (whether it's reporting an initial failure of
2167+
* AllocateDir or a subsequent directory read failure).
2168+
*
2169+
* If elevel < ERROR, returns NULL after any error. With the normal coding
2170+
* pattern, this will result in falling out of the loop immediately as
2171+
* though the directory contained no (more) entries.
21642172
*/
2165-
staticstructdirent*
2173+
structdirent*
21662174
ReadDirExtended(DIR*dir,constchar*dirname,intelevel)
21672175
{
21682176
structdirent*dent;
@@ -2198,14 +2206,22 @@ ReadDirExtended(DIR *dir, const char *dirname, int elevel)
21982206
/*
21992207
* Close a directory opened with AllocateDir.
22002208
*
2201-
* Note we do not check closedir's return value --- it is up to the caller
2202-
* to handle close errors.
2209+
* Returns closedir's return value (with errno set if it's not 0).
2210+
* Note we do not check the return value --- it is up to the caller
2211+
* to handle close errors if wanted.
2212+
*
2213+
* Does nothing if dir == NULL; we assume that directory open failure was
2214+
* already reported if desired.
22032215
*/
22042216
int
22052217
FreeDir(DIR*dir)
22062218
{
22072219
inti;
22082220

2221+
/* Nothing to do if AllocateDir failed */
2222+
if (dir==NULL)
2223+
return0;
2224+
22092225
DO_DB(elog(LOG,"FreeDir: Allocated %d",numAllocatedDescs));
22102226

22112227
/* Remove dir from list of allocated dirs, if it's present */

‎src/include/storage/fd.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ extern intClosePipeStream(FILE *file);
8787
/* Operations to allow use of the <dirent.h> library routines */
8888
externDIR*AllocateDir(constchar*dirname);
8989
externstructdirent*ReadDir(DIR*dir,constchar*dirname);
90+
externstructdirent*ReadDirExtended(DIR*dir,constchar*dirname,
91+
intelevel);
9092
externintFreeDir(DIR*dir);
9193

9294
/* Operations to allow use of a plain kernel FD, with automatic cleanup */

0 commit comments

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