On Tue, 11 Aug 2026 at 19:49, surya poondla <[email protected]> wrote:
> To make this forward-looking, a better direction would be a narrowly defined
> process-spawning abstraction, using posix_spawn() where appropriate or a 
> minimal
> fork/exec implementation.  The child-side path should only perform the
> operations strictly required before exec(), such as descriptor setup, signal
> setup, and resource-limit adjustment, while allocation, logging, error 
> handling,
> and other PostgreSQL work remain in the parent.

I agree with this. But I feel like that's exactly what this patchset
is currently doing. The newly introduced fork_for_shell_command makes
sure almost nothing is happening on the child-side of the fork. I
don't understand how the thing you describe differs from my currently
proposed implementation. Could you clarify?

> A few things from reading the patch:
> 1. Is there any supported way for a session to affect the soft limit after
> startup, such as calling setrlimit() from a PL/pgSQL (or other procedural
> language) function?  If the answer is "no", it would be good to state that
> assumption explicitly.

Nothing is explicitly exposed to make it easy to do so, but changing
the soft limit after startup should not cause any problems (except
that if you lower it you can open fewer files ofcourse).

> 2. In IncreaseOpenFileLimit(), rlim_cur + extra_files would overflow if 
> rlim_cur
> were RLIM_INFINITY, and Min() would then pick the wrapped value and lower the
> limit drastically.  That is unreachable today, but only via the earlier
> "rlim_cur == original_max_open_files.rlim_max" test combined with the
> rlim_cur <= rlim_max guarantee.  A comment or an explicit RLIM_INFINITY check
> would make it less fragile.

Hmm, yeah, checking for overflows there is probably a good idea.

> 3. Nit: SaveOriginalOpenFileLimit() returns void, but its comment says 
> "Returns
> true if successful, false otherwise."

Oh yeah, I guess that's a leftover from an earlier version.


Reply via email to