Re: Race condition in recovery?

On Thu, May 20, 2021 at 11:19 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, May 18, 2021 at 1:33 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > Yeah, it will be a fake 1-element list. But just to be clear that
> > 1-element can only be "ControlFile->checkPointCopy.ThisTimeLineID" and
> > nothing else, do you agree to this? Because we initialize
> > recoveryTargetTLI to this value and we might change it in
> > readRecoveryCommandFile() but for that, we must get the history file,
> > so if we are talking about the case where we don't have the history
> > file then "recoveryTargetTLI" will still be
> > "ControlFile->checkPointCopy.ThisTimeLineID".
>
> I don't think your conclusion is correct. As I understand it, you're
> talking about the state before
> ee994272ca50f70b53074f0febaec97e28f83c4e,

Right, I am talking about before this commit.

because as of now
> readRecoveryCommandFile() no longer exists in master. Before that
> commit, StartupXLOG did this:
>
> recoveryTargetTLI = ControlFile->checkPointCopy.ThisTimeLineID;
> readRecoveryCommandFile();
> expectedTLEs = readTimeLineHistory(recoveryTargetTLI);
>
> Now, readRecoveryCommandFile() can change recoveryTargetTLI. Before
> doing so, it will call existsTimeLineHistory if
> recovery_target_timeline was set to an integer, and findNewestTimeLine
> if it was set to latest. In the first case, existsTimeLineHistory()
> calls RestoreArchivedFile(), but that restores it using a temporary
> name, and KeepFileRestoredFromArchive is not called,

Yes, I agree with this.

so we might have
> the timeline history in RECOVERYHISTORY but that's not the filename
> we're actually going to try to read from inside readTimeLineHistory().
> In the second case, findNewestTimeLine() will call
> existsTimeLineHistory() which results in the same situation. So I
> think when readRecoveryCommandFile() returns expectedTLI can be
> different but the history file can be absent since it was only ever
> restored under a temporary name.

I agree that readTimeLineHistory() will not look for that filename,
but it will also try to get the file using (RestoreArchivedFile(path,
histfname, "RECOVERYHISTORY", 0, false)). So after we check the
history file existence in existsTimeLineHistory(), if the file got
removed from the archive (not sure how) then it is possible that now
readTimeLineHistory() will not find that history file again. Am I
missing something?

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