This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit b73d71d6f6636c7f261cd2ce0906a76810dc2b3e Author: Tom Lane <[email protected]> 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-g0y7yl...@mail.gmail.com --- src/backend/postmaster/pgarch.c | 70 +++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 6d4336c8cc..ee6507130a 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -116,11 +116,20 @@ static PgArchData *PgArch = NULL; * 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. */ -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; +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. @@ -236,9 +245,13 @@ 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_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, - ready_file_comparator, NULL); + arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, + ready_file_comparator, NULL); pgarch_MainLoop(); @@ -368,7 +381,7 @@ pgarch_ArchiverCopyLoop(void) char xlog[MAX_XFN_CHARS + 1]; /* 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 @@ -670,7 +683,7 @@ pgarch_readyXlog(char *xlog) SpinLockRelease(&PgArch->arch_lck); 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, @@ -678,14 +691,14 @@ 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 (arch_files->arch_files_size > 0) { struct stat st; char status_file[MAXPGPATH]; char *arch_file; - arch_files_size--; - arch_file = arch_files[arch_files_size]; + 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) @@ -699,6 +712,9 @@ pgarch_readyXlog(char *xlog) 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. @@ -732,53 +748,53 @@ pgarch_readyXlog(char *xlog) /* * 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. */ - arch_file = arch_filenames[arch_heap->bh_size]; + arch_file = arch_files->arch_filenames[arch_files->arch_heap->bh_size]; 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 (arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN) - binaryheap_build(arch_heap); + if (arch_files->arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN) + 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) { /* * Remove the lowest priority file and add the current one to * the heap. */ - arch_file = DatumGetCString(binaryheap_remove_first(arch_heap)); + arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap)); strcpy(arch_file, basename); - binaryheap_add(arch_heap, CStringGetDatum(arch_file)); + binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file)); } } FreeDir(rldir); /* If no files were found, simply return. */ - if (arch_heap->bh_size == 0) + 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_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN) - binaryheap_build(arch_heap); + 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_size = arch_heap->bh_size; - for (int i = 0; i < arch_files_size; i++) - arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_heap)); + 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_size--; - strcpy(xlog, arch_files[arch_files_size]); + arch_files->arch_files_size--; + strcpy(xlog, arch_files->arch_files[arch_files->arch_files_size]); return true; } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
