Backpatch critical performance fixes to pgarch.c
authorÁlvaro Herrera
Thu, 12 Dec 2024 15:21:18 +0000 (16:21 +0100)
committerÁlvaro Herrera
Thu, 12 Dec 2024 15:21:18 +0000 (16:21 +0100)
This backpatches commits beb4e9ba1652 and 1fb17b190341 (originally
appearing in previously in REL_15_STABLE) to REL_14_STABLE.  Performance
of the WAL archiver can become pretty critical at times, and reports
exist of users getting in serious trouble (hours of downtime, loss of
replicas) because of lack of this optimization.

We'd like to backpatch these to REL_13_STABLE too, but because of the
very invasive changes made by commit d75288fb27b8 in the 14 timeframe,
we deem it too risky :-(

Original commit messages appear below.

Discussion: https://postgr.es/m/202411131605[email protected]

  commit beb4e9ba1652a04f66ff20261444d06f678c0b2d
  Author:     Robert Haas 
  AuthorDate: Thu Nov 11 15:02:53 2021 -0500

  Improve performance of pgarch_readyXlog() with many status files.

  Presently, the archive_status directory was scanned for each file to
  archive.  When there are many status files, say because archive_command
  has been failing for a long time, these directory scans can get very
  slow.  With this change, the archiver remembers several files to archive
  during each directory scan, speeding things up.

  To ensure timeline history files are archived as quickly as possible,
  XLogArchiveNotify() forces the archiver to do a new directory scan as
  soon as the .ready file for one is created.

  Nathan Bossart, per a long discussion involving many people. It is
  not clear to me exactly who out of all those people reviewed this
  particular patch.

  Discussion: http://postgr.es/m/CA+TgmobhAbs2yabTuTRkJTq_kkC80-+jw=pfpypdOJ7+gAbQbw@mail.gmail.com
  Discussion: http://postgr.es/m/620F3CE1-0255-4D66-9D87-0EADE866985A@amazon.com

  commit 1fb17b1903414676bd371068739549cd2966fe87
  Author:     Tom Lane 
  AuthorDate: Wed Dec 29 17:02:50 2021 -0500

  Fix issues in pgarch's new directory-scanning logic.

  The arch_filenames[] array elements were one byte too small, so that
  a maximum-length filename would get corrupted if another entry
  were made after it.  (Noted by Thomas Munro, fix by Nathan Bossart.)

  Move these arrays into a palloc'd struct, so that we aren't wasting
  a few kilobytes of static data in each non-archiver process.

  Add a binaryheap_reset() call to make it plain that we start the
  directory scan with an empty heap.  I don't think there's any live
  bug of that sort, but it seems fragile, and this is very cheap
  insurance.

  Cleanup for commit beb4e9ba1, so no back-patch needed.

  Discussion: https://postgr.es/m/CA+hUKGLHAjHuKuwtzsW7uMJF4BVPcQRL-UMZG_HM-g0y7yLkUg@mail.gmail.com

src/backend/access/transam/xlogarchive.c
src/backend/postmaster/pgarch.c
src/include/postmaster/pgarch.h

index 26b023e754b00309ec39285356c858d375e00433..99ee995c2141caff34c5d0f5b22183c84bba2cb9 100644 (file)
@@ -489,6 +489,20 @@ XLogArchiveNotify(const char *xlog)
        return;
    }
 
+   /*
+    * Timeline history files are given the highest archival priority to lower
+    * the chance that a promoted standby will choose a timeline that is
+    * already in use.  However, the archiver ordinarily tries to gather
+    * multiple files to archive from each scan of the archive_status
+    * directory, which means that newly created timeline history files could
+    * be left unarchived for a while.  To ensure that the archiver picks up
+    * timeline history files as soon as possible, we force the archiver to
+    * scan the archive_status directory the next time it looks for a file to
+    * archive.
+    */
+   if (IsTLHistoryFileName(xlog))
+       PgArchForceDirScan();
+
    /* Notify archiver that it's got something to do */
    if (IsUnderPostmaster)
        PgArchWakeup();
index 74a7d7c4d0a99b2d15ff688e90fb4962e259f372..6b99be85a96c166258bca6a6d94112f22a41dca0 100644 (file)
@@ -35,6 +35,7 @@
 
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
+#include "lib/binaryheap.h"
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -47,6 +48,7 @@
 #include "storage/proc.h"
 #include "storage/procsignal.h"
 #include "storage/shmem.h"
+#include "storage/spin.h"
 #include "utils/guc.h"
 #include "utils/ps_status.h"
 
  */
 #define NUM_ORPHAN_CLEANUP_RETRIES 3
 
+/*
+ * Maximum number of .ready files to gather per directory scan.
+ */
+#define NUM_FILES_PER_DIRECTORY_SCAN 64
+
 /* Shared memory area for archiver process */
 typedef struct PgArchData
 {
    int         pgprocno;       /* pgprocno of archiver process */
+
+   /*
+    * Forces a directory scan in pgarch_readyXlog().  Protected by arch_lck.
+    */
+   bool        force_dir_scan;
+
+   slock_t     arch_lck;
 } PgArchData;
 
 
