Re: [Patch] ALTER SYSTEM READ ONLY

On Fri, Sep 24, 2021 at 5:07 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> On Thu, Sep 23, 2021 at 11:56 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > On Mon, Sep 20, 2021 at 11:20 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> > > Ok, understood, I have separated my changes into 0001 and 0002 patch,
> > > and the refactoring patches start from 0003.
> >
> > I think it would be better in the other order, with the refactoring
> > patches at the beginning of the series.
> >
>
> Ok, will do that. I did this other way to minimize the diff e.g.
> deletion diff of RecoveryXlogAction enum and
> DetermineRecoveryXlogAction(), etc.
>

I have reversed the patch order. Now refactoring patches will be
first, and the patch that removes the dependencies on global & local
variables will be the last. I did the necessary modification in the
refactoring patches too e.g. removed DetermineRecoveryXlogAction() and
RecoveryXlogAction enum which is no longer needed (thanks to commit #
1d919de5eb3fffa7cc9479ed6d2915fb89794459 to make code simple).

To find the value of InRecovery after we clear it, patch still uses
ControlFile's DBState, but now the check condition changed to a more
specific one which is less confusing.

In casual off-list discussion, the point was made to check
SharedRecoveryState to find out the InRecovery value afterward, and
check that using RecoveryInProgress(). But we can't depend on
SharedRecoveryState because at the start it gets initialized to
RECOVERY_STATE_CRASH irrespective of InRecovery that happens later.
Therefore, we can't use RecoveryInProgress() which always returns
true if SharedRecoveryState != RECOVERY_STATE_DONE.

I am posting only refactoring patches for now.

Regards,
Amul