Re: pg_rewind docs correction

On Sun, Sep 15, 2019 at 10:36:04AM -0400, James Coleman wrote:
> On Sun, Sep 15, 2019 at 10:25 AM Michael Paquier <michael@paquier.xyz> wrote:
>> +    The rewind operation is not expected to result in a consistent data
>> +    directory state either internally to the node or with respect to the rest
>> +    of the cluster. Instead the resulting data directory will only be consistent
>> +    after WAL replay has completed to at least the LSN at which changed blocks
>> +    copied from the source were originally written on the source.
>>
>> That's not necessarily true.  pg_rewind enforces in the control file
>> of the target the minimum consistency LSN to be
>> pg_current_wal_insert_lsn() when using a live source or the last
>> checkpoint LSN for a stopped source, so while that sounds true from
>> the point of view of all the blocks copied, the control file may still
>> cause a complain that the target recovering has not reached its
>> consistent point even if all the blocks are already at a position
>> not-so-far from what has been registered in the control file.
>
> I could just say "after WAL replay has completed to a consistent state"?

I still would not change this paragraph.  The first sentence means
that we have an equivalency, because that's the case if you think
about it as we make sure that the target is able to sync with the
source, and the target gets into a state where it as an on-disk state
equivalent to the target up to the minimum consistency point defined
in the control file once the tool has done its work (this last point
is too precise to be included in a global description to be honest).
And the second sentence makes clear what are the actual diffs are.

>> +   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. While only changed blocks from existing relation files are
>>
>> And here we could mention that all the blocks copied from the source
>> are the ones which are found in the WAL records of the target until
>> the end of WAL of its timeline.  Still, that's basically what is
>> mentioned in the first part of "How It Works", which explains things
>> better.  I honestly don't really see that all this paragraph is an
>> improvement over the simplicity of the original when it comes to
>> understand the global idea of what pg_rewind does.
>
> The problem with the original is that while simple, it's actually
> incorrect in that simplicity. Pg_rewind does *not* result in the data
> directory on the target matching the data directory on the source.

That's not what I get from the original docs, but I may be too much
used to it.

>> +  <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>
>>
>> No objections regarding that part.  Now it seems to me that we had
>> better apply that to the last part of "How it works" instead?  I kind
>> of agree that the last paragraph could provide more details regarding
>> the risks of overwriting the wanted configuration.  The existing docs
>> also mention that pg_rewind only creates a backup_label file to start
>> recovery, perhaps we could mention up to which point recovery happens
>> in this section?  There is a bit more here than just "apply the WAL".
>
> I'll look to see if there's a better place to put this.

Thanks.  From what I can see, we could improve further the doc part
about how the tool works in details, especially regarding the
configuration files which may get overwritten and be more precise
about that.
--
Michael