Re: stopgap fix for signal handling during restore_command

2023-10-17 Thread Nathan Bossart
On Tue, Oct 17, 2023 at 12:47:29PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> ... and it looks like some of the back-branches are failing for Windows. >> I'm assuming this is because c290e79 was only back-patched to v15. My >> first instinct is just to back-patch that one all the way to

Re: stopgap fix for signal handling during restore_command

2023-10-17 Thread Tom Lane
Nathan Bossart writes: > ... and it looks like some of the back-branches are failing for Windows. > I'm assuming this is because c290e79 was only back-patched to v15. My > first instinct is just to back-patch that one all the way to v11, but maybe > there's an alternative involving #ifdef WIN32.

Re: stopgap fix for signal handling during restore_command

2023-10-17 Thread Nathan Bossart
On Tue, Oct 17, 2023 at 10:46:47AM -0500, Nathan Bossart wrote: > Committed and back-patched. ... and it looks like some of the back-branches are failing for Windows. I'm assuming this is because c290e79 was only back-patched to v15. My first instinct is just to back-patch that one all the way

Re: stopgap fix for signal handling during restore_command

2023-10-17 Thread Nathan Bossart
Committed and back-patched. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: stopgap fix for signal handling during restore_command

2023-10-11 Thread Nathan Bossart
On Wed, Oct 11, 2023 at 01:02:14PM +0900, Michael Paquier wrote: > On Tue, Oct 10, 2023 at 08:39:29PM -0700, Andres Freund wrote: >> We shouldn't call proc_exit() in a signal handler. We perhaps have a few >> remaining calls left, but we should (and I think in some cases are) working >> on >>

Re: stopgap fix for signal handling during restore_command

2023-10-10 Thread Michael Paquier
On Tue, Oct 10, 2023 at 08:39:29PM -0700, Andres Freund wrote: > We shouldn't call proc_exit() in a signal handler. We perhaps have a few > remaining calls left, but we should (and I think in some cases are) working on > removing those. Hmm. I don't recall anything remaining, even after a quick

Re: stopgap fix for signal handling during restore_command

2023-10-10 Thread Andres Freund
Hi, On 2023-10-10 22:29:34 -0500, Nathan Bossart wrote: > On Tue, Oct 10, 2023 at 09:54:18PM -0500, Nathan Bossart wrote: > > On Tue, Oct 10, 2023 at 04:40:28PM -0700, Andres Freund wrote: > >> I'd make these elog(PANIC), I think. The paths are not performance critical > >> enough that a single

Re: stopgap fix for signal handling during restore_command

2023-10-10 Thread Nathan Bossart
On Tue, Oct 10, 2023 at 09:54:18PM -0500, Nathan Bossart wrote: > On Tue, Oct 10, 2023 at 04:40:28PM -0700, Andres Freund wrote: >> I'd make these elog(PANIC), I think. The paths are not performance critical >> enough that a single branch hurts, so the overhead of the check is >> irrelevant, >>

Re: stopgap fix for signal handling during restore_command

2023-10-10 Thread Nathan Bossart
On Tue, Oct 10, 2023 at 04:40:28PM -0700, Andres Freund wrote: > On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: >> diff --git a/src/backend/storage/lmgr/proc.c >> b/src/backend/storage/lmgr/proc.c >> index 22b4278610..b9e2c3aafe 100644 >> --- a/src/backend/storage/lmgr/proc.c >> +++

Re: stopgap fix for signal handling during restore_command

2023-10-10 Thread Andres Freund
On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: > Subject: [PATCH v10 1/2] Move extra code out of the Pre/PostRestoreCommand() > block. LGTM > From fb6957da01f11b75d1a1966f32b00e2e77c789a0 Mon Sep 17 00:00:00 2001 > From: Nathan Bossart > Date: Tue, 14 Feb 2023 09:44:53 -0800 > Subject:

Re: stopgap fix for signal handling during restore_command

2023-10-04 Thread Nathan Bossart
On Sun, Oct 01, 2023 at 08:50:15PM +0200, Peter Eisentraut wrote: > Is this still being contemplated? What is the status of this? I'll plan on committing this in the next couple of days. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: stopgap fix for signal handling during restore_command

2023-10-01 Thread Peter Eisentraut
On 22.04.23 01:07, Nathan Bossart wrote: On Wed, Mar 01, 2023 at 03:13:04PM -0800, Andres Freund wrote: On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: Here is an attempt at adding a signal safe function for writing to STDERR. Cool. I'm gently bumping this thread to see if anyone had

Re: stopgap fix for signal handling during restore_command

2023-04-21 Thread Nathan Bossart
On Wed, Mar 01, 2023 at 03:13:04PM -0800, Andres Freund wrote: > On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: >> Here is an attempt at adding a signal safe function for writing to STDERR. > > Cool. I'm gently bumping this thread to see if anyone had additional feedback on the proposed

Re: stopgap fix for signal handling during restore_command

2023-03-01 Thread Nathan Bossart
On Wed, Mar 01, 2023 at 03:13:04PM -0800, Andres Freund wrote: > FWIW, I think we could rely on va_start() et al to be signal safe. The > standardese isn't super clear about this, because they aren't functions, and > posix only talks about functions being async signal safe... Good to know. I

Re: stopgap fix for signal handling during restore_command

2023-03-01 Thread Andres Freund
Hi, On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: > On Tue, Feb 28, 2023 at 08:36:03PM -0800, Nathan Bossart wrote: > > On Sun, Feb 26, 2023 at 12:12:27PM -0800, Andres Freund wrote: > >> Partially I just want something that can easily be searched for, that can > >> have > >> comments

Re: stopgap fix for signal handling during restore_command

