Re: [Patch] ALTER SYSTEM READ ONLY

On Tue, May 11, 2021 at 11:17 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> I think I have much easier solution than this, will post that with update version patch set tomorrow.

I don't know what you have in mind, but based on this discussion, it
seems to me that we should just have 5 states instead of 4:

1. WAL is permitted.
2. WAL is being prohibited but some backends may not know about the change yet.
3. WAL is prohibited.
4. WAL is in the process of being permitted but XLogAcceptWrites() may
not have been called yet.
5. WAL is in the process of being permitted and XLogAcceptWrites() has
been called but some backends may not know about the change yet.

If we're in state #3 and someone does pg_prohibit_wal(false) then we
enter state #4. The checkpointer calls XLogAcceptWrites(), moves us to
state #5, and pushes out a barrier. Then it waits for the barrier to
be absorbed and, when it has been, it moves us to state #1. Then if
someone does pg_prohibit_wal(true) we move to state #2. The
checkpointer pushes out a barrier and waits for it to be absorbed.
Then it calls XLogFlush() and afterward moves us to state #3.

We can have any (reasonable) number of states that we want. There's
nothing magical about 4.

I also entirely agree with Dilip that we should do some renaming to
get rid of the read-write/read-only terminology, now that this is no
longer part of the syntax. In fact I made the exact same point in my
last review. The WALPROHIBIT_STATE_* constants are just one thing of
many that needs to be included in that renaming.

--
Robert Haas
EDB: http://www.enterprisedb.com