Re: pg_rewind docs correction

On Sun, Mar 08, 2020 at 05:13:21PM -0400, James Coleman wrote:
> I've added the information about how the backup label control file is
> written, and updated the How It Works steps to refer to that separately
> from restart.
>
> Additionally the How It Works is updated to include WAL segments and new
> relation files in the list of files copied wholesale, since that was
> previously stated but somewhat contradicted there.

- The result is equivalent to replacing the target data directory with the
- source one. Only changed blocks from relation files are copied;
- all other files are copied in full, including configuration files. The
- advantage of <application>pg_rewind</application> over taking a new base backup, or
- tools like <application>rsync</application>, is that <application>pg_rewind</application> does
- not require reading through unchanged blocks in the cluster. This makes
- it a lot faster when the database is large and only a small
- fraction of blocks differ between the clusters.
+ After a successful rewind and subsequent WAL replay, the target data
+ directory is equivalent to a base backup of the source data directory. While
+ only changed blocks from existing relation files are copied; all other files
+ are copied in full, including new relation files, configuration files, and WAL
+ segments. The advantage of <application>pg_rewind</application> over taking a

The first sentence you are adding refers to "subsequent WAL replay".
However, this paragraph emphasizes with the state of the target
cluster after running pg_rewind but *before* make the target cluster
start recovery. So shouldn't you just remove the part "and subsequent
WAL replay" from your first new sentence?

In the same paragraph, I think that you should remove the "While" from
"While only changed blocks", as the second part of the sentence refers
to the other files, WAl segments, etc.

The second paragraph of the docs regarding timeline lookup is
unchanged, which is fine.

- When the target server is started for the first time after running
- <application>pg_rewind</application>, it will go into recovery mode and replay all
- WAL generated in the source server after the point of divergence.
+ After running <application>pg_rewind</application> the data directory is
+ not immediately in a consistent state. However
+ <application>pg_rewind</application> configures the control file so that when
+ the target server is started again it will enter recovery mode and replay all
+ WAL generated in the source server after the point of divergence.

The second part of the third paragraph is not changed, and the
modification you are doing here is about the control file. I am
still unconvinced that this is a good change, because mentioning the
control file would be actually more adapted to the part "How it
works", where you are adding details about the backup_label file, and
already include details about the minimum consistency LSN itself
stored in the control file.

+ <para>
+ Because <application>pg_rewind</application> copies configuration files
+ entirely from the source, correcting recovery configuration options before
+ restarting the server is necessary if you intend to re-introduce the target
+ as a replica of the source. If you restart the server after the rewind
+ operation has finished but without configuring recovery, the target will
+ again diverge from the primary.
+ </para>

True that this is not outlined enough.

+ The relation files are now to their state at the last checkpoint completed
+ prior to the point at which the WAL timelines of the source and target
+ diverged plus the current state on the source of any blocks changed on the
+ target after that divergence.

"Relation files are now in a state equivalent to the moment of the
last completed checkpoint prior to the point.."?

- <filename>pg_stat_tmp/</filename>, and
- <filename>pg_subtrans/</filename> are omitted from the data copied
- from the source cluster. Any file or directory beginning with
- <filename>pgsql_tmp</filename> is omitted, as well as are
+ <filename>pg_stat_tmp/</filename>, and <filename>pg_subtrans/</filename>
+ are omitted from the data copied from the source cluster. The files

This is just reorganizing an existing list, why?

+ Create a backup label file to begin WAL replay at the checkpoint created
+ at failover and a minimum consistency LSN using
+ <literal>pg_current_wal_insert_lsn()</literal>, when using a live source
+ and the last checkpoint LSN, when using a stopped source.

Now would be the moment to mention the control file.

> I realized I didn't previously add this to the CF; since it's not a new
> patch I've added it to the current CF, but if this is incorrect please let
> me know.

The last CF of Postgres 13 began at the beginning of February :(
--
Michael