2023-03-01 Thread Nathan Bossart
On Tue, Feb 28, 2023 at 08:36:03PM -0800, Nathan Bossart wrote: > On Sun, Feb 26, 2023 at 12:12:27PM -0800, Andres Freund wrote: >> Partially I just want something that can easily be searched for, that can >> have >> comments attached to it documenting why what it is doing is safe. >> >> It'd

Re: stopgap fix for signal handling during restore_command

2023-02-28 Thread Nathan Bossart
On Sun, Feb 26, 2023 at 12:12:27PM -0800, Andres Freund wrote: > On 2023-02-26 11:39:00 -0800, Nathan Bossart wrote: >> What precisely did you have in mind? AFAICT you are asking for a wrapper >> around write(). > > Partially I just want something that can easily be searched for, that can have >

Re: stopgap fix for signal handling during restore_command

2023-02-26 Thread Andres Freund
Hi, On 2023-02-26 11:39:00 -0800, Nathan Bossart wrote: > On Sun, Feb 26, 2023 at 10:00:29AM -0800, Andres Freund wrote: > > On 2023-02-25 14:06:29 -0800, Nathan Bossart wrote: > >> On Sat, Feb 25, 2023 at 11:52:53AM -0800, Andres Freund wrote: > >> > I think I opined on this before, but we

Re: stopgap fix for signal handling during restore_command

2023-02-26 Thread Nathan Bossart
On Sun, Feb 26, 2023 at 10:00:29AM -0800, Andres Freund wrote: > On 2023-02-25 14:06:29 -0800, Nathan Bossart wrote: >> On Sat, Feb 25, 2023 at 11:52:53AM -0800, Andres Freund wrote: >> > I think I opined on this before, but we really ought to have a function to >> > do >> > some minimal signal

Re: stopgap fix for signal handling during restore_command

2023-02-26 Thread Andres Freund
Hi, On 2023-02-25 14:06:29 -0800, Nathan Bossart wrote: > On Sat, Feb 25, 2023 at 11:52:53AM -0800, Andres Freund wrote: > > I think I opined on this before, but we really ought to have a function to > > do > > some minimal signal safe output. Implemented centrally, instead of being > > open >

Re: stopgap fix for signal handling during restore_command

2023-02-25 Thread Nathan Bossart
On Sat, Feb 25, 2023 at 11:52:53AM -0800, Andres Freund wrote: > I think I opined on this before, but we really ought to have a function to do > some minimal signal safe output. Implemented centrally, instead of being open > coded in a bunch of places. While looking around for the right place to

Re: stopgap fix for signal handling during restore_command

2023-02-25 Thread Andres Freund
Hi, On 2023-02-25 11:28:25 -0800, Nathan Bossart wrote: > On Sat, Feb 25, 2023 at 11:07:42AM -0800, Andres Freund wrote: > > Why do we need that rc variable? Don't we normally get away with (void) > > write(...)? > > My compiler complains about that. :/ > >

Re: stopgap fix for signal handling during restore_command

2023-02-25 Thread Nathan Bossart
On Sat, Feb 25, 2023 at 11:28:25AM -0800, Nathan Bossart wrote: > On Sat, Feb 25, 2023 at 11:07:42AM -0800, Andres Freund wrote: >> I think the much more interesting assertion here would be to check that >> MyProc->pid equals the current pid. > > I don't mind changing this, but why is this a more

Re: stopgap fix for signal handling during restore_command

2023-02-25 Thread Nathan Bossart
On Sat, Feb 25, 2023 at 11:07:42AM -0800, Andres Freund wrote: > On 2023-02-23 20:33:23 -0800, Nathan Bossart wrote:> >> if (in_restore_command) >> -proc_exit(1); >> +{ >> +/* >> + * If we are in a child process (e.g., forked by system() in >> +

Re: stopgap fix for signal handling during restore_command

2023-02-25 Thread Andres Freund
Hi, On 2023-02-23 20:33:23 -0800, Nathan Bossart wrote:> > if (in_restore_command) > - proc_exit(1); > + { > + /* > + * If we are in a child process (e.g., forked by system() in > + * RestoreArchivedFile()), we don't want to call any

Re: stopgap fix for signal handling during restore_command

2023-02-23 Thread Nathan Bossart
On Fri, Feb 24, 2023 at 01:25:01PM +1300, Thomas Munro wrote: > I think you should have a trailing \n when writing to stderr. Oops. I added that in v7. > Here's that reproducer I speculated about (sorry I confused SIGQUIT > and SIGTERM in my earlier email, ENOCOFFEE). Seems to do the job, and

Re: stopgap fix for signal handling during restore_command

2023-02-23 Thread Thomas Munro
On Fri, Feb 24, 2023 at 1:25 PM Thomas Munro wrote: > ENOCOFFEE Erm, I realised after sending that I'd accidentally sent a version that uses fork() anyway, and now if I change it back to vfork() it doesn't fail the way I wanted to demonstrate, at least on Linux. I don't have time or desire to

Re: stopgap fix for signal handling during restore_command

2023-02-23 Thread Thomas Munro
On Fri, Feb 24, 2023 at 12:15 PM Nathan Bossart wrote: > Thoughts? I think you should have a trailing \n when writing to stderr. Here's that reproducer I speculated about (sorry I confused SIGQUIT and SIGTERM in my earlier email, ENOCOFFEE). Seems to do the job, and I tested on a Linux box for

stopgap fix for signal handling during restore_command

2023-02-23 Thread Nathan Bossart
On Wed, Feb 22, 2023 at 09:48:10PM +1300, Thomas Munro wrote: > On Tue, Feb 21, 2023 at 5:50 PM Nathan Bossart > wrote: >> I'm happy to create a new thread if needed, but I can't tell if there is >> any interest in this stopgap/back-branch fix. Perhaps we should just jump >> straight to the