Re: [Patch] ALTER SYSTEM READ ONLY

On Wed, Jun 17, 2020 at 9:02 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.)

> > 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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company