@@ -86,6 +100,31 @@ typedef struct PgArchData
 static time_t last_sigterm_time = 0;
 static PgArchData *PgArch = NULL;
 
+/*
+ * Stuff for tracking multiple files to archive from each scan of
+ * archive_status.  Minimizing the number of directory scans when there are
+ * many files to archive can significantly improve archival rate.
+ *
+ * arch_heap is a max-heap that is used during the directory scan to track
+ * the highest-priority files to archive.  After the directory scan
+ * completes, the file names are stored in ascending order of priority in
+ * arch_files.  pgarch_readyXlog() returns files from arch_files until it
+ * is empty, at which point another directory scan must be performed.
+ *
+ * We only need this data in the archiver process, so make it a palloc'd
+ * struct rather than a bunch of static arrays.
+ */
+struct arch_files_state
+{
+   binaryheap *arch_heap;
+   int         arch_files_size;    /* number of live entries in arch_files[] */
+   char       *arch_files[NUM_FILES_PER_DIRECTORY_SCAN];
+   /* buffers underlying heap, and later arch_files[], entries: */
+   char        arch_filenames[NUM_FILES_PER_DIRECTORY_SCAN][MAX_XFN_CHARS + 1];
+};
+
+static struct arch_files_state *arch_files = NULL;
+
 /*
  * Flags set by interrupt handlers for later service in the main loop.
  */
@@ -103,6 +142,7 @@ static bool pgarch_readyXlog(char *xlog);
 static void pgarch_archiveDone(char *xlog);
 static void pgarch_die(int code, Datum arg);
 static void HandlePgArchInterrupts(void);
+static int ready_file_comparator(Datum a, Datum b, void *arg);
 
 /* Report shared memory space needed by PgArchShmemInit */
 Size
@@ -129,6 +169,7 @@ PgArchShmemInit(void)
        /* First time through, so initialize */
        MemSet(PgArch, 0, PgArchShmemSize());
        PgArch->pgprocno = INVALID_PGPROCNO;
+       SpinLockInit(&PgArch->arch_lck);
    }
 }
 
@@ -198,6 +239,14 @@ PgArchiverMain(void)
     */
    PgArch->pgprocno = MyProc->pgprocno;
 
+   /* Create workspace for pgarch_readyXlog() */
+   arch_files = palloc(sizeof(struct arch_files_state));
+   arch_files->arch_files_size = 0;
+
+   /* Initialize our max-heap for prioritizing files to archive. */
+   arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
+                                               ready_file_comparator, NULL);
+
    pgarch_MainLoop();
 
    proc_exit(0);
