Re: PITR promote bug: Checkpointer writes to older timeline

On Mon, Mar 15, 2021 at 03:01:09PM +0900, Kyotaro Horiguchi wrote:
> Logical decoding stuff is (I think) designed to turn any backend into
> a walsender, which may need to maintain ThisTimeLineID.  It seems to
> me that logical decoding stuff indents to maintain ThisTimeLineID of
> such backends at reading a WAL record. logical_read_xlog_page also
> updates ThisTimeLineID and pg_logical_slot_get_changes_guts(),
> pg_replication_slot_advance() (and maybe other functions) updates
> ThisTimeLineID. So it is natural that local_read_xlog_page() updates
> it since it is intended to be used used in logical decoding plugins.

Logical decoding contexts cannot be created while in recovery as per
CheckLogicalDecodingRequirements(), and as mentioned not everything is
in place to allow that.  FWIW, I think that it is just confusing for
pg_replication_slot_advance() and pg_logical_slot_get_changes_guts()
to update it, and we just look for the latest value each time it is
necessary when reading a new WAL page.

> It is initialized by IndentifySystem(). And the logical walsender
> intends to maintain ThisTimeLineID by subsequent calls to
> GetStandbyFlushRecPtr(), which happen in logical_read_xlog_page().

I don't understand this part about logical_read_xlog_page(), though.
Do you mean a different routine or a different code path?

> I agree that it's better that the replay TLI is stored in a separate
> variable. It is what I was complained on in the previous mails. (It
> might not have been so obvious, though..)

Okay.  I understood that this was what you implied.

> I don't think there's any acutual user of the function for the
> purpose, but..  Anyawy if we remove the update of ThisTimeLineID from
> read_local_xlog_page, I think we should remove or rewrite the
> following comment for the function.  It no longer works as written in
> the catchphrase.

Who knows.  We cannot know all the users of this code, and the API is
public.

> > * Public because it would likely be very helpful for someone writing another
> > * output method outside walsender, e.g. in a bgworker.

I don't see a reason to remove this comment as this routine can still
be useful for many purposes.  What kind of rewording do you have in
mind?
--
Michael