Re: [Patch] ALTER SYSTEM READ ONLY

On Wed, Jun 17, 2020 at 8:12 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 9:02 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > Do we prohibit the checkpointer to write dirty pages and write a
> > checkpoint record as well?  If so, will the checkpointer process
> > writes the current dirty pages and writes a checkpoint record or we
> > skip that as well?
>
> I think the definition of this feature should be that you can't write
> WAL. So, it's OK to write dirty pages in general, for example to allow
> for buffer replacement so we can continue to run read-only queries.
> But there's no reason for the checkpointer to do it: it shouldn't try
> to checkpoint, and therefore it shouldn't write dirty pages either.
> (I'm not sure if this is how the patch currently works; I'm describing
> how I think it should work.)
>
You are correct -- writing dirty pages is not restricted.

> > > If there are open transactions that have acquired an XID, the sessions are killed
> > > before the barrier is absorbed.
> >
> > What about prepared transactions?
>
> They don't matter. The problem with a running transaction that has an
> XID is that somebody might end the session, and then we'd have to
> write either a commit record or an abort record. But a prepared
> transaction doesn't have that problem. You can't COMMIT PREPARED or
> ROLLBACK PREPARED while the system is read-only, as I suppose anybody
> would expect, but their mere existence isn't a problem.
>
> > What if vacuum is on an unlogged relation?  Do we allow writes via
> > vacuum to unlogged relation?
>
> Interesting question. I was thinking that we should probably teach the
> autovacuum launcher to stop launching workers while the system is in a
> READ ONLY state, but what about existing workers? Anything that
> generates invalidation messages, acquires an XID, or writes WAL has to
> be blocked in a read-only state; but I'm not sure to what extent the
> first two of those things would be a problem for vacuuming an unlogged
> table. I think you couldn't truncate it, at least, because that
> acquires an XID.
>
> > > Another part of the patch that quite uneasy and need a discussion is that when the
> > > shutdown in the read-only state we do skip shutdown checkpoint and at a restart, first
> > > startup recovery will be performed and latter the read-only state will be restored to
> > > prohibit further WAL write irrespective of recovery checkpoint succeed or not. The
> > > concern is here if this startup recovery checkpoint wasn't ok, then it will never happen
> > > even if it's later put back into read-write mode.
> >
> > I am not able to understand this problem.  What do you mean by
> > "recovery checkpoint succeed or not", do you add a try..catch and skip
> > any error while performing recovery checkpoint?
>
> What I think should happen is that the end-of-recovery checkpoint
> should be skipped, and then if the system is put back into read-write
> mode later we should do it then. But I think right now the patch
> performs the end-of-recovery checkpoint before restoring the read-only
> state, which seems 100% wrong to me.
>
Yeah, we need more thought on how to proceed further.  I am kind of agree that
the current behavior is not right with Robert since writing end-of-recovery
checkpoint violates the no-wal-write rule.

Regards,
Amul