From bfb9dfd93720098cf8f3e7d802df9b02ebe3dc20 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 2 Sep 2022 16:58:06 +0900 Subject: [PATCH] Expand the use of get_dirent_type(), shaving a few calls to stat()/lstat() Several backend-side loops scanning one or more directories with ReadDir() (WAL segment recycle/removal in xlog.c, backend-side directory copy, temporary file removal, configuration file parsing, some logical decoding logic and some pgtz stuff) already know the type of the entry being scanned thanks to the dirent structure associated to the entry, on platforms where we know about DT_REG, DT_DIR and DT_LNK to make the difference between a regular file, a directory and a symbolic link. Relying on the direct structure of an entry saves a few system calls to stat() and lstat() in the loops updated here, shaving some code while on it. The logic of the code remains the same, calling stat() or lstat() depending on if it is necessary to look through symlinks. Authors: Nathan Bossart, Bharath Rupireddy Reviewed-by: Andres Freund, Thomas Munro, Michael Paquier Discussion: https://postgr.es/m/CALj2ACV8n-J-f=yiLUOx2=HrQGPSOZM3nWzyQQvLPcccPXxEdg@mail.gmail.com --- src/backend/access/heap/rewriteheap.c | 7 +++- src/backend/access/transam/xlog.c | 26 ++++++------- src/backend/replication/logical/snapbuild.c | 6 ++- src/backend/replication/slot.c | 6 ++- src/backend/storage/file/copydir.c | 21 +++-------- src/backend/storage/file/fd.c | 12 ++---- src/backend/utils/misc/guc-file.l | 42 ++++++++------------- src/timezone/pgtz.c | 8 +--- 8 files changed, 53 insertions(+), 75 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 9dd885d936..2f08fbe8d3 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -113,6 +113,7 @@ #include "access/xact.h" #include "access/xloginsert.h" #include "catalog/catalog.h" +#include "common/file_utils.h" #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" @@ -1213,7 +1214,6 @@ CheckPointLogicalRewriteHeap(void) mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { - struct stat statbuf; Oid dboid; Oid relid; XLogRecPtr lsn; @@ -1221,13 +1221,16 @@ CheckPointLogicalRewriteHeap(void) TransactionId create_xid; uint32 hi, lo; + PGFileType de_type; if (strcmp(mapping_de->d_name, ".") == 0 || strcmp(mapping_de->d_name, "..") == 0) continue; snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); - if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) continue; /* Skip over files that cannot be ours. */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index fbf2d34eef..7a710e6490 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -657,8 +657,9 @@ static void PreallocXlogFiles(XLogRecPtr endptr, TimeLineID tli); static void RemoveTempXlogFiles(void); static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr lastredoptr, XLogRecPtr endptr, TimeLineID insertTLI); -static void RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, - XLogSegNo *endlogSegNo, TimeLineID insertTLI); +static void RemoveXlogFile(const struct dirent *segment_de, + XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo, + TimeLineID insertTLI); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); static void CleanupBackupHistory(void); @@ -3596,8 +3597,7 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr lastredoptr, XLogRecPtr endptr, /* Update the last removed location in shared memory first */ UpdateLastRemovedPtr(xlde->d_name); - RemoveXlogFile(xlde->d_name, recycleSegNo, &endlogSegNo, - insertTLI); + RemoveXlogFile(xlde, recycleSegNo, &endlogSegNo, insertTLI); } } } @@ -3669,8 +3669,7 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) * - but seems safer to let them be archived and removed later. */ if (!XLogArchiveIsReady(xlde->d_name)) - RemoveXlogFile(xlde->d_name, recycleSegNo, &endLogSegNo, - newTLI); + RemoveXlogFile(xlde, recycleSegNo, &endLogSegNo, newTLI); } } @@ -3680,9 +3679,9 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) /* * Recycle or remove a log file that's no longer needed. * - * segname is the name of the segment to recycle or remove. recycleSegNo - * is the segment number to recycle up to. endlogSegNo is the segment - * number of the current (or recent) end of WAL. + * segment_de is the dirent structure of the segment to recycle or remove. + * recycleSegNo is the segment number to recycle up to. endlogSegNo is + * the segment number of the current (or recent) end of WAL. * * endlogSegNo gets incremented if the segment is recycled so as it is not * checked again with future callers of this function. @@ -3691,14 +3690,15 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) * should be used for this timeline. */ static void -RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, - XLogSegNo *endlogSegNo, TimeLineID insertTLI) +RemoveXlogFile(const struct dirent *segment_de, + XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo, + TimeLineID insertTLI) { char path[MAXPGPATH]; #ifdef WIN32 char newpath[MAXPGPATH]; #endif - struct stat statbuf; + const char *segname = segment_de->d_name; snprintf(path, MAXPGPATH, XLOGDIR "/%s", segname); @@ -3710,7 +3710,7 @@ RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, if (wal_recycle && *endlogSegNo <= recycleSegNo && XLogCtl->InstallXLogFileSegmentActive && /* callee rechecks this */ - lstat(path, &statbuf) == 0 && S_ISREG(statbuf.st_mode) && + get_dirent_type(path, segment_de, false, DEBUG2) == PGFILETYPE_REG && InstallXLogFileSegment(endlogSegNo, path, true, recycleSegNo, insertTLI)) { diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index bf72ad45ec..1d8ebb4c0d 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -123,6 +123,7 @@ #include "access/heapam_xlog.h" #include "access/transam.h" #include "access/xact.h" +#include "common/file_utils.h" #include "miscadmin.h" #include "pgstat.h" #include "replication/logical.h" @@ -2061,15 +2062,16 @@ CheckPointSnapBuild(void) uint32 hi; uint32 lo; XLogRecPtr lsn; - struct stat statbuf; + PGFileType de_type; if (strcmp(snap_de->d_name, ".") == 0 || strcmp(snap_de->d_name, "..") == 0) continue; snprintf(path, sizeof(path), "pg_logical/snapshots/%s", snap_de->d_name); + de_type = get_dirent_type(path, snap_de, false, DEBUG1); - if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) { elog(DEBUG1, "only regular files expected: %s", path); continue; diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 850b74936f..8fec1cb4a5 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -41,6 +41,7 @@ #include "access/transam.h" #include "access/xlog_internal.h" +#include "common/file_utils.h" #include "common/string.h" #include "miscadmin.h" #include "pgstat.h" @@ -1442,17 +1443,18 @@ StartupReplicationSlots(void) replication_dir = AllocateDir("pg_replslot"); while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL) { - struct stat statbuf; char path[MAXPGPATH + 12]; + PGFileType de_type; if (strcmp(replication_de->d_name, ".") == 0 || strcmp(replication_de->d_name, "..") == 0) continue; snprintf(path, sizeof(path), "pg_replslot/%s", replication_de->d_name); + de_type = get_dirent_type(path, replication_de, false, DEBUG1); /* we're only creating directories here, skip if it's not our's */ - if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_DIR) continue; /* we crashed while a slot was being setup or deleted, clean up */ diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index 658fd95ba9..8a866191e1 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -22,6 +22,7 @@ #include #include +#include "common/file_utils.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/copydir.h" @@ -50,7 +51,7 @@ copydir(char *fromdir, char *todir, bool recurse) while ((xlde = ReadDir(xldir, fromdir)) != NULL) { - struct stat fst; + PGFileType xlde_type; /* If we got a cancel signal during the copy of the directory, quit */ CHECK_FOR_INTERRUPTS(); @@ -62,18 +63,15 @@ copydir(char *fromdir, char *todir, bool recurse) snprintf(fromfile, sizeof(fromfile), "%s/%s", fromdir, xlde->d_name); snprintf(tofile, sizeof(tofile), "%s/%s", todir, xlde->d_name); - if (lstat(fromfile, &fst) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", fromfile))); + xlde_type = get_dirent_type(fromfile, xlde, false, ERROR); - if (S_ISDIR(fst.st_mode)) + if (xlde_type == PGFILETYPE_DIR) { /* recurse to handle subdirectories */ if (recurse) copydir(fromfile, tofile, true); } - else if (S_ISREG(fst.st_mode)) + else if (xlde_type == PGFILETYPE_REG) copy_file(fromfile, tofile); } FreeDir(xldir); @@ -89,8 +87,6 @@ copydir(char *fromdir, char *todir, bool recurse) while ((xlde = ReadDir(xldir, todir)) != NULL) { - struct stat fst; - if (strcmp(xlde->d_name, ".") == 0 || strcmp(xlde->d_name, "..") == 0) continue; @@ -101,12 +97,7 @@ copydir(char *fromdir, char *todir, bool recurse) * We don't need to sync subdirectories here since the recursive * copydir will do it before it returns */ - if (lstat(tofile, &fst) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", tofile))); - - if (S_ISREG(fst.st_mode)) + if (get_dirent_type(tofile, xlde, false, ERROR) == PGFILETYPE_REG) fsync_fname(tofile, false); } FreeDir(xldir); diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 1aaab6c554..20c3741aa1 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -3156,17 +3156,11 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all) PG_TEMP_FILE_PREFIX, strlen(PG_TEMP_FILE_PREFIX)) == 0) { - struct stat statbuf; + PGFileType type = get_dirent_type(rm_path, temp_de, false, LOG); - if (lstat(rm_path, &statbuf) < 0) - { - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", rm_path))); + if (type == PGFILETYPE_ERROR) continue; - } - - if (S_ISDIR(statbuf.st_mode)) + else if (type == PGFILETYPE_DIR) { /* recursively remove contents, then directory itself */ RemovePgTempFilesInDir(rm_path, false, true); diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index ce5633844c..88460422dd 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -14,6 +14,7 @@ #include #include +#include "common/file_utils.h" #include "mb/pg_wchar.h" #include "miscadmin.h" #include "storage/fd.h" @@ -1017,7 +1018,7 @@ ParseConfigDirectory(const char *includedir, while ((de = ReadDir(d, directory)) != NULL) { - struct stat st; + PGFileType de_type; char filename[MAXPGPATH]; /* @@ -1034,32 +1035,9 @@ ParseConfigDirectory(const char *includedir, join_path_components(filename, directory, de->d_name); canonicalize_path(filename); - if (stat(filename, &st) == 0) + de_type = get_dirent_type(filename, de, true, elevel); + if (de_type == PGFILETYPE_ERROR) { - if (!S_ISDIR(st.st_mode)) - { - /* Add file to array, increasing its size in blocks of 32 */ - if (num_filenames >= size_filenames) - { - size_filenames += 32; - filenames = (char **) repalloc(filenames, - size_filenames * sizeof(char *)); - } - filenames[num_filenames] = pstrdup(filename); - num_filenames++; - } - } - else - { - /* - * stat does not care about permissions, so the most likely reason - * a file can't be accessed now is if it was removed between the - * directory listing and now. - */ - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", - filename))); record_config_file_error(psprintf("could not stat file \"%s\"", filename), calling_file, calling_lineno, @@ -1067,6 +1045,18 @@ ParseConfigDirectory(const char *includedir, status = false; goto cleanup; } + else if (de_type != PGFILETYPE_DIR) + { + /* Add file to array, increasing its size in blocks of 32 */ + if (num_filenames >= size_filenames) + { + size_filenames += 32; + filenames = (char **) repalloc(filenames, + size_filenames * sizeof(char *)); + } + filenames[num_filenames] = pstrdup(filename); + num_filenames++; + } } if (num_filenames > 0) diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c index 3c278dd0f3..72f9e3d5e6 100644 --- a/src/timezone/pgtz.c +++ b/src/timezone/pgtz.c @@ -17,6 +17,7 @@ #include #include +#include "common/file_utils.h" #include "datatype/timestamp.h" #include "miscadmin.h" #include "pgtz.h" @@ -429,7 +430,6 @@ pg_tzenumerate_next(pg_tzenum *dir) { struct dirent *direntry; char fullname[MAXPGPATH * 2]; - struct stat statbuf; direntry = ReadDir(dir->dirdesc[dir->depth], dir->dirname[dir->depth]); @@ -447,12 +447,8 @@ pg_tzenumerate_next(pg_tzenum *dir) snprintf(fullname, sizeof(fullname), "%s/%s", dir->dirname[dir->depth], direntry->d_name); - if (stat(fullname, &statbuf) != 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat \"%s\": %m", fullname))); - if (S_ISDIR(statbuf.st_mode)) + if (get_dirent_type(fullname, direntry, true, ERROR) == PGFILETYPE_DIR) { /* Step into the subdirectory */ if (dir->depth >= MAX_TZDIR_DEPTH - 1)