database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-29 10:41:53 +03:00
Code Activity

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

Browse Source 
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
This commit is contained in:
Tom Lane
2021-12-29 17:02:50 -05:00
parent 113fa3945f
commit 1fb17b1903
1 changed files with 43 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
src/backend/postmaster/pgarch.c
View File

@ -111,11 +111,20 @@ static PgArchData *PgArch = NULL;
* completes, the file names are stored in ascending order of priority in * completes, the file names are stored in ascending order of priority in
* arch_files. pgarch_readyXlog() returns files from arch_files until it * arch_files. pgarch_readyXlog() returns files from arch_files until it
* is empty, at which point another directory scan must be performed. * 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.
*/ */
static binaryheap *arch_heap = NULL; struct arch_files_state
static char arch_filenames[NUM_FILES_PER_DIRECTORY_SCAN][MAX_XFN_CHARS]; {
static char *arch_files[NUM_FILES_PER_DIRECTORY_SCAN]; binaryheap *arch_heap;
static int arch_files_size = 0; 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. * Flags set by interrupt handlers for later service in the main loop.
@ -231,9 +240,13 @@ PgArchiverMain(void)
*/ */
PgArch->pgprocno = MyProc->pgprocno; 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. */ /* Initialize our max-heap for prioritizing files to archive. */
arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
ready_file_comparator, NULL); ready_file_comparator, NULL);
pgarch_MainLoop(); pgarch_MainLoop();
@ -363,7 +376,7 @@ pgarch_ArchiverCopyLoop(void)
char xlog[MAX_XFN_CHARS + 1]; char xlog[MAX_XFN_CHARS + 1];
/* force directory scan in the first call to pgarch_readyXlog() */ /* force directory scan in the first call to pgarch_readyXlog() */
arch_files_size = 0; arch_files->arch_files_size = 0;
/* /*
* loop through all xlogs with archive_status of .ready and archive * loop through all xlogs with archive_status of .ready and archive
@ -658,7 +671,7 @@ pgarch_readyXlog(char *xlog)
SpinLockRelease(&PgArch->arch_lck); SpinLockRelease(&PgArch->arch_lck);
if (force_dir_scan) if (force_dir_scan)
arch_files_size = 0; arch_files->arch_files_size = 0;
/* /*
* If we still have stored file names from the previous directory scan, * If we still have stored file names from the previous directory scan,
@ -666,14 +679,14 @@ pgarch_readyXlog(char *xlog)
* is still present, as the archive_command for a previous file may * is still present, as the archive_command for a previous file may
* have already marked it done. * have already marked it done.
*/ */
while (arch_files_size > 0) while (arch_files->arch_files_size > 0)
{ {
struct stat st; struct stat st;
char status_file[MAXPGPATH]; char status_file[MAXPGPATH];
char *arch_file; char *arch_file;
arch_files_size--; arch_files->arch_files_size--;
arch_file = arch_files[arch_files_size]; arch_file = arch_files->arch_files[arch_files->arch_files_size];
StatusFilePath(status_file, arch_file, ".ready"); StatusFilePath(status_file, arch_file, ".ready");
if (stat(status_file, &st) == 0) if (stat(status_file, &st) == 0)
@ -687,6 +700,9 @@ pgarch_readyXlog(char *xlog)
errmsg("could not stat file \"%s\": %m", status_file))); 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 * Open the archive status directory and read through the list of files
* with the .ready suffix, looking for the earliest files. * with the .ready suffix, looking for the earliest files.
@ -720,53 +736,53 @@ pgarch_readyXlog(char *xlog)
/* /*
* Store the file in our max-heap if it has a high enough priority. * Store the file in our max-heap if it has a high enough priority.
*/ */
if (arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN) if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
{ {
/* If the heap isn't full yet, quickly add it. */ /* If the heap isn't full yet, quickly add it. */
arch_file = arch_filenames[arch_heap->bh_size]; arch_file = arch_files->arch_filenames[arch_files->arch_heap->bh_size];
strcpy(arch_file, basename); strcpy(arch_file, basename);
binaryheap_add_unordered(arch_heap, CStringGetDatum(arch_file)); binaryheap_add_unordered(arch_files->arch_heap, CStringGetDatum(arch_file));
/* If we just filled the heap, make it a valid one. */ /* If we just filled the heap, make it a valid one. */
if (arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN) if (arch_files->arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN)
binaryheap_build(arch_heap); binaryheap_build(arch_files->arch_heap);
} }
else if (ready_file_comparator(binaryheap_first(arch_heap), else if (ready_file_comparator(binaryheap_first(arch_files->arch_heap),
CStringGetDatum(basename), NULL) > 0) CStringGetDatum(basename), NULL) > 0)
{ {
/* /*
* Remove the lowest priority file and add the current one to * Remove the lowest priority file and add the current one to
* the heap. * the heap.
*/ */
arch_file = DatumGetCString(binaryheap_remove_first(arch_heap)); arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
strcpy(arch_file, basename); strcpy(arch_file, basename);
binaryheap_add(arch_heap, CStringGetDatum(arch_file)); binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file));
} }
} }
FreeDir(rldir); FreeDir(rldir);
/* If no files were found, simply return. */ /* If no files were found, simply return. */
if (arch_heap->bh_size == 0) if (arch_files->arch_heap->bh_size == 0)
return false; return false;
/* /*
* If we didn't fill the heap, we didn't make it a valid one. Do that * If we didn't fill the heap, we didn't make it a valid one. Do that
* now. * now.
*/ */
if (arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN) if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
binaryheap_build(arch_heap); binaryheap_build(arch_files->arch_heap);
/* /*
* Fill arch_files array with the files to archive in ascending order * Fill arch_files array with the files to archive in ascending order
* of priority. * of priority.
*/ */
arch_files_size = arch_heap->bh_size; arch_files->arch_files_size = arch_files->arch_heap->bh_size;
for (int i = 0; i < arch_files_size; i++) for (int i = 0; i < arch_files->arch_files_size; i++)
arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_heap)); arch_files->arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
/* Return the highest priority file. */ /* Return the highest priority file. */
arch_files_size--; arch_files->arch_files_size--;
strcpy(xlog, arch_files[arch_files_size]); strcpy(xlog, arch_files->arch_files[arch_files->arch_files_size]);
return true; return true;
} }