Re: global / super barriers (for checksums)

On Wed, Nov 13, 2019 at 2:45 PM Andres Freund <andres@anarazel.de> wrote:
> I might be missing something. Aren't all of the places where those
> checks are places where we currently can't do a CHECK_FOR_INTERRUPTS()?
> I've swapped this thoroughly out of my mind, but going through them:
>
> 1) AutoVacLauncherMain() - doesn't do CFI()
> 2) BackgroundWriterMain() - dito
> 3) CheckpointerMain() - dito
> 4) HandleStartupProcInterrupts() - dito
> 5) WalWriterMain() - dito
> 6) BufferSync() - dito, called from CheckpointerMain(), and startup process
> 7) ProcessClientWriteInterrupt() - can't do generic CFI, don't want to
>    process all interrupts while writing out data, to avoid corrupting
>    the output stream or loosing messages
>
> Which one do you think we should convert to CFI()? As far as I can tell
> we can't make the non-backend cases use the postgres.c
> ProcessInterrupts(), and the ProcessClientWriteInterrupt() one can't do
> so either.

I haven't looked through all of these, but in AutoVacLauncherMain, a
trivial conversion to CFI doesn't seem to break anything horribly (see
attached). It does change the error message when we exit, making it
chattier, but I think we could find our way around that problem. Note
that AutoVacLauncherMain, and some of the others, do a
HOLD_INTERRUPTS() and a RESUME_INTERRUPTS() in the error-recovery
block, so somebody evidently thought some of that code might call
CHECK_FOR_INTERRUPTS(), and I can't prove off-hand that none of the
other logic which isn't so protected doesn't have some way to reach
CHECK_FOR_INTERRUPTS(). It seems to me that there are so many places
where PostgreSQL calls CHECK_FOR_INTERRUPTS() that it's somewhat
unwise to assume that just "can't happen."

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