On Tue, 11 May 2021 at 7:50 PM, Dilip Kumar <dilipbal...@gmail.com> wrote:
> On Tue, May 11, 2021 at 6:56 PM Amul Sul <sula...@gmail.com> wrote: > > > > On Tue, May 11, 2021 at 6:48 PM Dilip Kumar <dilipbal...@gmail.com> > wrote: > > > > > > On Tue, May 11, 2021 at 4:50 PM Amul Sul <sula...@gmail.com> wrote: > > > > > > > > On Tue, May 11, 2021 at 4:13 PM Dilip Kumar <dilipbal...@gmail.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. I think I have much easier solution than this, will post that with update version patch set tomorrow. Regards, Amul