Small language fixes in comments and user-facing docs.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 88efb38556..39596db193 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -26162,7 +26162,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <para> The functions shown in <xref linkend="functions-data-sanity-table"/> - provide means to check for health of data file in a cluster. + provide a means to check for health of a data file in a cluster. </para> <table id="functions-data-sanity-table"> @@ -26179,8 +26179,8 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <literal><function>pg_check_relation(<parameter>relation</parameter> <type>regclass</type> [, <parameter>fork</parameter> <type>text</type>])</function></literal> </entry> <entry><type>setof record</type></entry> - <entry>Validate the checksums for all blocks of all or the given fork of - a given relation.</entry> + <entry>Validate the checksum for all blocks of a relation. + </entry> </row> </tbody> </tgroup> @@ -26190,15 +26190,15 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <primary>pg_check_relation</primary> </indexterm> <para id="functions-check-relation-note" xreflabel="pg_check_relation"> - <function>pg_check_relation</function> iterates over all the blocks of a - given relation and verify their checksum. If provided, - <replaceable>fork</replaceable> should be <literal>'main'</literal> for the + <function>pg_check_relation</function> iterates over all blocks of a + given relation and verifies their checksums. If passed, + <replaceable>fork</replaceable> specifies that only checksums of the given + fork are to be verified. Fork should be <literal>'main'</literal> for the main data fork, <literal>'fsm'</literal> for the free space map, <literal>'vm'</literal> for the visibility map, or - <literal>'init'</literal> for the initialization fork, and only this - specific fork will be verifies, otherwise all forks will. The function - returns the list of blocks for which the found checksum doesn't match the - expected one. See <xref + <literal>'init'</literal> for the initialization fork. + The function returns a list of blocks for which the computed and stored + checksums don't match. See <xref linkend="runtime-config-resource-checksum-verification-cost"/> for information on how to configure cost-based verification delay. You must be a member of the <literal>pg_read_all_stats</literal> role to use this diff --git a/src/backend/storage/page/checksum.c b/src/backend/storage/page/checksum.c index eb2c919c34..17cd95ec95 100644 --- a/src/backend/storage/page/checksum.c +++ b/src/backend/storage/page/checksum.c @@ -36,7 +36,7 @@ * actual storage, you have to discard the operating system cache before * running those functions. * - * To avoid torn page and possible false positive when reading data, and + * To avoid torn pages and possible false positives when reading data, and to * keeping overhead as low as possible, the following heuristics are used: * * - a shared LWLock is taken on the target buffer pool partition mapping, and @@ -92,8 +92,8 @@ check_one_block(Relation relation, ForkNumber forknum, BlockNumber blkno, *chk_expected = *chk_found = NoComputedChecksum; /* - * To avoid too much overhead, the buffer will be first read without - * the locks that would guarantee the lack of false positive, as such + * To avoid excessive overhead, the buffer will be first read without + * the locks that would prevent false positives, as such * events should be quite rare. */ Retry: @@ -120,10 +120,10 @@ Retry: } /* - * Perform a checksum check on the passed page. Returns whether the page is + * Perform a checksum check on the passed page. Return True iff the page is * valid or not, and assign the expected and found checksum in chk_expected and * chk_found, respectively. Note that a page can look like new but could be - * the result of a corruption. We still check for this case, but we can't + * the result of corruption. We still check for this case, but we can't * compute its checksum as pg_checksum_page() is explicitly checking for * non-new pages, so NoComputedChecksum will be set in chk_found. */ @@ -139,7 +139,7 @@ check_buffer(char *buffer, uint32 blkno, uint16 *chk_expected, if (PageIsNew(page)) { /* - * Check if the page is really new or if there's a corruption that + * Check if the page is really new or if there's corruption that * affected PageIsNew detection. Note that PageIsVerified won't try to * detect checksum corruption in this case, so there's no risk of * duplicated corruption report. @@ -151,7 +151,7 @@ check_buffer(char *buffer, uint32 blkno, uint16 *chk_expected, } /* - * There's a corruption, but since this affect PageIsNew, we + * There's corruption, but since this affects PageIsNew, we * can't compute a checksum, so set NoComputedChecksum for the * expected checksum. */ @@ -218,8 +218,8 @@ check_delay_point(void) * held. Reading with this lock is to avoid the unlikely but possible case * that a buffer wasn't present in shared buffers when we checked but it then * alloc'ed in shared_buffers, modified and flushed concurrently when we - * later try to read it, leading to false positive due to torn page. Caller - * can read first the buffer without holding the target buffer mapping + * later try to read it, leading to false positives due to a torn page. Caller + * can first read the buffer without holding the target buffer mapping * partition LWLock to have an optimistic approach, and reread the buffer * from disk in case of error. * @@ -280,7 +280,7 @@ check_get_buffer(Relation relation, ForkNumber forknum, checkit = false; /* - * Read the buffer from disk, taking on IO lock to prevent torn-page + * Read the buffer from disk, taking an IO lock to prevent torn-page * reads, in the unlikely event that it was concurrently dirtied and * flushed. */ @@ -320,7 +320,7 @@ check_get_buffer(Relation relation, ForkNumber forknum, /* * Didn't find it in the buffer pool and didn't read it while holding the * buffer mapping partition lock. We'll have to try to read it from - * disk, after releasing the target partition lock to avoid too much + * disk, after releasing the target partition lock to avoid excessive * overhead. It means that it's possible to get a torn page later, so * we'll have to retry with a suitable lock in case of error to avoid * false positive. diff --git a/src/backend/utils/adt/checksumfuncs.c b/src/backend/utils/adt/checksumfuncs.c index d005b8d01f..fa5823677a 100644 --- a/src/backend/utils/adt/checksumfuncs.c +++ b/src/backend/utils/adt/checksumfuncs.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * checksumfuncs.c - * Functions for checksums related feature such as online verification + * Functions for checksum related feature such as online verification * * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -181,7 +181,7 @@ check_relation_fork(TupleDesc tupdesc, Tuplestorestate *tupstore, if (check_one_block(relation, forknum, blkno, &chk_expected, &chk_found)) { - /* Buffer not corrupted or no worth checking, continue */ + /* Buffer not corrupted or not worth checking, continue */ continue; } @@ -192,7 +192,7 @@ check_relation_fork(TupleDesc tupdesc, Tuplestorestate *tupstore, values[i++] = Int32GetDatum(forknum); values[i++] = UInt32GetDatum(blkno); /* - * This can happen if a corruption makes the block appears as + * This can happen if corruption makes the block appears as * PageIsNew() but isn't a new page. */ if (chk_expected == NoComputedChecksum) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 5a51dccca9..57401580c3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2383,7 +2383,7 @@ static struct config_int ConfigureNamesInt[] = { {"checksum_cost_page", PGC_USERSET, RESOURCES_CHECKSUM_DELAY, - gettext_noop("Checksum cost for verifying a page found."), + gettext_noop("Checksum cost for verifying a page."), NULL }, &ChecksumCostPage, diff --git a/src/test/check_relation/t/01_checksums_check.pl b/src/test/check_relation/t/01_checksums_check.pl index 1ad34adcb9..2a3f2880ea 100644 --- a/src/test/check_relation/t/01_checksums_check.pl +++ b/src/test/check_relation/t/01_checksums_check.pl @@ -218,7 +218,7 @@ $ENV{PGOPTIONS} = '--client-min-messages=WARNING'; my ($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT" . " current_setting('data_checksums')"); -is($stdout, 'on', 'Data checksums shoud be enabled'); +is($stdout, 'on', 'Data checksums should be enabled'); ($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT" . " current_setting('block_size')"); @@ -254,7 +254,7 @@ is(check_checksums_call($node, 'u_t1_id_idx'), '1', 'Can check an unlogged index . " current_setting('data_directory') || '/' || pg_relation_filepath('t1')" ); -isnt($stdout, '', 'A relfinode should be returned'); +isnt($stdout, '', 'A relfilenode should be returned'); my $filename = $stdout; -- 2.17.0
>From 7b9e61d640b5137dbcd0221dae0c293b705989ba Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sun, 12 Jul 2020 12:25:34 -0500 Subject: [PATCH] fixes for online checksum verification --- doc/src/sgml/func.sgml | 20 ++++++++--------- src/backend/storage/page/checksum.c | 22 +++++++++---------- src/backend/utils/adt/checksumfuncs.c | 6 ++--- src/backend/utils/misc/guc.c | 2 +- .../check_relation/t/01_checksums_check.pl | 4 ++-- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 88efb38556..39596db193 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -26162,7 +26162,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <para> The functions shown in <xref linkend="functions-data-sanity-table"/> - provide means to check for health of data file in a cluster. + provide a means to check for health of a data file in a cluster. </para> <table id="functions-data-sanity-table"> @@ -26179,8 +26179,8 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <literal><function>pg_check_relation(<parameter>relation</parameter> <type>regclass</type> [, <parameter>fork</parameter> <type>text</type>])</function></literal> </entry> <entry><type>setof record</type></entry> - <entry>Validate the checksums for all blocks of all or the given fork of - a given relation.</entry> + <entry>Validate the checksum for all blocks of a relation. + </entry> </row> </tbody> </tgroup> @@ -26190,15 +26190,15 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8'); <primary>pg_check_relation</primary> </indexterm> <para id="functions-check-relation-note" xreflabel="pg_check_relation"> - <function>pg_check_relation</function> iterates over all the blocks of a - given relation and verify their checksum. If provided, - <replaceable>fork</replaceable> should be <literal>'main'</literal> for the + <function>pg_check_relation</function> iterates over all blocks of a + given relation and verifies their checksums. If passed, + <replaceable>fork</replaceable> specifies that only checksums of the given + fork are to be verified. Fork should be <literal>'main'</literal> for the main data fork, <literal>'fsm'</literal> for the free space map, <literal>'vm'</literal> for the visibility map, or - <literal>'init'</literal> for the initialization fork, and only this - specific fork will be verifies, otherwise all forks will. The function - returns the list of blocks for which the found checksum doesn't match the - expected one. See <xref + <literal>'init'</literal> for the initialization fork. + The function returns a list of blocks for which the computed and stored + checksums don't match. See <xref linkend="runtime-config-resource-checksum-verification-cost"/> for information on how to configure cost-based verification delay. You must be a member of the <literal>pg_read_all_stats</literal> role to use this diff --git a/src/backend/storage/page/checksum.c b/src/backend/storage/page/checksum.c index eb2c919c34..17cd95ec95 100644 --- a/src/backend/storage/page/checksum.c +++ b/src/backend/storage/page/checksum.c @@ -36,7 +36,7 @@ * actual storage, you have to discard the operating system cache before * running those functions. * - * To avoid torn page and possible false positive when reading data, and + * To avoid torn pages and possible false positives when reading data, and to * keeping overhead as low as possible, the following heuristics are used: * * - a shared LWLock is taken on the target buffer pool partition mapping, and @@ -92,8 +92,8 @@ check_one_block(Relation relation, ForkNumber forknum, BlockNumber blkno, *chk_expected = *chk_found = NoComputedChecksum; /* - * To avoid too much overhead, the buffer will be first read without - * the locks that would guarantee the lack of false positive, as such + * To avoid excessive overhead, the buffer will be first read without + * the locks that would prevent false positives, as such * events should be quite rare. */ Retry: @@ -120,10 +120,10 @@ Retry: } /* - * Perform a checksum check on the passed page. Returns whether the page is + * Perform a checksum check on the passed page. Return True iff the page is * valid or not, and assign the expected and found checksum in chk_expected and * chk_found, respectively. Note that a page can look like new but could be - * the result of a corruption. We still check for this case, but we can't + * the result of corruption. We still check for this case, but we can't * compute its checksum as pg_checksum_page() is explicitly checking for * non-new pages, so NoComputedChecksum will be set in chk_found. */ @@ -139,7 +139,7 @@ check_buffer(char *buffer, uint32 blkno, uint16 *chk_expected, if (PageIsNew(page)) { /* - * Check if the page is really new or if there's a corruption that + * Check if the page is really new or if there's corruption that * affected PageIsNew detection. Note that PageIsVerified won't try to * detect checksum corruption in this case, so there's no risk of * duplicated corruption report. @@ -151,7 +151,7 @@ check_buffer(char *buffer, uint32 blkno, uint16 *chk_expected, } /* - * There's a corruption, but since this affect PageIsNew, we + * There's corruption, but since this affects PageIsNew, we * can't compute a checksum, so set NoComputedChecksum for the * expected checksum. */ @@ -218,8 +218,8 @@ check_delay_point(void) * held. Reading with this lock is to avoid the unlikely but possible case * that a buffer wasn't present in shared buffers when we checked but it then * alloc'ed in shared_buffers, modified and flushed concurrently when we - * later try to read it, leading to false positive due to torn page. Caller - * can read first the buffer without holding the target buffer mapping + * later try to read it, leading to false positives due to a torn page. Caller + * can first read the buffer without holding the target buffer mapping * partition LWLock to have an optimistic approach, and reread the buffer * from disk in case of error. * @@ -280,7 +280,7 @@ check_get_buffer(Relation relation, ForkNumber forknum, checkit = false; /* - * Read the buffer from disk, taking on IO lock to prevent torn-page + * Read the buffer from disk, taking an IO lock to prevent torn-page * reads, in the unlikely event that it was concurrently dirtied and * flushed. */ @@ -320,7 +320,7 @@ check_get_buffer(Relation relation, ForkNumber forknum, /* * Didn't find it in the buffer pool and didn't read it while holding the * buffer mapping partition lock. We'll have to try to read it from - * disk, after releasing the target partition lock to avoid too much + * disk, after releasing the target partition lock to avoid excessive * overhead. It means that it's possible to get a torn page later, so * we'll have to retry with a suitable lock in case of error to avoid * false positive. diff --git a/src/backend/utils/adt/checksumfuncs.c b/src/backend/utils/adt/checksumfuncs.c index d005b8d01f..fa5823677a 100644 --- a/src/backend/utils/adt/checksumfuncs.c +++ b/src/backend/utils/adt/checksumfuncs.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * checksumfuncs.c - * Functions for checksums related feature such as online verification + * Functions for checksum related feature such as online verification * * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -181,7 +181,7 @@ check_relation_fork(TupleDesc tupdesc, Tuplestorestate *tupstore, if (check_one_block(relation, forknum, blkno, &chk_expected, &chk_found)) { - /* Buffer not corrupted or no worth checking, continue */ + /* Buffer not corrupted or not worth checking, continue */ continue; } @@ -192,7 +192,7 @@ check_relation_fork(TupleDesc tupdesc, Tuplestorestate *tupstore, values[i++] = Int32GetDatum(forknum); values[i++] = UInt32GetDatum(blkno); /* - * This can happen if a corruption makes the block appears as + * This can happen if corruption makes the block appears as * PageIsNew() but isn't a new page. */ if (chk_expected == NoComputedChecksum) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 5a51dccca9..57401580c3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2383,7 +2383,7 @@ static struct config_int ConfigureNamesInt[] = { {"checksum_cost_page", PGC_USERSET, RESOURCES_CHECKSUM_DELAY, - gettext_noop("Checksum cost for verifying a page found."), + gettext_noop("Checksum cost for verifying a page."), NULL }, &ChecksumCostPage, diff --git a/src/test/check_relation/t/01_checksums_check.pl b/src/test/check_relation/t/01_checksums_check.pl index 1ad34adcb9..2a3f2880ea 100644 --- a/src/test/check_relation/t/01_checksums_check.pl +++ b/src/test/check_relation/t/01_checksums_check.pl @@ -218,7 +218,7 @@ $ENV{PGOPTIONS} = '--client-min-messages=WARNING'; my ($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT" . " current_setting('data_checksums')"); -is($stdout, 'on', 'Data checksums shoud be enabled'); +is($stdout, 'on', 'Data checksums should be enabled'); ($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT" . " current_setting('block_size')"); @@ -254,7 +254,7 @@ is(check_checksums_call($node, 'u_t1_id_idx'), '1', 'Can check an unlogged index . " current_setting('data_directory') || '/' || pg_relation_filepath('t1')" ); -isnt($stdout, '', 'A relfinode should be returned'); +isnt($stdout, '', 'A relfilenode should be returned'); my $filename = $stdout; -- 2.17.0