On Thu, Mar 14, 2019 at 04:26:20PM +0100, Michael Banck wrote: > Am Donnerstag, den 14.03.2019, 15:26 +0100 schrieb Magnus Hagander: >> One big-hammer method could be similar to what pg_upgrade does -- >> temporarily rename away the controlfile so postgresql can't start, and >> when done, put it back. > > That sounds like a good solution to me. I've made PoC patch for that, > see attached.
Indeed. I did not know this trick from pg_upgrade. We could just use the same. > The only question is whether pg_checksums should try to move pg_control > back (i) on failure (ii) when interrupted? Yes, we should have a callback on SIGINT and SIGTERM here which just moves back in place the control file if the temporary one exists. I have been able to grab some time to incorporate the feedback gathered on this thread, and please find attached a new version of the patch to add --enable/--disable. The main changes are: - When enabling checksums, fsync first the data directory, and at the end then update/flush the control file and its parent folder as Fabien has reported. - When disabling checksums, only work on the control file, as Fabien has also reported. - Rename the control file when beginning the enabling operation, with a callback to rename the file back if the operation is interrupted. Does this make sense? -- Michael
From 2ebb032e7bea22829396e88ff9cc1b52f1b754d4 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 13 Mar 2019 11:12:53 +0900 Subject: [PATCH] Add options to enable and disable checksums in pg_checksums An offline cluster can now work with more modes in pg_checksums: - --enable can enable checksums in a cluster, updating all blocks with a correct checksum, and update the control file at the end. - --disable can disable checksums in a cluster, updating the the control file. - --check is an extra option able to verify checksums for a cluster. When using --disable, only the control file is updated and then flushed. When using --enable, the process gets more complicated as the operation can be long: - Rename the control file to a temporary name, to prevent a parallel startup of Postgres. - Scan all files and update their checksums. - Rename back the control file. - Flush the data directory. - Update the control file and then flush it, to make the change durable. If the operation is interrupted, the control file gets moved back in place. If no mode is specified in the options, then --check is used for compatibility with older versions of pg_verify_checksums (now renamed to pg_checksums in v12). Author: Michael Banck, Michael Paquier Reviewed-by: Fabien Coelho, Magnus Hagander, Sergei Kornilov Discussion: https://postgr.es/m/20181221201616.gd4...@nighthawk.caipicrew.dd-dns.de --- doc/src/sgml/ref/pg_checksums.sgml | 54 ++++- src/bin/pg_checksums/pg_checksums.c | 287 +++++++++++++++++++++++--- src/bin/pg_checksums/t/002_actions.pl | 76 +++++-- src/tools/pgindent/typedefs.list | 1 + 4 files changed, 370 insertions(+), 48 deletions(-) diff --git a/doc/src/sgml/ref/pg_checksums.sgml b/doc/src/sgml/ref/pg_checksums.sgml index 6a47dda683..dc41553bc4 100644 --- a/doc/src/sgml/ref/pg_checksums.sgml +++ b/doc/src/sgml/ref/pg_checksums.sgml @@ -16,7 +16,7 @@ PostgreSQL documentation <refnamediv> <refname>pg_checksums</refname> - <refpurpose>verify data checksums in a <productname>PostgreSQL</productname> database cluster</refpurpose> + <refpurpose>enable, disable or check data checksums in a <productname>PostgreSQL</productname> database cluster</refpurpose> </refnamediv> <refsynopsisdiv> @@ -36,11 +36,24 @@ PostgreSQL documentation <refsect1 id="r1-app-pg_checksums-1"> <title>Description</title> <para> - <application>pg_checksums</application> verifies data checksums in a - <productname>PostgreSQL</productname> cluster. The server must be shut - down cleanly before running <application>pg_checksums</application>. - The exit status is zero if there are no checksum errors, otherwise nonzero. + <application>pg_checksums</application> checks, enables or disables data + checksums in a <productname>PostgreSQL</productname> cluster. The server + must be shut down cleanly before running + <application>pg_checksums</application>. The exit status is zero if there + are no checksum errors when checking them, and nonzero if at least one + checksum failure is detected. If enabling or disabling checksums, the + exit status is nonzero if the operation failed. </para> + + <para> + Checking checksums requires to scan every file holding them in the data + folder. Disabling checksums requires only an update of the file + <filename>pg_control</filename>. Enabling checksums first renames + the file <filename>pg_control</filename> to + <filename>pg_control.pg_checksums_in_progress</filename> to prevent + a parallel startup of the cluster, then it updates all files with + checksums, and it finishes by renaming and updating + <filename>pg_control</filename> to mark checksums as enabled. </refsect1> <refsect1> @@ -60,6 +73,37 @@ PostgreSQL documentation </listitem> </varlistentry> + <varlistentry> + <term><option>-c</option></term> + <term><option>--check</option></term> + <listitem> + <para> + Checks checksums. This is the default mode if nothing else is + specified. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><option>-d</option></term> + <term><option>--disable</option></term> + <listitem> + <para> + Disables checksums. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><option>-e</option></term> + <term><option>--enable</option></term> + <listitem> + <para> + Enables checksums. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><option>-v</option></term> <term><option>--verbose</option></term> diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c index 5d4083fa9f..b98299a292 100644 --- a/src/bin/pg_checksums/pg_checksums.c +++ b/src/bin/pg_checksums/pg_checksums.c @@ -1,7 +1,8 @@ /*------------------------------------------------------------------------- * * pg_checksums.c - * Verifies page level checksums in an offline cluster. + * Checks, enables or disables page level checksums for an offline + * cluster * * Copyright (c) 2010-2019, PostgreSQL Global Development Group * @@ -16,35 +17,78 @@ #include <dirent.h> #include <sys/stat.h> #include <unistd.h> +#include <signal.h> -#include "catalog/pg_control.h" +#include "access/xlog_internal.h" #include "common/controldata_utils.h" +#include "common/file_perm.h" +#include "common/file_utils.h" #include "getopt_long.h" #include "pg_getopt.h" #include "storage/bufpage.h" #include "storage/checksum.h" #include "storage/checksum_impl.h" -#include "storage/fd.h" static int64 files = 0; static int64 blocks = 0; static int64 badblocks = 0; static ControlFileData *ControlFile; - +static char *DataDir = NULL; static char *only_relfilenode = NULL; static bool verbose = false; +static char controlfile_path[MAXPGPATH]; +static char controlfile_path_temp[MAXPGPATH]; + + +typedef enum +{ + PG_MODE_CHECK, + PG_MODE_DISABLE, + PG_MODE_ENABLE +} PgChecksumMode; + +/* + * Filename components. + * + * XXX: fd.h is not declared here as frontend side code is not able to + * interact with the backend-side definitions for the various fsync + * wrappers. + */ +#define PG_TEMP_FILES_DIR "pgsql_tmp" +#define PG_TEMP_FILE_PREFIX "pgsql_tmp" + +/* + * Locations of persistent and temporary control files. The control + * file gets renamed into a temporary location when enabling checksums + * to prevent a parallel startup of Postgres. + */ +#define CONTROL_FILE_PATH "global/pg_control" +#define CONTROL_FILE_PATH_TEMP CONTROL_FILE_PATH ".pg_checksums_in_progress" + + +#ifndef WIN32 +#define pg_mv_file rename +#else +#define pg_mv_file pgrename +#endif + +static PgChecksumMode mode = PG_MODE_CHECK; static const char *progname; static void usage(void) { - printf(_("%s verifies data checksums in a PostgreSQL database cluster.\n\n"), progname); + printf(_("%s enables, disables or verifies data checksums in a PostgreSQL database cluster.\n\n"), progname); printf(_("Usage:\n")); printf(_(" %s [OPTION]... [DATADIR]\n"), progname); printf(_("\nOptions:\n")); printf(_(" [-D, --pgdata=]DATADIR data directory\n")); + printf(_(" -c, --check check data checksums\n")); + printf(_(" This is the default mode if nothing is specified.\n")); + printf(_(" -d, --disable disable data checksums\n")); + printf(_(" -e, --enable enable data checksums\n")); printf(_(" -v, --verbose output verbose messages\n")); printf(_(" -r RELFILENODE check only relation with specified relfilenode\n")); printf(_(" -V, --version output version information, then exit\n")); @@ -54,6 +98,32 @@ usage(void) printf(_("Report bugs to <pgsql-b...@lists.postgresql.org>.\n")); } +/* + * Clean up the temporary control file when enabling checksums in the + * event of an interruption. + */ +static void +signal_cleanup(int signum) +{ + /* nothing to do if there is no temporary control file */ + if (access(controlfile_path_temp, F_OK) != 0) + exit(signum); + + if (pg_mv_file(controlfile_path_temp, controlfile_path)) + { + fprintf(stderr, _("%s: could not rename file \"%s\" to \"%s\": %s\n"), + progname, controlfile_path_temp, controlfile_path, + strerror(errno)); + exit(1); + } + + if (fsync_fname(controlfile_path, false, progname) != 0 || + fsync_parent_path(controlfile_path, progname)) + exit(1); + + exit(signum); +} + /* * List of files excluded from checksum validation. * @@ -61,6 +131,7 @@ usage(void) */ static const char *const skip[] = { "pg_control", + "pg_control.pg_checksums_in_progress", "pg_filenode.map", "pg_internal.init", "PG_VERSION", @@ -90,8 +161,14 @@ scan_file(const char *fn, BlockNumber segmentno) PageHeader header = (PageHeader) buf.data; int f; BlockNumber blockno; + int flags; + + Assert(mode == PG_MODE_ENABLE || + mode == PG_MODE_CHECK); + + flags = (mode == PG_MODE_ENABLE) ? O_RDWR : O_RDONLY; + f = open(fn, PG_BINARY | flags, 0); - f = open(fn, O_RDONLY | PG_BINARY, 0); if (f < 0) { fprintf(stderr, _("%s: could not open file \"%s\": %s\n"), @@ -121,18 +198,47 @@ scan_file(const char *fn, BlockNumber segmentno) continue; csum = pg_checksum_page(buf.data, blockno + segmentno * RELSEG_SIZE); - if (csum != header->pd_checksum) + if (mode == PG_MODE_CHECK) { - if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION) - fprintf(stderr, _("%s: checksum verification failed in file \"%s\", block %u: calculated checksum %X but block contains %X\n"), - progname, fn, blockno, csum, header->pd_checksum); - badblocks++; + if (csum != header->pd_checksum) + { + if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION) + fprintf(stderr, _("%s: checksum verification failed in file \"%s\", block %u: calculated checksum %X but block contains %X\n"), + progname, fn, blockno, csum, header->pd_checksum); + badblocks++; + } + } + else if (mode == PG_MODE_ENABLE) + { + /* Set checksum in page header */ + header->pd_checksum = csum; + + /* Seek back to beginning of block */ + if (lseek(f, -BLCKSZ, SEEK_CUR) < 0) + { + fprintf(stderr, _("%s: seek failed for block %d in file \"%s\": %s\n"), progname, blockno, fn, strerror(errno)); + exit(1); + } + + /* Write block with checksum */ + if (write(f, buf.data, BLCKSZ) != BLCKSZ) + { + fprintf(stderr, "%s: could not update checksum of block %d in file \"%s\": %s\n", + progname, blockno, fn, strerror(errno)); + exit(1); + } } } if (verbose) - fprintf(stderr, - _("%s: checksums verified in file \"%s\"\n"), progname, fn); + { + if (mode == PG_MODE_CHECK) + fprintf(stderr, + _("%s: checksums verified in file \"%s\"\n"), progname, fn); + if (mode == PG_MODE_ENABLE) + fprintf(stderr, + _("%s: checksums enabled in file \"%s\"\n"), progname, fn); + } close(f); } @@ -234,12 +340,14 @@ int main(int argc, char *argv[]) { static struct option long_options[] = { + {"check", no_argument, NULL, 'c'}, {"pgdata", required_argument, NULL, 'D'}, + {"disable", no_argument, NULL, 'd'}, + {"enable", no_argument, NULL, 'e'}, {"verbose", no_argument, NULL, 'v'}, {NULL, 0, NULL, 0} }; - char *DataDir = NULL; int c; int option_index; bool crc_ok; @@ -262,10 +370,19 @@ main(int argc, char *argv[]) } } - while ((c = getopt_long(argc, argv, "D:r:v", long_options, &option_index)) != -1) + while ((c = getopt_long(argc, argv, "cD:der:v", long_options, &option_index)) != -1) { switch (c) { + case 'c': + mode = PG_MODE_CHECK; + break; + case 'd': + mode = PG_MODE_DISABLE; + break; + case 'e': + mode = PG_MODE_ENABLE; + break; case 'v': verbose = true; break; @@ -312,6 +429,15 @@ main(int argc, char *argv[]) exit(1); } + /* Relfilenode checking only works in --check mode */ + if (mode != PG_MODE_CHECK && only_relfilenode) + { + fprintf(stderr, _("%s: relfilenode option only possible with --check\n"), progname); + fprintf(stderr, _("Try \"%s --help\" for more information.\n"), + progname); + exit(1); + } + /* Check if cluster is running */ ControlFile = get_controlfile(DataDir, progname, &crc_ok); if (!crc_ok) @@ -330,29 +456,134 @@ main(int argc, char *argv[]) if (ControlFile->state != DB_SHUTDOWNED && ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY) { - fprintf(stderr, _("%s: cluster must be shut down to verify checksums\n"), progname); + fprintf(stderr, _("%s: cluster must be shut down\n"), progname); exit(1); } - if (ControlFile->data_checksum_version == 0) + if (ControlFile->data_checksum_version == 0 && + mode == PG_MODE_CHECK) { fprintf(stderr, _("%s: data checksums are not enabled in cluster\n"), progname); exit(1); } + if (ControlFile->data_checksum_version == 0 && + mode == PG_MODE_DISABLE) + { + fprintf(stderr, _("%s: data checksums are already disabled in cluster.\n"), progname); + exit(1); + } + if (ControlFile->data_checksum_version > 0 && + mode == PG_MODE_ENABLE) + { + fprintf(stderr, _("%s: data checksums are already enabled in cluster.\n"), progname); + exit(1); + } - /* Scan all files */ - scan_directory(DataDir, "global"); - scan_directory(DataDir, "base"); - scan_directory(DataDir, "pg_tblspc"); + /* + * Allocate the control file paths here, as this gets used in various + * phases. + */ + snprintf(controlfile_path, sizeof(controlfile_path), + "%s/%s", DataDir, CONTROL_FILE_PATH); + snprintf(controlfile_path_temp, sizeof(controlfile_path_temp), + "%s/%s", DataDir, CONTROL_FILE_PATH_TEMP); - printf(_("Checksum scan completed\n")); - printf(_("Data checksum version: %d\n"), ControlFile->data_checksum_version); - printf(_("Files scanned: %s\n"), psprintf(INT64_FORMAT, files)); - printf(_("Blocks scanned: %s\n"), psprintf(INT64_FORMAT, blocks)); - printf(_("Bad checksums: %s\n"), psprintf(INT64_FORMAT, badblocks)); + /* Prevent leaving behind any intermediate state */ + pqsignal(SIGINT, signal_cleanup); + pqsignal(SIGTERM, signal_cleanup); - if (badblocks > 0) - return 1; + /* + * The operation is good to move on with all the sanity checks done. + * Enabling checksums can take a long time as all the files need to + * be scanned and rewritten. Hence, first, prevent any parallel startup + * of the instance by renaming the control file when enabling checksums + * so that it cannot be started by accident during the operation. + */ + if (mode == PG_MODE_ENABLE) + { + printf(_("Renaming \"%s\" to \"%s\"\n"), controlfile_path, + controlfile_path_temp); + if (pg_mv_file(controlfile_path, controlfile_path_temp) != 0) + { + fprintf(stderr, _("%s: could not rename file \"%s\" to \"%s\": %s\n"), + progname, controlfile_path, controlfile_path_temp, + strerror(errno)); + exit(1); + } + } + + /* Operate on all files if checking or enabling checksums */ + if (mode == PG_MODE_CHECK || mode == PG_MODE_ENABLE) + { + scan_directory(DataDir, "global"); + scan_directory(DataDir, "base"); + scan_directory(DataDir, "pg_tblspc"); + + printf(_("Checksum operation completed\n")); + printf(_("Files scanned: %s\n"), psprintf(INT64_FORMAT, files)); + printf(_("Blocks scanned: %s\n"), psprintf(INT64_FORMAT, blocks)); + if (mode == PG_MODE_CHECK) + { + printf(_("Bad checksums: %s\n"), psprintf(INT64_FORMAT, badblocks)); + printf(_("Data checksum version: %d\n"), ControlFile->data_checksum_version); + + if (badblocks > 0) + exit(1); + } + } + + /* + * Now that enabling data checksums is done, first put the control + * file back in place and then flush the data directory. The control + * file is updated and flushed in a follow-up step to never have the + * data folder into an inconsistent state should a crash happen + * in-between. + */ + if (mode == PG_MODE_ENABLE) + { + printf(_("Renaming \"%s\" to \"%s\"\n"), controlfile_path_temp, + controlfile_path); + if (pg_mv_file(controlfile_path_temp, controlfile_path) != 0) + { + fprintf(stderr, _("%s: could not rename file \"%s\" to \"%s\": %s\n"), + progname, controlfile_path_temp, controlfile_path, + strerror(errno)); + exit(1); + } + + printf(_("Syncing data folder\n")); + fsync_pgdata(DataDir, progname, PG_VERSION_NUM); + } + + /* + * Finally update and flush the control file. + */ + if (mode == PG_MODE_ENABLE || mode == PG_MODE_DISABLE) + { + /* Update the control control file */ + printf(_("Updating control file\n")); + ControlFile->data_checksum_version = + (mode == PG_MODE_ENABLE) ? PG_DATA_CHECKSUM_VERSION : 0; + update_controlfile(DataDir, progname, ControlFile); + + /* + * Flush the control file and its parent path to make the change + * durable. + */ + if (fsync_fname(controlfile_path, false, progname) != 0 || + fsync_parent_path(controlfile_path, progname) != 0) + { + /* errors are already logged on failure */ + exit(1); + } + + if (verbose) + printf(_("Data checksum version: %d\n"), ControlFile->data_checksum_version); + if (mode == PG_MODE_ENABLE) + printf(_("Checksums enabled in cluster\n")); + else + printf(_("Checksums disabled in cluster\n")); + } return 0; } diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl index 97284e8930..3ab18a6b89 100644 --- a/src/bin/pg_checksums/t/002_actions.pl +++ b/src/bin/pg_checksums/t/002_actions.pl @@ -5,7 +5,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 45; +use Test::More tests => 62; # Utility routine to create and check a table with corrupted checksums @@ -38,8 +38,8 @@ sub check_relation_corruption # Checksums are correct for single relfilenode as the table is not # corrupted yet. - command_ok(['pg_checksums', '-D', $pgdata, - '-r', $relfilenode_corrupted], + command_ok(['pg_checksums', '--check', '-D', $pgdata, '-r', + $relfilenode_corrupted], "succeeds for single relfilenode on tablespace $tablespace with offline cluster"); # Time to create some corruption @@ -49,15 +49,15 @@ sub check_relation_corruption close $file; # Checksum checks on single relfilenode fail - $node->command_checks_all([ 'pg_checksums', '-D', $pgdata, '-r', - $relfilenode_corrupted], + $node->command_checks_all([ 'pg_checksums', '--check', '-D', $pgdata, + '-r', $relfilenode_corrupted], 1, [qr/Bad checksums:.*1/], [qr/checksum verification failed/], "fails with corrupted data for single relfilenode on tablespace $tablespace"); # Global checksum checks fail as well - $node->command_checks_all([ 'pg_checksums', '-D', $pgdata], + $node->command_checks_all([ 'pg_checksums', '--check', '-D', $pgdata], 1, [qr/Bad checksums:.*1/], [qr/checksum verification failed/], @@ -67,22 +67,22 @@ sub check_relation_corruption $node->start; $node->safe_psql('postgres', "DROP TABLE $table;"); $node->stop; - $node->command_ok(['pg_checksums', '-D', $pgdata], + $node->command_ok(['pg_checksums', '--check', '-D', $pgdata], "succeeds again after table drop on tablespace $tablespace"); $node->start; return; } -# Initialize node with checksums enabled. +# Initialize node with checksums disabled. my $node = get_new_node('node_checksum'); -$node->init(extra => ['--data-checksums']); +$node->init(); my $pgdata = $node->data_dir; -# Control file should know that checksums are enabled. +# Control file should know that checksums are disabled. command_like(['pg_controldata', $pgdata], - qr/Data page checksum version:.*1/, - 'checksums enabled in control file'); + qr/Data page checksum version:.*0/, + 'checksums disabled in control file'); # These are correct but empty files, so they should pass through. append_to_file "$pgdata/global/99999", ""; @@ -100,13 +100,59 @@ append_to_file "$pgdata/global/pgsql_tmp_123", "foo"; mkdir "$pgdata/global/pgsql_tmp"; append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo"; +# Enable checksums. +command_ok(['pg_checksums', '--enable', '-D', $pgdata], + "checksums successfully enabled in cluster"); + +# Successive attempt to enable checksums fails. +command_fails(['pg_checksums', '--enable', '-D', $pgdata], + "enabling checksums fails if already enabled"); + +# Control file should know that checksums are enabled. +command_like(['pg_controldata', $pgdata], + qr/Data page checksum version:.*1/, + 'checksums enabled in control file'); + +# Disable checksums again. +command_ok(['pg_checksums', '--disable', '-D', $pgdata], + "checksums successfully disabled in cluster"); + +# Successive attempt to disable checksums fails. +command_fails(['pg_checksums', '--disable', '-D', $pgdata], + "disabling checksums fails if already disabled"); + +# Control file should know that checksums are disabled. +command_like(['pg_controldata', $pgdata], + qr/Data page checksum version:.*0/, + 'checksums disabled in control file'); + +# Enable checksums again for follow-up tests. +command_ok(['pg_checksums', '--enable', '-D', $pgdata], + "checksums successfully enabled in cluster"); + +# Control file should know that checksums are enabled. +command_like(['pg_controldata', $pgdata], + qr/Data page checksum version:.*1/, + 'checksums enabled in control file'); + # Checksums pass on a newly-created cluster -command_ok(['pg_checksums', '-D', $pgdata], +command_ok(['pg_checksums', '--check', '-D', $pgdata], "succeeds with offline cluster"); +# Checksums are verified if no other arguments are specified +command_ok(['pg_checksums', '-D', $pgdata], + "verifies checksums as default action"); + +# Specific relation files cannot be requested when action is --disable +# or --enable. +command_fails(['pg_checksums', '--disable', '-r', '1234', '-D', $pgdata], + "fails when relfilenodes are requested and action is --disable"); +command_fails(['pg_checksums', '--enable', '-r', '1234', '-D', $pgdata], + "fails when relfilenodes are requested and action is --enable"); + # Checks cannot happen with an online cluster $node->start; -command_fails(['pg_checksums', '-D', $pgdata], +command_fails(['pg_checksums', '--check', '-D', $pgdata], "fails with online cluster"); # Check corruption of table on default tablespace. @@ -133,7 +179,7 @@ sub fail_corrupt my $file_name = "$pgdata/global/$file"; append_to_file $file_name, "foo"; - $node->command_checks_all([ 'pg_checksums', '-D', $pgdata], + $node->command_checks_all([ 'pg_checksums', '--check', '-D', $pgdata], 1, [qr/^$/], [qr/could not read block 0 in file.*$file\":/], diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b821df9e71..e86fecb849 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1697,6 +1697,7 @@ PgBenchExprType PgBenchFunction PgBenchValue PgBenchValueType +PgChecksumMode PgFdwAnalyzeState PgFdwDirectModifyState PgFdwModifyState -- 2.20.1
signature.asc
Description: PGP signature
