Re: BackgroundPsql swallowing errors on windows

Hi,

On 2025-02-16 09:39:43 -0800, Noah Misch wrote:
> On Thu, Feb 13, 2025 at 12:39:04PM -0500, Andres Freund wrote:
> > I suspect what's happening is that the communication with the
> > external process allows for reordering between stdout/stderr.
> >
> > And indeed, changing BackgroundPsql::query() to emit the banner on both stdout
> > and stderr and waiting on both seems to fix the issue.
>
> That makes sense. I wondered how one might fix IPC::Run to preserve the
> relative timing of stdout and stderr, not perturbing the timing the way that
> disrupted your test run. I can think of two strategies:
>
> - Remove the proxy.
>
> - Make pipe data visible to Perl variables only when at least one of the
> proxy<-program_under_test pipes had no data ready to read. In other words,
> if both pipes have data ready, make all that data visible to Perl code
> simultaneously. (When both the stdout pipe and the stderr pipe have data
> ready, one can't determine data arrival order.)
>
> Is there a possibly-less-invasive change that might work?

I don't really know enough about IPC::Run's internals to answer. My
interpretation of how it might work, purely from observation, is that it opens
one tcp connection for each "pipe" and that that's what's introducing the
potential of reordering, as the different sockets can have different delivery
timeframes. If that's it, it seems proxying all the pipes through one
connection might be an option.

I did briefly look at IPC::Run's code, but couldn't figure out how it all fits
together quickly enough...

> v2 of the patch looks fine.

Thanks for reviewing!

Greetings,

Andres Freund