From 58f4c263acab28a38abf67fb3194f965d723bbc7 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Thu, 3 Sep 2026 10:42:48 -0700 Subject: [PATCH v2 3/3] WIP: pg_checksums: Ignore stray files in directories Commit 025584a16 taught pg_basebackup to ignore files that don't belong to the cluster during checksum operations, but pg_checksums itself was left as-is. Align them now and get rid of some special-case code in pg_checksums. For PG20 only. The inconsistency in back branches is odd, but it doesn't seem to rise to bug level, and it'd be good to put this new behavior through a beta. TODO: make absolutely sure we can skip temp relations here Reported-by: Edwin Polkerman Discussion: https://postgr.es/m/CAOYmi%2B%3DE0zz%2B_MgQXAoOyn5C0vW53740izzRiTv6excqtCJfRg%40mail.gmail.com --- src/bin/pg_checksums/pg_checksums.c | 91 +++++---------------------- src/bin/pg_checksums/t/002_actions.pl | 10 ++- 2 files changed, 24 insertions(+), 77 deletions(-) diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c index 5bb7a7bd200..ad25c7a9e08 100644 --- a/src/bin/pg_checksums/pg_checksums.c +++ b/src/bin/pg_checksums/pg_checksums.c @@ -40,7 +40,7 @@ static int64 blocks_written = 0; static int64 badblocks = 0; static ControlFileData *ControlFile; -static char *only_filenode = NULL; +static int only_filenode = 0; static bool do_sync = true; static bool verbose = false; static bool showprogress = false; @@ -88,34 +88,6 @@ usage(void) printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL); } -/* - * Definition of one element part of an exclusion list, used for files - * to exclude from checksum validation. "name" is the name of the file - * or path to check for exclusion. If "match_prefix" is true, any items - * matching the name as prefix are excluded. - */ -struct exclude_list_item -{ - const char *name; - bool match_prefix; -}; - -/* - * List of files excluded from checksum validation. - * - * Note: this list should be kept in sync with what basebackup.c includes. - */ -static const struct exclude_list_item skip[] = { - {"pg_control", false}, - {"pg_filenode.map", false}, - {"pg_internal.init", true}, - {"PG_VERSION", false}, -#ifdef EXEC_BACKEND - {"config_exec_params", true}, -#endif - {NULL, false} -}; - /* * Report current progress status. Parts borrowed from * src/bin/pg_basebackup/pg_basebackup.c. @@ -154,26 +126,8 @@ progress_report(bool finished) fputc((!finished && isatty(fileno(stderr))) ? '\r' : '\n', stderr); } -static bool -skipfile(const char *fn) -{ - int excludeIdx; - - for (excludeIdx = 0; skip[excludeIdx].name != NULL; excludeIdx++) - { - int cmplen = strlen(skip[excludeIdx].name); - - if (!skip[excludeIdx].match_prefix) - cmplen++; - if (strncmp(skip[excludeIdx].name, fn, cmplen) == 0) - return true; - } - - return false; -} - static void -scan_file(const char *fn, int segmentno) +scan_file(const char *fn, unsigned segmentno) { PGIOAlignedBlock buf; PageHeader header = (PageHeader) buf.data; @@ -339,36 +293,22 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly) pg_fatal("could not stat file \"%s\": %m", fn); if (S_ISREG(st.st_mode)) { - char fnonly[MAXPGPATH]; - char *forkpath, - *segmentpath; - int segmentno = 0; - - if (skipfile(de->d_name)) - continue; + RelFileNumber relfileno; + ForkNumber fork; + unsigned segmentno; /* - * Cut off at the segment boundary (".") to get the segment number - * in order to mix it into the checksum. Then also cut off at the - * fork boundary, to get the filenode the file belongs to for - * filtering. + * If this is a relfile, get the segment number in order to mix it + * into the checksum, and filter on the relfile number if + * requested. Skip any temporary relations and anything that's not + * a relation at all. */ - strlcpy(fnonly, de->d_name, sizeof(fnonly)); - segmentpath = strchr(fnonly, '.'); - if (segmentpath != NULL) - { - *segmentpath++ = '\0'; - segmentno = atoi(segmentpath); - if (segmentno == 0) - pg_fatal("invalid segment number %d in file name \"%s\"", - segmentno, fn); - } - - forkpath = strchr(fnonly, '_'); - if (forkpath != NULL) - *forkpath++ = '\0'; + if (!parse_filename_for_nontemp_relation(de->d_name, + &relfileno, &fork, + &segmentno)) + continue; - if (only_filenode && strcmp(only_filenode, fnonly) != 0) + if (only_filenode && relfileno != only_filenode) /* filenode not to be included */ continue; @@ -490,9 +430,8 @@ main(int argc, char *argv[]) case 'f': if (!option_parse_int(optarg, "-f/--filenode", 1, INT_MAX, - NULL)) + &only_filenode)) exit(1); - only_filenode = pstrdup(optarg); break; case 'N': do_sync = false; diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl index 94a11a534e9..d3fea8ef3f3 100644 --- a/src/bin/pg_checksums/t/002_actions.pl +++ b/src/bin/pg_checksums/t/002_actions.pl @@ -110,13 +110,14 @@ append_to_file "$pgdata/global/99999_init.123", ""; append_to_file "$pgdata/global/99999_fsm.123", ""; append_to_file "$pgdata/global/99999_vm.123", ""; -# These are temporary files and folders with dummy contents, which +# These are temporary files/folders/relations with dummy contents, which # should be ignored by the scan. append_to_file "$pgdata/global/pgsql_tmp_123", "foo"; mkdir "$pgdata/global/pgsql_tmp"; append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo"; append_to_file "$pgdata/global/pg_internal.init", "foo"; append_to_file "$pgdata/global/pg_internal.init.123", "foo"; +append_to_file "$pgdata/global/t1_99999", "foo"; # These are non-postgres macOS files, which should be ignored by the scan. # Only perform this test on non-macOS systems though as creating incorrect @@ -124,6 +125,13 @@ append_to_file "$pgdata/global/pg_internal.init.123", "foo"; append_to_file "$pgdata/global/.DS_Store", "foo" unless ($Config{osname} eq 'darwin'); +# These don't look like our relfile segments, so they should be ignored too. +append_to_file "$pgdata/global/foo", "foo"; +append_to_file "$pgdata/global/bar.baz", "foo"; +append_to_file "$pgdata/global/001", "foo"; +append_to_file "$pgdata/global/1.001", "foo"; +append_to_file "$pgdata/global/1_foobar", "foo"; + # Enable checksums. command_ok([ 'pg_checksums', '--enable', '--no-sync', '--pgdata' => $pgdata ], "checksums successfully enabled in cluster"); -- 2.34.1