improve-beb4e9b.patch

application/octet-stream

Filename: improve-beb4e9b.patch
Type: application/octet-stream
Part: 0
Message: Re: Strange path from pgarch_readyXlog()

Patch

Format: unified
File+
src/backend/postmaster/pgarch.c 47 36
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 434939be9b..87022b4755 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -106,16 +106,16 @@ static PgArchData *PgArch = NULL;
  * 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.
+ * During the scan of the archive_status directory, arch_heap is a max-heap
+ * (lowest priority file in root node) that stores the highest-priority files to
+ * archive.  After the directory scan, arch_heap is rebuilt as a min-heap so
+ * that the highest-priority file is stored in the root node.
+ * pgarch_readyXlog() returns files from arch_heap until it is empty, at which
+ * point another directory scan must be performed.
  */
 static binaryheap *arch_heap = NULL;
-static char arch_filenames[NUM_FILES_PER_DIRECTORY_SCAN][MAX_XFN_CHARS];
-static char *arch_files[NUM_FILES_PER_DIRECTORY_SCAN];
-static int arch_files_size = 0;
+static char **arch_filenames = NULL;
+static bool arch_heap_min_heap = false;
 
 /*
  * Flags set by interrupt handlers for later service in the main loop.
@@ -231,9 +231,15 @@ PgArchiverMain(void)
 	 */
 	PgArch->pgprocno = MyProc->pgprocno;
 
-	/* Initialize our max-heap for prioritizing files to archive. */
+	/* Initialize our heap for prioritizing files to archive. */
 	arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
-									ready_file_comparator, NULL);
+									ready_file_comparator,
+									&arch_heap_min_heap);
+
+	/* Initialize our array for storing archive filenames. */
+	arch_filenames = (char **) palloc(NUM_FILES_PER_DIRECTORY_SCAN * sizeof(char *));
+	for (int i = 0; i < NUM_FILES_PER_DIRECTORY_SCAN; ++i)
+		arch_filenames[i] = (char *) palloc(MAX_XFN_CHARS + 1);
 
 	pgarch_MainLoop();
 
@@ -363,7 +369,7 @@ pgarch_ArchiverCopyLoop(void)
 	char		xlog[MAX_XFN_CHARS + 1];
 
 	/* force directory scan in the first call to pgarch_readyXlog() */
-	arch_files_size = 0;
+	binaryheap_reset(arch_heap);
 
 	/*
 	 * loop through all xlogs with archive_status of .ready and archive
@@ -658,7 +664,7 @@ pgarch_readyXlog(char *xlog)
 	SpinLockRelease(&PgArch->arch_lck);
 
 	if (force_dir_scan)
-		arch_files_size = 0;
+		binaryheap_reset(arch_heap);
 
 	/*
 	 * If we still have stored file names from the previous directory scan,
@@ -666,14 +672,13 @@ pgarch_readyXlog(char *xlog)
 	 * is still present, as the archive_command for a previous file may
 	 * have already marked it done.
 	 */
-	while (arch_files_size > 0)
+	while (!binaryheap_empty(arch_heap))
 	{
 		struct stat	st;
 		char		status_file[MAXPGPATH];
 		char	   *arch_file;
 
-		arch_files_size--;
-		arch_file = arch_files[arch_files_size];
+		arch_file = DatumGetCString(binaryheap_remove_first(arch_heap));
 		StatusFilePath(status_file, arch_file, ".ready");
 
 		if (stat(status_file, &st) == 0)
@@ -687,6 +692,12 @@ pgarch_readyXlog(char *xlog)
 					 errmsg("could not stat file \"%s\": %m", status_file)));
 	}
 
+	/*
+	 * Assemble a max-heap (lowest priority file in the root node) during the
+	 * directory scan.
+	 */
+	arch_heap_min_heap = false;
+
 	/*
 	 * Open the archive status directory and read through the list of files
 	 * with the .ready suffix, looking for the earliest files.
@@ -746,27 +757,18 @@ pgarch_readyXlog(char *xlog)
 	FreeDir(rldir);
 
 	/* If no files were found, simply return. */
-	if (arch_heap->bh_size == 0)
+	if (binaryheap_empty(arch_heap))
 		return false;
 
 	/*
-	 * If we didn't fill the heap, we didn't make it a valid one.  Do that
-	 * now.
+	 * Switch the heap to a min-heap so that the highest priority file is stored
+	 * in the root node.
 	 */
-	if (arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
-		binaryheap_build(arch_heap);
-
-	/*
-	 * Fill arch_files array with the files to archive in ascending order
-	 * of priority.
-	 */
-	arch_files_size = arch_heap->bh_size;
-	for (int i = 0; i < arch_files_size; i++)
-		arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_heap));
+	arch_heap_min_heap = true;
+	binaryheap_build(arch_heap);
 
 	/* Return the highest priority file. */
-	arch_files_size--;
-	strcpy(xlog, arch_files[arch_files_size]);
+	strcpy(xlog, DatumGetCString(binaryheap_remove_first(arch_heap)));
 
 	return true;
 }
@@ -774,16 +776,25 @@ pgarch_readyXlog(char *xlog)
 /*
  * 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.
+ * Compares the archival priority of the given files to archive.  When the extra
+ * argument is true, the comparator returns values for constructing a min-heap
+ * (highest priority file in the root node).  In this case, if "a" has a higher
+ * priority than "b", a positive value will be returned.  If "b" has a higher
+ * priority than "a", a negative value will be returned.  If "a" and "b" have
+ * equivalent values, 0 will be returned.
+ *
+ * When the extra argument is false, the comparator returns values for
+ * constructing a max-heap (lowest priority file in the root node).  In this
+ * case, 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 min_heap = *((bool *) arg);
+	char *a_str = DatumGetCString(min_heap ? b : a);
+	char *b_str = DatumGetCString(min_heap ? a : b);
 	bool a_history = IsTLHistoryFileName(a_str);
 	bool b_history = IsTLHistoryFileName(b_str);