Hi Chris, On 2022-07-01 13:29:44 -0700, Andres Freund wrote: > On 2022-07-01 19:55:16 +0200, Alvaro Herrera wrote: > > On 2022-Jul-01, Andres Freund wrote: > > > > > On 2022-07-01 17:41:05 +0200, Alvaro Herrera wrote: > > > > Nicola Contu reported two years ago to pgsql-general[1] that they were > > > > having sporadic query failures, because EINTR is reported on some system > > > > call. I have been told that the problem persists, though it is very > > > > infrequent. I propose the attached patch. Kyotaro proposed a slightly > > > > different patch which also protects write(), but I think that's not > > > > necessary. > > > > > > What is the reason for the || ProcDiePending || QueryCancelPending bit? > > > What > > > if there's dsm operations intentionally done while QueryCancelPending? > > > > That mirrors the test for the other block in that function, which was > > added by 63efab4ca139, whose commit message explains: > > > > Allow DSM allocation to be interrupted. > > > > Chris Travers reported that the startup process can repeatedly try to > > cancel a backend that is in a posix_fallocate()/EINTR loop and cause it > > to loop forever. Teach the retry loop to give up if an interrupt is > > pending. Don't actually check for interrupts in that loop though, > > because a non-local exit would skip some clean-up code in the caller. > > That whole approach seems quite wrong to me. At the absolute very least the > code needs to check if interrupts are being processed in the current context > before just giving up due to ProcDiePending || QueryCancelPending. > > I'm very unconvinced this ought to be fixed in dsm_impl_posix_resize(), rather > than the startup process signalling. > > There is an argument for allowing more things to be cancelled, but we'd need a > retry loop for the !INTERRUPTS_CAN_BE_PROCESSED() case.
Chris, do you have any additional details about the machine that lead to this change? OS version, whether it might have been swapping, etc? I wonder if what happened is that posix_fallocate() used glibc's fallback implementation because the kernel was old enough to not support fallocate() for tmpfs. Looks like support for fallocate() for tmpfs was added in 3.5 ([1]). So e.g. a rhel 6 wouldn't have had that. Greetings, Andres Freund [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e2d12e22c59ce714008aa5266d769f8568d74eac