Horton

Re: Weird failure with latches in curculio on v15

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>, Fujii Masao <fujii@postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>, Michael Paquier <michael@paquier.xyz>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-01T17:20:16Z
Lists: pgsql-hackers 
Hi,

On 2023-02-01 12:08:24 -0500, Robert Haas wrote:
> On Wed, Feb 1, 2023 at 11:58 AM Andres Freund <andres@anarazel.de> wrote:
> > > 9a740f81e clearly made things a lot worse, but it wasn't great
> > > before.  Can we see a way forward to removing the problem entirely?
> >
> > Yea, I think we can - we should stop relying on system(). If we instead
> > run the command properly as a subprocess, we don't need to do bad things
> > in the signal handler anymore.
> 
> I like the idea of not relying on system(). In most respects, doing
> fork() + exec() ourselves seems superior. We can control where the
> output goes, what we do while waiting, etc. But system() runs the
> command through the shell, so that for example you don't have to
> invent your own way of splitting a string into words to be passed to
> exec[whatever](). I've never understood how you're supposed to get
> that behavior other than by calling system().

We could just exec the shell in the forked process, using -c to invoke
the command. That should give us pretty much the same efficiency as
system(), with a lot more control.

I think we already do that somewhere. <dig>. Ah, yes, spawn_process() in
pg_regress.c.  I suspect we couldn't use exec for restore_command etc,
as I think it's not uncommon to use && in the command.


Perhaps we should abstract the relevant pieces of spawn_process() that
into something more general? The OS specifics are sufficiently
complicated that I don't think it'd be good to have multiple copies.


It's too bad that we have the history of passing things to shell,
otherwise we could define a common argument handling of the GUC and just
execve ourselves, but that ship has sailed.

Greetings,

Andres Freund

Commits

  1. Avoid calling proc_exit() in processes forked by system().

  2. Move extra code out of the Pre/PostRestoreCommand() section.

  3. Revert refactoring of restore command code to shell_restore.c

  4. Refactor code in charge of running shell-based recovery commands

  5. Clean up inconsistent use of fflush().

  6. Report wait events for local shell commands like archive_command.