Re: [Patch] ALTER SYSTEM READ ONLY

On Tue, May 11, 2021 at 6:56 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> On Tue, May 11, 2021 at 6:48 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Tue, May 11, 2021 at 4:50 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> > >
> > > On Tue, May 11, 2021 at 4:13 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > > I might be missing something, but assume the behavior should be like this
> > > >
> > > > 1. If the state is getting changed from WALPROHIBIT_STATE_READ_WRITE
> > > > -> WALPROHIBIT_STATE_READ_ONLY, then as soon as the backend process
> > > > the barrier, we can immediately abort any read-write transaction(and
> > > > stop allowing WAL writing), because once we ensure that all session
> > > > has responded that now they have no read-write transaction then we can
> > > > safely change the state from WALPROHIBIT_STATE_GOING_READ_ONLY to
> > > > WALPROHIBIT_STATE_READ_ONLY.
> > > >
> > >
> > > Yes, that's what the current patch is doing from the first patch version.
> > >
> > > > 2. OTOH, if we are changing from WALPROHIBIT_STATE_READ_ONLY ->
> > > > WALPROHIBIT_STATE_READ_WRITE, then we don't need to allow the backend
> > > > to consider the system as read-write, instead, we should wait until
> > > > the shared state is changed to WALPROHIBIT_STATE_READ_WRITE.
> > > >
> > >
> > > I am sure that only not enough will have the same issue where
> > > LocalXLogInsertAllowed gets set the same as the read-only as described in
> > > my previous reply.
> >
> > Okay, but while browsing the code I do not see any direct if condition
> > based on the "LocalXLogInsertAllowed" variable, can you point me to
> > some references?
> > I only see one if check on this variable and that is in
> > XLogInsertAllowed() function, but now in XLogInsertAllowed() function,
> > you are already checking IsWALProhibited. No?
> >
>
> I am not sure I understood this. Where am I checking IsWALProhibited()?
>
> IsWALProhibited() is called by XLogInsertAllowed() once when
> LocalXLogInsertAllowed is in a reset state, and that result will be
> cached in LocalXLogInsertAllowed and will be used in the subsequent
> XLogInsertAllowed() call.

Okay, got what you were trying to say. But that can be easily
fixable, I mean if the state is WALPROHIBIT_STATE_GOING_READ_WRITE
then what we can do is don't allow to write the WAL but let's not set
the LocalXLogInsertAllowed to 0. So until we are in the intermediate
state WALPROHIBIT_STATE_GOING_READ_WRITE, we will always have to rely
on GetWALProhibitState(), I know this will add a performance penalty
but this is for the short period until we are in the intermediate
state. After that as soon as it will set to
WALPROHIBIT_STATE_READ_WRITE then the XLogInsertAllowed() will set
LocalXLogInsertAllowed to 1.

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