On Wed, Sep 15, 2021 at 6:49 AM Amul Sul <sula...@gmail.com> wrote: > Initially, I thought to > use SharedRecoveryState which is always set to RECOVERY_STATE_ARCHIVE, > if the archive recovery requested. But there is another case where > SharedRecoveryState could be RECOVERY_STATE_ARCHIVE irrespective of > ArchiveRecoveryRequested value, that is the presence of a backup label > file.
Right, there's a difference between whether archive recovery has been *requested* and whether it is actually *happening*. > If we want to use SharedRecoveryState, we need one more state > which could differentiate between ArchiveRecoveryRequested and the > backup label file presence case. To move ahead, I have copied > ArchiveRecoveryRequested into shared memory and it will be cleared > once archive cleanup is finished. With all these changes, we could get > rid of xlogaction argument and DetermineRecoveryXlogAction() function. > Could move its logic to PerformRecoveryXLogAction() directly. Putting these changes into 0001 seems to make no sense. It seems like they should be part of 0003, or maybe a new 0004 patch. > And for ThisTimeLineID, I don't think we need to do anything since this > value is available with all the backend as per the following comment: > " > /* > * ThisTimeLineID will be same in all backends --- it identifies current > * WAL timeline for the database system. > */ > TimeLineID ThisTimeLineID = 0; I'm not sure I find that argument totally convincing. The two variables aren't assigned at exactly the same places in the code, nonwithstanding the comment. I'm not saying you're wrong. I'm just saying I don't believe it just because the comment says so. -- Robert Haas EDB: http://www.enterprisedb.com
