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

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 9dd885d936f5d94f48f9c9ba7479f0e5aeeb2674..2f08fbe8d31d3eca76de59426e15d7c8e081b461 100644 (file)
--- 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 fbf2d34eef9f7101af43cca0117f5babff4451d1..7a710e6490dbc7e3b24b917fc0f982c74e1530c7 100644 (file)
--- 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 bf72ad45ec7334dbbfb9195d4dd916ffd4f8e711..1d8ebb4c0d0d287310801dd8c1f02d43eeffb547 100644 (file)
--- 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 850b74936fe65315be387f079b67fb7cf1dfa72d..8fec1cb4a54bf900d8dd43f94d8fd276aaf12b52 100644 (file)
--- 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 658fd95ba950c7055787d274cb1d1ae4ab6d4856..8a866191e1fbf142749a7f5d95875f12be7292bf 100644 (file)
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -22,6 +22,7 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#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 1aaab6c5542caffc619f0f6c606fcab4c40f56f0..20c3741aa1ee2861552282d2d491fe24e1d4dd1a 100644 (file)
--- 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 ce5633844c343230ef509c24f318591f4fa4f604..88460422dd398c378d9a9dacaeb2aa460cbae1db 100644 (file)
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -14,6 +14,7 @@
 #include <ctype.h>
 #include <unistd.h>
 
+#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 3c278dd0f33800c005fbf6be7b5924be40cb4317..72f9e3d5e6cc832243a062668a156b3c861b15e1 100644 (file)
--- a/src/timezone/pgtz.c
+++ b/src/timezone/pgtz.c
@@ -17,6 +17,7 @@
 #include <sys/stat.h>
 #include <time.h>
 
+#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)