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: Nathan Bossart <nathandbossart@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>, Fujii Masao <fujii@postgresql.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-05T16:08:38Z
Lists: pgsql-hackers 
Hi,

On 2023-02-04 10:03:54 -0800, Nathan Bossart wrote:
> On Sat, Feb 04, 2023 at 03:30:29AM -0800, Andres Freund wrote:
> > That's kind of my problem with these changes. They try to introduce new
> > abstraction layers, but don't provide real abstraction, because they're
> > very tightly bound to the way the functions were called before the
> > refactoring.  And none of these restrictions are actually documented.
> 
> Okay.  Michael, why don't we revert the shell_restore stuff for now?  Once
> the archive modules interface changes and the fix for this
> SIGTERM-during-system() problem are in, I will work through this feedback
> and give recovery modules another try.  I'm still hoping to have recovery
> modules ready in time for the v16 feature freeze.
> 
> My intent was to improve this code by refactoring and reducing code
> duplication, but I seem to have missed the mark.  I am sorry.

FWIW, I think the patches were going roughly the right direction, they
just needs a bit more work.

I don't think we should expose the proc_exit() hack, and its supporting
infrastructure, to the pluggable *_command logic. It's bad enough as-is,
but having to do this stuff within an extension module seems likely to
end badly. There's just way too much action-at-a-distance.

I think Thomas has been hacking on a interruptible system()
replacement. With that, a lot of this ugliness would be resolved.

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.