Re: [Patch] ALTER SYSTEM READ ONLY

Hello,

I think we should really term this feature, as it stands, as a means to
solely stop WAL writes from happening.

The feature doesn't truly make the system read-only (e.g. dirty buffer
flushes may succeed the system being put into a read-only state), which
does make it confusing to a degree.

Ideally, if we were to have a read-only system, we should be able to run
pg_checksums on it, or take file-system snapshots etc, without the need
to shut down the cluster. It would also enable an interesting use case:
we should also be able to do a live upgrade on any running cluster and
entertain read-only queries at the same time, given that all the
cluster's files will be immutable?

So if we are not going to address those cases, we should change the
syntax and remove the notion of read-only. It could be:

ALTER SYSTEM SET wal_writes TO off|on;
or
ALTER SYSTEM SET prohibit_wal TO off|on;

If we are going to try to make it truly read-only, and cater to the
other use cases, we have to:

Perform a checkpoint before declaring the system read-only (i.e. before
the command returns). This may be expensive of course, as Andres has
pointed out in this thread, but it is a price that has to be paid. If we
do this checkpoint, then we can avoid an additional shutdown checkpoint
and an end-of-recovery checkpoint (if we restart the primary after a
crash while in read-only mode). Also, we would have to prevent any
operation that touches control files, which I am not sure we do today in
the current patch.

Why not have the best of both worlds? Consider:

ALTER SYSTEM SET read_only to {off, on, wal};

-- on: wal writes off + no writes to disk
-- off: default
-- wal: only wal writes off

Of course, there can probably be better syntax for the above.

Regards,

Soumyadeep (VMware)