On Wed, Nov 13, 2019 at 2:45 PM Andres Freund <and...@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
trivial-av-conversion-cfi.patch
Description: Binary data