From d5dd101c7c360706fb28f977703ceb1a3aadf201 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 17 Aug 2022 07:05:01 +0000 Subject: [PATCH v16] Make more use of get_dirent_type() in place of stat() or lstat() We don't need to call stat() or lstat() in several ReadDir() loops, if d_type is available, as it usually is on most systems. If not, get_dirent_type() will do that under the covers. Authors: Nathan Bossart, Bharath Rupireddy Reviewed-by: Thomas Munro Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20220215231123.GA2584239%40nathanxps13 --- src/backend/access/heap/rewriteheap.c | 7 +++- src/backend/access/transam/xlog.c | 17 ++++---- .../replication/logical/reorderbuffer.c | 28 +++++++++---- 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 | 20 +++++---- src/backend/utils/misc/guc-file.l | 42 +++++++------------ src/timezone/pgtz.c | 8 +--- 9 files changed, 78 insertions(+), 77 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 9dd885d936..5b20d1e1d5 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, LOG); + + 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 87b243e0d4..16363a8f84 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 char *segname, const struct dirent *xlde, + XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo, + TimeLineID insertTLI); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); static void CleanupBackupHistory(void); @@ -3597,7 +3598,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, + RemoveXlogFile(xlde->d_name, xlde, recycleSegNo, &endlogSegNo, insertTLI); } } @@ -3670,7 +3671,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, + RemoveXlogFile(xlde->d_name, xlde, recycleSegNo, &endLogSegNo, newTLI); } } @@ -3692,14 +3693,14 @@ 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 char *segname, const struct dirent *xlde, + XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo, + TimeLineID insertTLI) { char path[MAXPGPATH]; #ifdef WIN32 char newpath[MAXPGPATH]; #endif - struct stat statbuf; snprintf(path, MAXPGPATH, XLOGDIR "/%s", segname); @@ -3711,7 +3712,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, xlde, false, LOG) == PGFILETYPE_REG && InstallXLogFileSegment(endlogSegNo, path, true, recycleSegNo, insertTLI)) { diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 1c21a1d14b..f61c5b747e 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -91,6 +91,7 @@ #include "access/xact.h" #include "access/xlog_internal.h" #include "catalog/catalog.h" +#include "common/file_utils.h" #include "lib/binaryheap.h" #include "miscadmin.h" #include "pgstat.h" @@ -254,7 +255,8 @@ static void ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn, static void ReorderBufferRestoreCleanup(ReorderBuffer *rb, ReorderBufferTXN *txn); static void ReorderBufferTruncateTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, bool txn_prepared); -static void ReorderBufferCleanupSerializedTXNs(const char *slotname); +static void ReorderBufferCleanupSerializedTXNs(const char *slotname, + bool slot_dir_checked); static void ReorderBufferSerializedPath(char *path, ReplicationSlot *slot, TransactionId xid, XLogSegNo segno); @@ -375,7 +377,8 @@ ReorderBufferAllocate(void) * prior exit avoided calling ReorderBufferFree. Failure to do this can * produce duplicated txns, and it's very cheap if there's nothing there. */ - ReorderBufferCleanupSerializedTXNs(NameStr(MyReplicationSlot->data.name)); + ReorderBufferCleanupSerializedTXNs(NameStr(MyReplicationSlot->data.name), + false); return buffer; } @@ -395,7 +398,8 @@ ReorderBufferFree(ReorderBuffer *rb) MemoryContextDelete(context); /* Free disk space used by unconsumed reorder buffers */ - ReorderBufferCleanupSerializedTXNs(NameStr(MyReplicationSlot->data.name)); + ReorderBufferCleanupSerializedTXNs(NameStr(MyReplicationSlot->data.name), + false); } /* @@ -4464,7 +4468,7 @@ ReorderBufferRestoreCleanup(ReorderBuffer *rb, ReorderBufferTXN *txn) * prior crash or decoding session exit. */ static void -ReorderBufferCleanupSerializedTXNs(const char *slotname) +ReorderBufferCleanupSerializedTXNs(const char *slotname, bool slot_dir_checked) { DIR *spill_dir; struct dirent *spill_de; @@ -4473,8 +4477,12 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname) sprintf(path, "pg_replslot/%s", slotname); - /* we're only handling directories here, skip if it's not ours */ - if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) + /* + * We're only handling directories here, skip if it's not ours. Also, skip + * if the caller has already performed this check. + */ + if (!slot_dir_checked && + lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode)) return; spill_dir = AllocateDir(path); @@ -4524,6 +4532,7 @@ StartupReorderBuffer(void) { DIR *logical_dir; struct dirent *logical_de; + char path[MAXPGPATH * 2 + 12]; logical_dir = AllocateDir("pg_replslot"); while ((logical_de = ReadDir(logical_dir, "pg_replslot")) != NULL) @@ -4536,11 +4545,16 @@ StartupReorderBuffer(void) if (!ReplicationSlotValidateName(logical_de->d_name, DEBUG2)) continue; + /* we're only handling directories here, skip if it's not one */ + sprintf(path, "pg_replslot/%s", logical_de->d_name); + if (get_dirent_type(path, logical_de, false, LOG) != PGFILETYPE_DIR) + continue; + /* * ok, has to be a surviving logical slot, iterate and delete * everything starting with xid-* */ - ReorderBufferCleanupSerializedTXNs(logical_de->d_name); + ReorderBufferCleanupSerializedTXNs(logical_de->d_name, true); } FreeDir(logical_dir); } diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1ff2c12240..f90059bb4e 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" @@ -2049,15 +2050,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, LOG); - 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..e9b926d7de 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, LOG); /* 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 e3b19ca1ed..bd6f1270f9 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2706,6 +2706,10 @@ TryAgain: * * The pathname passed to AllocateDir must be passed to this routine too, * but it is only used for error reporting. + * + * If you need to discover the file type of an entry, consider using + * get_dirent_type() (instead of stat()/lstat()) to avoid extra system calls on + * many platforms. */ struct dirent * ReadDir(DIR *dir, const char *dirname) @@ -2721,6 +2725,10 @@ ReadDir(DIR *dir, const char *dirname) * If elevel < ERROR, returns NULL after any error. With the normal coding * pattern, this will result in falling out of the loop immediately as * though the directory contained no (more) entries. + * + * If you need to discover the file type of an entry, consider using + * get_dirent_type() (instead of stat()/lstat()) to avoid extra system calls on + * many platforms. */ struct dirent * ReadDirExtended(DIR *dir, const char *dirname, int elevel) @@ -3157,17 +3165,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) -- 2.34.1