Re: doc review for v13

On Tue, Aug 18, 2020 at 12:17:03PM -0500, Justin Pryzby wrote:
> The WAL sender process is currently performing
> - <function>pg_start_backup</function> to set up for
> - taking a base backup, and waiting for backup start
> + <function>pg_start_backup</function> to prepare to
> + take a base backup, and waiting for the start-of-backup
> checkpoint to finish.

Wouldn't it be more simple to use "to prepare for a base backup" here?

> Run the dump in parallel by dumping <replaceable class="parameter">njobs</replaceable>
> - tables simultaneously. This option reduces the time of the dump but it also
> + tables simultaneously. This option may reduces the time needed to perform the dump but it also
> increases the load on the database server. You can only use this option with the
> [...]
> Execute the reindex commands in parallel by running
> <replaceable class="parameter">njobs</replaceable>
> - commands simultaneously. This option reduces the time of the
> - processing but it also increases the load on the database server.
> + commands simultaneously. This option may reduce the processing time
> + but it also increases the load on the database server.
> [...]
> Execute the vacuum or analyze commands in parallel by running
> <replaceable class="parameter">njobs</replaceable>
> - commands simultaneously. This option reduces the time of the
> - processing but it also increases the load on the database server.
> + commands simultaneously. This option may reduce the processing time
> + but it also increases the load on the database server.
> </para>
> <para>
> <application>vacuumdb</application> will open

The original versions are fine IMO.

> Replication is only supported by tables, including partitioned tables.
> Attempts to replicate other types of relations such as views, materialized
> - views, or foreign tables, will result in an error.
> + views, or foreign tables will result in an error.
> </para>

I think that the original is fine.

> * If the partition's attributes don't match the root relation's, we'll
> * need to make a new attrmap which maps partition attribute numbers to
> - * remoterel's, instead the original which maps root relation's attribute
> + * remoterel's, instead of the original which maps root relation's attribute
> * numbers to remoterel's.

Indeed.

> from the parent table will be created in the partition, if they don't
> already exist.
> If any of the <literal>CHECK</literal> constraints of the table being
> - attached is marked <literal>NO INHERIT</literal>, the command will fail;
> + attached are marked <literal>NO INHERIT</literal>, the command will fail;
> such constraints must be recreated without the
> <literal>NO INHERIT</literal> clause.

Singular or plural depends on the context when if comes to any with a
countable word, and plural looks more natural to me here. So, right.

> enough for error messages. Detail and hint messages can be relegated to a
> verbose mode, or perhaps a pop-up error-details window. Also, details and
> hints would normally be suppressed from the server log to save
> - space. Reference to implementation details is best avoided since users
> - aren't expected to know the details.
> + space. References to implementation details are best avoided since users
> + aren't expected to know them.

Original is fine IMO (see 6335c80).

> not contain any checksums. Otherwise, it will contain a checksum
> of each file in the backup using the specified algorithm. In addition,
> the manifest will always contain a <literal>SHA256</literal>
> - checksum of its own contents. The <literal>SHA</literal> algorithms
> + checksum of its own content. The <literal>SHA</literal> algorithms
> are significantly more CPU-intensive than <literal>CRC32C</literal>,
> so selecting one of them may increase the time required to complete
> the backup.
> [...]
> every check which will be performed by a running server when attempting
> to make use of the backup. Even if you use this tool, you should still
> perform test restores and verify that the resulting databases work as
> - expected and that they appear to contain the correct data. However,
> + expected and that they contain the correct data. However,
> <application>pg_verifybackup</application> can detect many problems
> that commonly occur due to storage problems or user error.
> [...]
> @@ -82,7 +82,7 @@ PostgreSQL documentation
> for any files for which the computed checksum does not match the
> checksum stored in the manifest. This step is not performed for any files
> which produced errors in the previous step, since they are already known
> - to have problems. Also, files which were ignored in the previous step are
> + to have problems. Files which were ignored in the previous step are
> also ignored in this step.

No sure this needs to change

> </para>
>
> @@ -121,7 +121,7 @@ PostgreSQL documentation
> <title>Options</title>
>
> <para>
> - The following command-line options control the behavior.
> + The following command-line options control the behavior of this program.

"pg_verifybackup accepts the following command-line arguments:" is
more consistent with the style of all the other tools. This needs to
be fixed.

> The <productname>PostgreSQL</productname> server will listen for both
> normal and <acronym>GSSAPI</acronym>-encrypted connections on the same TCP
> - port, and will negotiate with any connecting client on whether to
> + port, and will negotiate with any connecting client whether to
> use <acronym>GSSAPI</acronym> for encryption (and for authentication). By

Right.

> specify suppression of the <literal>CONTEXT:</literal> portion of a message in
> the postmaster log. This should only be used for verbose debugging
> messages where the repeated inclusion of context would bloat the log
> - volume too much.
> + too much.

Okay here.

> A logical slot will emit each change just once in normal operation.
> The current position of each slot is persisted only at checkpoint, so in
> the case of a crash the slot may return to an earlier LSN, which will
> - then cause recent changes to be resent when the server restarts.
> + then cause recent changes to be re-sent when the server restarts.
> Logical decoding clients are responsible for avoiding ill effects from
> handling the same message more than once. Clients may wish to record
> the last LSN they saw when decoding and skip over any repeated data or
> [...]
> It is safe to use <literal>off</literal> for logical replication:
> If the subscriber loses transactions because of missing
> - synchronization, the data will be resent from the publisher.
> + synchronization, the data will be re-sent from the publisher.
> </para>
> [...]
> - /* prevent signal from being resent more than once */
> + /* prevent signal from being re-sent more than once */
> allow_autovacuum_cancel = false;

"resent" is wrong, but "re-sent" does not sound like the best choice
to me. Shouldn't we just say "sent again" for all three places?
--
Michael