Re: [Patch] ALTER SYSTEM READ ONLY

On Fri, Sep 10, 2021 at 12:20 PM Mark Dilger
<mark(dot)dilger(at)enterprisedb(dot)com> wrote:
> A better example may be found in ginmetapage.c:
>
> needwal = RelationNeedsWAL(indexrel);
> if (needwal)
> {
> CheckWALPermitted();
> computeLeafRecompressWALData(leaf);
> }
>
> /* Apply changes to page */
> START_CRIT_SECTION();

Yeah, that looks sketchy. Why not move CheckWALPermitted() down a line?

> Even if CheckWALPermitted is assumed to be close enough to atomic to not be a problem (I don't agree), that argument can't be made here, as computeLeafRecompressWALData is not trivial and signals could easily be processed while it is running.

I think the relevant question here is not "could a signal handler
fire?" but "can we hit a CHECK_FOR_INTERRUPTS()?". If the relevant
question is the former, then there's no hope of ever making it work
because there's always a race condition. But the signal handler is
only setting flags whose only effect is to make a subsequent
CHECK_FOR_INTERRUPTS() do something, so it doesn't really matter when
the signal handler can run, but when CHECK_FOR_INTERRUPTS() can call
ProcessInterrupts().

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