Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 561885d

Browse files
committed
Improve error handling in RemovePgTempFiles().
Modify this function and its subsidiaries so that syscall failures are reported via ereport(LOG), rather than silently ignored as before. We don't want to throw a hard ERROR, as that would prevent database startup, and getting rid of leftover temporary files is not important enough for that. On the other hand, not reporting trouble at all seems like an odd choice not in line with current project norms, especially since any failure here is quite unexpected. On the same reasoning, adjust these functions' AllocateDir/ReadDir calls so that failure to scan a directory results in LOG not ERROR. I also removed the previous practice of silently ignoring ENOENT failures during directory opens --- there are some corner cases where that could happen given a previous database crash, but that seems like a bad excuse for ignoring a condition that isn't expected in most cases. A LOG message during postmaster start seems OK in such situations, and better than no output at all. In passing, make RemovePgTempRelationFiles' test for "is the file name all digits" look more like the way it's done elsewhere. Discussion: https://postgr.es/m/19907.1512402254@sss.pgh.pa.us
1 parent 2069e6f commit 561885d

File tree

1 file changed

+35
-35
lines changed
  • src/backend/storage/file

1 file changed

+35
-35
lines changed

src/backend/storage/file/fd.c

+35-35
Original file line numberDiff line numberDiff line change
@@ -2994,6 +2994,10 @@ CleanupTempFiles(bool isProcExit)
29942994
* the temp files for debugging purposes. This does however mean that
29952995
* OpenTemporaryFile had better allow for collision with an existing temp
29962996
* file name.
2997+
*
2998+
* NOTE: this function and its subroutines generally report syscall failures
2999+
* with ereport(LOG) and keep going. Removing temp files is not so critical
3000+
* that we should fail to start the database when we can't do it.
29973001
*/
29983002
void
29993003
RemovePgTempFiles(void)
@@ -3014,7 +3018,7 @@ RemovePgTempFiles(void)
30143018
*/
30153019
spc_dir = AllocateDir("pg_tblspc");
30163020

3017-
while ((spc_de = ReadDir(spc_dir, "pg_tblspc")) != NULL)
3021+
while ((spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
30183022
{
30193023
if (strcmp(spc_de->d_name, ".") == 0 ||
30203024
strcmp(spc_de->d_name, "..") == 0)
@@ -3055,18 +3059,8 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool unlink_all)
30553059
char rm_path[MAXPGPATH * 2];
30563060

30573061
temp_dir = AllocateDir(tmpdirname);
3058-
if (temp_dir == NULL)
3059-
{
3060-
/* anything except ENOENT is fishy */
3061-
if (errno != ENOENT)
3062-
ereport(LOG,
3063-
(errcode_for_file_access(),
3064-
errmsg("could not open directory \"%s\": %m",
3065-
tmpdirname)));
3066-
return;
3067-
}
30683062

3069-
while ((temp_de = ReadDir(temp_dir, tmpdirname)) != NULL)
3063+
while ((temp_de = ReadDirExtended(temp_dir, tmpdirname, LOG)) != NULL)
30703064
{
30713065
if (strcmp(temp_de->d_name, ".") == 0 ||
30723066
strcmp(temp_de->d_name, "..") == 0)
@@ -3082,22 +3076,38 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool unlink_all)
30823076
{
30833077
struct stat statbuf;
30843078

3085-
/* note that we ignore any error here and below */
30863079
if (lstat(rm_path, &statbuf) < 0)
3080+
{
3081+
ereport(LOG,
3082+
(errcode_for_file_access(),
3083+
errmsg("could not stat file \"%s\": %m", rm_path)));
30873084
continue;
3085+
}
30883086

30893087
if (S_ISDIR(statbuf.st_mode))
30903088
{
3089+
/* recursively remove contents, then directory itself */
30913090
RemovePgTempFilesInDir(rm_path, true);
3092-
rmdir(rm_path);
3091+
3092+
if (rmdir(rm_path) < 0)
3093+
ereport(LOG,
3094+
(errcode_for_file_access(),
3095+
errmsg("could not remove directory \"%s\": %m",
3096+
rm_path)));
30933097
}
30943098
else
3095-
unlink(rm_path);
3099+
{
3100+
if (unlink(rm_path) < 0)
3101+
ereport(LOG,
3102+
(errcode_for_file_access(),
3103+
errmsg("could not remove file \"%s\": %m",
3104+
rm_path)));
3105+
}
30963106
}
30973107
else
3098-
elog(LOG,
3099-
"unexpected file found in temporary-files directory: \"%s\"",
3100-
rm_path);
3108+
ereport(LOG,
3109+
(errmsg("unexpected file found in temporary-files directory: \"%s\"",
3110+
rm_path)));
31013111
}
31023112

31033113
FreeDir(temp_dir);
@@ -3112,29 +3122,15 @@ RemovePgTempRelationFiles(const char *tsdirname)
31123122
char dbspace_path[MAXPGPATH * 2];
31133123

31143124
ts_dir = AllocateDir(tsdirname);
3115-
if (ts_dir == NULL)
3116-
{
3117-
/* anything except ENOENT is fishy */
3118-
if (errno != ENOENT)
3119-
ereport(LOG,
3120-
(errcode_for_file_access(),
3121-
errmsg("could not open directory \"%s\": %m",
3122-
tsdirname)));
3123-
return;
3124-
}
31253125

3126-
while ((de = ReadDir(ts_dir, tsdirname)) != NULL)
3126+
while ((de = ReadDirExtended(ts_dir, tsdirname, LOG)) != NULL)
31273127
{
3128-
int i = 0;
3129-
31303128
/*
31313129
* We're only interested in the per-database directories, which have
31323130
* numeric names. Note that this code will also (properly) ignore "."
31333131
* and "..".
31343132
*/
3135-
while (isdigit((unsigned char) de->d_name[i]))
3136-
++i;
3137-
if (de->d_name[i] != '\0' || i == 0)
3133+
if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
31383134
continue;
31393135

31403136
snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s",
@@ -3163,7 +3159,11 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
31633159
snprintf(rm_path, sizeof(rm_path), "%s/%s",
31643160
dbspacedirname, de->d_name);
31653161

3166-
unlink(rm_path); /* note we ignore any error */
3162+
if (unlink(rm_path) < 0)
3163+
ereport(LOG,
3164+
(errcode_for_file_access(),
3165+
errmsg("could not remove file \"%s\": %m",
3166+
rm_path)));
31673167
}
31683168

31693169
FreeDir(dbspace_dir);

0 commit comments

Comments
 (0)