On 31/07/2021 10:54, Heikki Linnakangas wrote:
> On 31/07/2021 02:11, Andres Freund wrote:
>>> @@ -5592,111 +5591,6 @@ validateRecoveryParameters(void)
>>> }
>>> }
>>>
>>> -/*
>>> - * Exit archive-recovery state
>>> - */
>>> -static void
>>> -exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
>>> -{
>>
>> I don't really understand the motivation for this part of the change? This
>> kind of seems to run counter to the stated goals of the patch series? Seems
>> like it'd need a different commit message at last?
>
> Hmm. Some parts of exitArchiveRecovery are being moved into
> xlogrecovery.c, so it becomes smaller than before. Maybe there's still
> enough code left there that a separate function makes sense. I'll try
> that differently.
So, my issue with exitArchiveRecovery() was that after this refactoring,
the function didn't really exit archive recovery anymore.
InArchiveRecovery flag is cleared earlier already, in xlogrecovery.c. I
renamed exitArchiveRecovery() to XLogInitNewTimeline(), and moved the
unlinking of the signal files into the caller. The function now only
initializes the first WAL segment on the new timeline, and the new name
reflects that. I'm pretty happy with this now.
>>> + /*---- BEGIN FreeWalRecovery ----*/
>>> +
>>> /* Shut down xlogreader */
>>> if (readFile >= 0)
>>> {
>>
>> FWIW, FreeWalRecovery() for something that closes and unlinks files among
>> other things doesn't seem like a great name.
>
> Rename to CloseWalRecovery(), maybe? I'll try that.
I renamed it to ShutdownWalRecovery(). I also refactored the
FinishWalRecovery() function so that instead of having a dozen output
pointer parameters, it returns a struct with all the return values. New
patch set attached.
- Heikki