From 3c2c369c495760d8f96d3cdeab623c4f0372792a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Thu, 15 Sep 2022 13:39:53 -0500 Subject: [PATCH 2/2] f!justin ci-os-only: freebsd --- src/bin/pg_waldump/meson.build | 1 + src/bin/pg_waldump/pg_waldump.c | 172 +++++++++++----------- src/bin/pg_waldump/t/002_save_fullpage.pl | 2 +- 3 files changed, 86 insertions(+), 89 deletions(-) diff --git a/src/bin/pg_waldump/meson.build b/src/bin/pg_waldump/meson.build index 9605976870d..34e37bffc36 100644 --- a/src/bin/pg_waldump/meson.build +++ b/src/bin/pg_waldump/meson.build @@ -29,6 +29,7 @@ tests += { 'tap': { 'tests': [ 't/001_basic.pl', + 't/002_save_fullpage.pl', ], }, } diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 2b6f772dc71..e35d15132f4 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -76,7 +76,6 @@ typedef struct XLogDumpConfig bool filter_by_fpw; /* save options */ - bool save_fpw; bool fixup_fpw; char *save_fpw_path; } XLogDumpConfig; @@ -458,96 +457,94 @@ XLogRecordSaveFPWs(XLogReaderState *record, const char *savepath, bool fixup) for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { + char page[BLCKSZ] = {0}; + char filename[MAXPGPATH]; + char forkname[FORKNAMECHARS + 2]; /* _ + \0 */ + FILE *OPF; + BlockNumber blk; + RelFileLocator rnode; + ForkNumber fork; + if (!XLogRecHasBlockRef(record, block_id)) continue; - if (XLogRecHasBlockImage(record, block_id)) - { - char page[BLCKSZ]; - - memset(page, 0, BLCKSZ); - - if (RestoreBlockImage(record, block_id, page)) - { - /* we have our extracted FPI, let's save it now */ - char filename[MAXPGPATH]; - char forkname[FORKNAMECHARS + 2]; /* _ + \0 */ - FILE *OPF; - BlockNumber blk; - RelFileLocator rnode; - ForkNumber fork; - - XLogRecGetBlockTagExtended(record, block_id, - &rnode, &fork, &blk, NULL); - - /* - * The raw page as stored in the WAL record includes the LSN - * of the block as it appeared when it was originally written, - * however this differs than the effects of replaying this - * same FPI in recovery, as recovery calls RestoreBlockImage() - * and then sets the LSN as part of one action. What this - * means is that a page as recovered from WAL and the version - * of the page saved here will differ by the LSN and the - * checksum (if enabled). - * - * There are potentially use-cases for both versions (with and - * without mentioned fixups), so allow this to be - * user-selected, unless the restored page was empty, in which - * case we leave it alone. - */ - - if (fixup && !PageIsNew(page)) - { - PageSetLSN(page, record->ReadRecPtr); + if (!XLogRecHasBlockImage(record, block_id)) + continue; - /* - * If checksum field is non-zero then we have checksums - * enabled, so recalculate the checksum with new LSN - * (whether this is considered a hack or heuristics is an - * exercise for the reader). - * - * We make this choice to allow pages saved by this - * function to work as expected with the checksum-related - * functions in pageinspect without having to worry about - * zero_damaged_pages or other considerations. - * - * FPIs in WAL do not have the checksum field updated in - * the page image; in a checksums-enabled cluster, this - * task is handled by FlushBuffer() when a dirty buffer is - * written out to disk. Since we are running outside of - * Postmaster that won't work in this case, so we handle - * ourselves. - */ + if (!RestoreBlockImage(record, block_id, page)) + continue; - if (((PageHeader) page)->pd_checksum) - ((PageHeader) page)->pd_checksum = pg_checksum_page((char *) page, blk); - } + /* we have our extracted FPI, let's save it now */ + + XLogRecGetBlockTagExtended(record, block_id, + &rnode, &fork, &blk, NULL); + + /* + * The raw page as stored in the WAL record includes the LSN + * of the block as it appeared when it was originally written, + * however this differs than the effects of replaying this + * same FPI in recovery, as recovery calls RestoreBlockImage() + * and then sets the LSN as part of one action. What this + * means is that a page as recovered from WAL and the version + * of the page saved here will differ by the LSN and the + * checksum (if enabled). + * + * There are potentially use-cases for both versions (with and + * without mentioned fixups), so allow this to be + * user-selected, unless the restored page was empty, in which + * case we leave it alone. + */ + + if (fixup && !PageIsNew(page)) + { + PageSetLSN(page, record->ReadRecPtr); + + /* + * If checksum field is non-zero then we have checksums + * enabled, so recalculate the checksum with new LSN + * (whether this is considered a hack or heuristics is an + * exercise for the reader). + * + * We make this choice to allow pages saved by this + * function to work as expected with the checksum-related + * functions in pageinspect without having to worry about + * zero_damaged_pages or other considerations. + * + * FPIs in WAL do not have the checksum field updated in + * the page image; in a checksums-enabled cluster, this + * task is handled by FlushBuffer() when a dirty buffer is + * written out to disk. Since we are running outside of + * Postmaster that won't work in this case, so we handle + * ourselves. + */ + + if (((PageHeader) page)->pd_checksum) + ((PageHeader) page)->pd_checksum = pg_checksum_page((char *) page, blk); + } - if (fork >= 0 && fork <= MAX_FORKNUM) - { - if (fork) - sprintf(forkname, "_%s", forkNames[fork]); - else - forkname[0] = 0; - } - else - pg_fatal("found invalid fork number: %u", fork); + if (fork >= 0 && fork <= MAX_FORKNUM) + { + if (fork) + sprintf(forkname, "_%s", forkNames[fork]); + else + forkname[0] = 0; + } + else + pg_fatal("found invalid fork number: %u", fork); - snprintf(filename, MAXPGPATH, "%s/%08X-%08X.%u.%u.%u.%u%s", savepath, - LSN_FORMAT_ARGS(record->ReadRecPtr), - rnode.spcOid, rnode.dbOid, rnode.relNumber, blk, forkname); + snprintf(filename, MAXPGPATH, "%s/%08X-%08X.%u.%u.%u.%u%s", savepath, + LSN_FORMAT_ARGS(record->ReadRecPtr), + rnode.spcOid, rnode.dbOid, rnode.relNumber, blk, forkname); - OPF = fopen(filename, PG_BINARY_W); - if (!OPF) - pg_fatal("couldn't open file for output: %s", filename); + OPF = fopen(filename, PG_BINARY_W); + if (!OPF) + pg_fatal("couldn't open file for output: %s", filename); - if (pg_pwrite(fileno(OPF), page, BLCKSZ, 0) != BLCKSZ) - pg_fatal("couldn't write out complete fullpage image to file: %s", filename); + if (pg_pwrite(fileno(OPF), page, BLCKSZ, 0) != BLCKSZ) + pg_fatal("couldn't write out complete fullpage image to file: %s", filename); - fsync(fileno(OPF)); - fclose(OPF); - } - } + fsync(fileno(OPF)); + fclose(OPF); } } @@ -791,9 +788,9 @@ usage(void) " (default: 1 or the value used in STARTSEG)\n")); printf(_(" -V, --version output version information, then exit\n")); printf(_(" -w, --fullpage only show records with a full page write\n")); - printf(_(" -W, --save-fpi=path save found full page images to given path\n" - " as LSN.T.D.R.B_F\n")); - printf(_(" -X, --fixup-fpi=path like --save-fpi but apply LSN fixups to \n")); + printf(_(" -W, --save-fpi=path save full page images to given path as\n" + " LSN.T.D.R.B_F\n")); + printf(_(" -X, --fixup-fpi=path like --save-fpi but apply LSN fixups to saved page\n")); printf(_(" -x, --xid=XID only show records with transaction ID XID\n")); printf(_(" -z, --stats[=record] show statistics instead of records\n" " (optionally, show per-record statistics)\n")); @@ -889,7 +886,6 @@ main(int argc, char **argv) config.filter_by_fpw = false; config.stats = false; config.stats_per_record = false; - config.save_fpw = false; config.fixup_fpw = false; config.save_fpw_path = NULL; @@ -1040,7 +1036,6 @@ main(int argc, char **argv) break; case 'W': case 'X': - config.save_fpw = true; config.fixup_fpw = (option == 'X'); config.save_fpw_path = pg_strdup(optarg); break; @@ -1108,7 +1103,8 @@ main(int argc, char **argv) goto bad_argument; } - if (!dir_status && mkdir(config.save_fpw_path, 0700) < 0) + /* Create the dir if it doesn't exist */ + if (dir_status == 0 && mkdir(config.save_fpw_path, 0700) < 0) { pg_log_error("could not create output directory \"%s\": %m", config.save_fpw_path); @@ -1294,7 +1290,7 @@ main(int argc, char **argv) XLogRecStoreStats(&stats, xlogreader_state); stats.endptr = xlogreader_state->EndRecPtr; } - else if (config.save_fpw) + else if (config.save_fpw_path) { if (XLogRecordHasFPW(xlogreader_state)) XLogRecordSaveFPWs(xlogreader_state, config.save_fpw_path, config.fixup_fpw); diff --git a/src/bin/pg_waldump/t/002_save_fullpage.pl b/src/bin/pg_waldump/t/002_save_fullpage.pl index f605b4f6adb..6df670c39ba 100644 --- a/src/bin/pg_waldump/t/002_save_fullpage.pl +++ b/src/bin/pg_waldump/t/002_save_fullpage.pl @@ -60,7 +60,7 @@ EOF # get the relation node, etc for the new table my $relation = $primary->safe_psql('db1', - q{SELECT CASE WHEN reltablespace = 0 THEN dattablespace ELSE reltablespace END || '/' || pg_database.oid || '/' || relfilenode FROM pg_class, pg_database WHERE relname = 'test_table' AND datname = current_database()} + q{SELECT format('%s/%s/%s', CASE WHEN reltablespace = 0 THEN dattablespace ELSE reltablespace END, pg_database.oid, relfilenode) FROM pg_class, pg_database WHERE relname = 'test_table' AND datname = current_database()} ); diag $relation; -- 2.25.1