@@ -325,6 +374,9 @@ pgarch_ArchiverCopyLoop(void)
 {
    char        xlog[MAX_XFN_CHARS + 1];
 
+   /* force directory scan in the first call to pgarch_readyXlog() */
+   arch_files->arch_files_size = 0;
+
    /*
     * loop through all xlogs with archive_status of .ready and archive
     * them...mostly we expect this to be a single file, though it is possible
@@ -600,18 +652,57 @@ pgarch_archiveXlog(char *xlog)
 static bool
 pgarch_readyXlog(char *xlog)
 {
-   /*
-    * open xlog status directory and read through list of xlogs that have the
-    * .ready suffix, looking for earliest file. It is possible to optimise
-    * this code, though only a single file is expected on the vast majority
-    * of calls, so....
-    */
    char        XLogArchiveStatusDir[MAXPGPATH];
    DIR        *rldir;
    struct dirent *rlde;
-   bool        found = false;
-   bool        historyFound = false;
+   bool        force_dir_scan;
+
+   /*
+    * If a directory scan was requested, clear the stored file names and
+    * proceed.
+    */
+   SpinLockAcquire(&PgArch->arch_lck);
+   force_dir_scan = PgArch->force_dir_scan;
+   PgArch->force_dir_scan = false;
+   SpinLockRelease(&PgArch->arch_lck);
+
+   if (force_dir_scan)
+       arch_files->arch_files_size = 0;
+
+   /*
+    * If we still have stored file names from the previous directory scan,
+    * try to return one of those.  We check to make sure the status file is
+    * still present, as the archive_command for a previous file may have
+    * already marked it done.
+    */
+   while (arch_files->arch_files_size > 0)
+   {
+       struct stat st;
+       char        status_file[MAXPGPATH];
+       char       *arch_file;
+
+       arch_files->arch_files_size--;
+       arch_file = arch_files->arch_files[arch_files->arch_files_size];
+       StatusFilePath(status_file, arch_file, ".ready");
+
+       if (stat(status_file, &st) == 0)
+       {
+           strcpy(xlog, arch_file);
+           return true;
+       }
+       else if (errno != ENOENT)
+           ereport(ERROR,
+                   (errcode_for_file_access(),
+                    errmsg("could not stat file \"%s\": %m", status_file)));
+   }
 
+   /* arch_heap is probably empty, but let's make sure */
+   binaryheap_reset(arch_files->arch_heap);
+
+   /*
+    * Open the archive status directory and read through the list of files
+    * with the .ready suffix, looking for the earliest files.
+    */
    snprintf(XLogArchiveStatusDir, MAXPGPATH, XLOGDIR "/archive_status");
    rldir = AllocateDir(XLogArchiveStatusDir);
 
@@ -619,7 +710,7 @@ pgarch_readyXlog(char *xlog)
    {
        int         basenamelen = (int) strlen(rlde->d_name) - 6;
        char        basename[MAX_XFN_CHARS + 1];
-       bool        ishistory;
+       char       *arch_file;
 
        /* Ignore entries with unexpected number of characters */
        if (basenamelen < MIN_XFN_CHARS ||
@@ -638,32 +729,97 @@ pgarch_readyXlog(char *xlog)
        memcpy(basename, rlde->d_name, basenamelen);
        basename[basenamelen] = '\0';
 
-       /* Is this a history file? */
-       ishistory = IsTLHistoryFileName(basename);
-
        /*
-        * Consume the file to archive.  History files have the highest
-        * priority.  If this is the first file or the first history file
-        * ever, copy it.  In the presence of a history file already chosen as
-        * target, ignore all other files except history files which have been
-        * generated for an older timeline than what is already chosen as
-        * target to archive.
+        * Store the file in our max-heap if it has a high enough priority.
         */
-       if (!found || (ishistory && !historyFound))
+       if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
        {
-           strcpy(xlog, basename);
-           found = true;
-           historyFound = ishistory;
+           /* If the heap isn't full yet, quickly add it. */
+           arch_file = arch_files->arch_filenames[arch_files->arch_heap->bh_size];
+           strcpy(arch_file, basename);
+           binaryheap_add_unordered(arch_files->arch_heap, CStringGetDatum(arch_file));
+
+           /* If we just filled the heap, make it a valid one. */
+           if (arch_files->arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN)
+               binaryheap_build(arch_files->arch_heap);
        }
-       else if (ishistory || !historyFound)
+       else if (ready_file_comparator(binaryheap_first(arch_files->arch_heap),
+                                      CStringGetDatum(basename), NULL) > 0)
        {
-           if (strcmp(basename, xlog) < 0)
-               strcpy(xlog, basename);
+           /*
+            * Remove the lowest priority file and add the current one to the
+            * heap.
+            */
+           arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
+           strcpy(arch_file, basename);
+           binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file));
        }
    }
    FreeDir(rldir);
 
-   return found;
+   /* If no files were found, simply return. */
+   if (arch_files->arch_heap->bh_size == 0)
+       return false;
+
+   /*
+    * If we didn't fill the heap, we didn't make it a valid one.  Do that
+    * now.
+    */
+   if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
+       binaryheap_build(arch_files->arch_heap);
+
+   /*
+    * Fill arch_files array with the files to archive in ascending order of
+    * priority.
+    */
+   arch_files->arch_files_size = arch_files->arch_heap->bh_size;
+   for (int i = 0; i < arch_files->arch_files_size; i++)
+       arch_files->arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
+
+   /* Return the highest priority file. */
+   arch_files->arch_files_size--;
+   strcpy(xlog, arch_files->arch_files[arch_files->arch_files_size]);
+
+   return true;
+}
+
+/*
+ * ready_file_comparator
+ *
+ * Compares the archival priority of the given files to archive.  If "a"
+ * has a higher priority than "b", a negative value will be returned.  If
+ * "b" has a higher priority than "a", a positive value will be returned.
+ * If "a" and "b" have equivalent values, 0 will be returned.
+ */
+static int
+ready_file_comparator(Datum a, Datum b, void *arg)
+{
+   char       *a_str = DatumGetCString(a);
+   char       *b_str = DatumGetCString(b);
+   bool        a_history = IsTLHistoryFileName(a_str);
+   bool        b_history = IsTLHistoryFileName(b_str);
+
+   /* Timeline history files always have the highest priority. */
+   if (a_history != b_history)
+       return a_history ? -1 : 1;
+
+   /* Priority is given to older files. */
+   return strcmp(a_str, b_str);
+}
+
+/*
+ * PgArchForceDirScan
+ *
+ * When called, the next call to pgarch_readyXlog() will perform a
+ * directory scan.  This is useful for ensuring that important files such
+ * as timeline history files are archived as quickly as possible.
+ */
+void
+PgArchForceDirScan(void)
+{
+   SpinLockAcquire(&PgArch->arch_lck);
+   PgArch->force_dir_scan = true;
+   SpinLockRelease(&PgArch->arch_lck);
 }
 
 /*
index 1e47a143e13378b753fedeb4ad2267ef34acd2cc..732615be57068d8a8865127f8cc1b9261a3845fa 100644 (file)
@@ -31,5 +31,6 @@ extern void PgArchShmemInit(void);
 extern bool PgArchCanRestart(void);
 extern void PgArchiverMain(void) pg_attribute_noreturn();
 extern void PgArchWakeup(void);
+extern void PgArchForceDirScan(void);
 
 #endif                         /* _PGARCH_H */