Horton

Re: Race condition in recovery?

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-05-17T05:16:24Z
Lists: pgsql-hackers 
On Mon, May 17, 2021 at 10:09 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, May 17, 2021 at 8:50 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > Before the commit expectedTLEs is always initialized with just one
> > entry for the TLI of the last checkpoint record.
>
> Right
>
> > (1) If XLogFileReadAnyTLI() found the segment but no history file
> > found, that is, using the dummy TLE-list, expectedTLEs is initialized
> > with the dummy one-entry list. So there's no behavioral change in this
> > aspect.
>
> Yeah, you are right.

But do you agree that one line entry will always be a checkpoint
timeline entry?  Because if you notice below code[1] in function
"readRecoveryCommandFile();", then you will realize that once we come
out of this function either the "recoveryTargetTLI" is checkpoint TL
wherever it was before calling this function or we must have the
history file.  That means after exiting this function if we execute
this line (expectedTLEs = readTimeLineHistory(recoveryTargetTLI);)
that means either "expectedTLEs" could point to one dummy entry which
will be nothing but the checkpoint TL entry or it will be holding
complete history.

The patch is trying to say that without the history file the
checkpoint TL will not be found in "expectedTLEs" because the older TL
(checkpoint TL) is not the ancestor of the target
timeline(recoveryTargetTLI).  But ideally, either the target timeline
should be the same as the checkpoint timeline or we must have the
history file as I stated in the above paragraph.  Am I missing
something?

[1]
if (rtli)
{
  /* Timeline 1 does not have a history file, all else should */
  if (rtli != 1 && !existsTimeLineHistory(rtli))
   ereport(FATAL,
  (errmsg("recovery target timeline %u does not exist",
  rtli)));
  recoveryTargetTLI = rtli;
  recoveryTargetIsLatest = false;
}
else
{
  /* We start the "latest" search from pg_control's timeline */
  recoveryTargetTLI = findNewestTimeLine(recoveryTargetTLI);
  recoveryTargetIsLatest = true;
}


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Commits

  1. Further refinement of stuck_on_old_timeline recovery test

  2. Adjust new test case to set wal_keep_segments.

  3. Adjust new test case to set wal_keep_size.

  4. Fix corner case failure of new standby to follow new primary.

  5. Back-port a few PostgresNode.pm methods.

  6. Allow PostgresNode.pm's backup method to accept backup_options.

  7. Add end-to-end testing of pg_basebackup's tar-format output.

  8. Allow PostgresNode.pm tests to wait for catchup

  9. Delay reading timeline history file until it's fetched from master.