Date: Sun, 28 Feb 2021 22:33:55 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org, Bruce Momjian <bruce@momjian.us>,
	Thomas Munro <thomas.munro@gmail.com>
Subject: Re: doc review for v14
Message-ID: <20210301043354.GJ20769@telsasoft.com>
References: <X+RNUu+zotOjfL7K@paquier.xyz>
 <20201227202604.GC26311@telsasoft.com>
 <YAv3TG9fOrNXxJoK@paquier.xyz>
 <YBD/jjCqGoSFeUWW@paquier.xyz>
 <YBO6ZGbYRoc5Rkra@paquier.xyz>
 <20210210235557.GQ20012@telsasoft.com>
 <20210222080345.GA23313@telsasoft.com>
 <YDX924cg0EKAgW8v@paquier.xyz>
 <20210301004647.GF20769@telsasoft.com>
 <YDxpXjWpLj2Ww2n7@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YDxpXjWpLj2Ww2n7@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Mon, Mar 01, 2021 at 01:11:10PM +0900, Michael Paquier wrote:
> On Sun, Feb 28, 2021 at 06:46:47PM -0600, Justin Pryzby wrote:
> > It looks like you applied 0010...but I agree that it's not an improvement.  It
> > appears that's something I intended to go back and revisit myself.
> > The rest of the patch looks right, to me.
> 
> Oops.  This was not intended.
> 
> > I'm suggesting to either revert that part, or apply these more polished changes
> > in 0002.
> 
> I would just group both things together.  Monday helping, I can see
> that the new wording is better on a couple of points after doing a
> diff of wal.sgml with c82d59d6:
> - "checksum protected" in the first sentence is weird, so I agree that
> using "By default, data pages are not protected by checksums" is an
> improvement.
> - "assigned" is indeed a bit strange, "includes" is an improvement,
> and I would tend to not use a passive form here.

+1

> - "to recover from corrupt data" is redundant with "to recover data"
> so the second one should be removed.  My take is to use "page
> corruptions" instead of "corrupt data", which should be corrupted data
> to be grammatically correct.

> -   Checksums verification is normally ENABLED when the cluster is initialized using <link
> +   Checksums are normally enabled when the cluster is initialized using <link

I still have an issue with the sentence that begins:
"Checksums are normally enabled..."

It sounds much too close to "Checksums are typically enabled.", which is wrong.
So I proposed something like:

|Enabling checksums is normally done when the cluster is first created by <link
|...

Note, the patch I sent said "create" but should be "created".

> -   When attempting to recover from corrupt data, it may be necessary to bypass
> -   the checksum protection. To do this, temporarily set the configuration
> -   parameter <xref linkend="guc-ignore-checksum-failure" />.
> +   When attempting to recover from page corruptions, it may be necessary to
> +   bypass the checksum protection. To do this, temporarily set the
> +   configuration parameter <xref linkend="guc-ignore-checksum-failure" />.

"page corruptions" is wrong .. you could say "corrupt pages"

-- 
Justin