Hi,
On 2025-03-06 22:49:20 +0200, Heikki Linnakangas wrote:
> In short, all the 4 patches look good to me. Thanks for picking this up!
>
> On 06/03/2025 22:16, Andres Freund wrote:
> > On 2025-03-05 20:49:33 -0800, Noah Misch wrote:
> > > > This behaviour makes it really hard to debug problems. It'd have been a lot
> > > > easier to understand the problem if we'd seen psql's stderr before the test
> > > > died.
> > > >
> > > > I guess that mean at the very least we'd need to put an eval {} around the
> > > > ->pump() call., print $self->{stdout}, ->{stderr} and reraise an error?
> > >
> > > That sounds right.
> >
> > In the attached patch I did that for wait_connect(). I did verify that it
> > works by implementing the wait_connect() fix before fixing
> > 002_connection_limits.pl, which fails if a sleep(1) is added just before the
> > proc_exit(1) for FATAL.
>
> +1. For the archives sake, I just want to clarify that this pump stuff is
> all about getting better error messages on a test failure. It doesn't help
> with the original issue.
Agreed.
> This is all annoyingly complicated, but getting good error messages is worth
> it.
Yea. I really look forward to having a way to write stuff like this that
doesn't involve hackily driving psql from 100m away using rubber bands.
> > On 2025-03-05 08:23:32 +0900, Michael Paquier wrote:>> Why not adding an
> > injection point with a WARNING or a LOG generated,
> then
> > > check the server logs for the code path taken based on the elog() generated
> > > with the point name?
> >
> > I think the log_min_messages approach is a lot simpler. If we need something
> > like this more widely we can reconsider injection points...
>
> +1. It's a little annoying to depend on a detail like the "client backend
> process exited" debug message, but seems like the best fix for now.
We use the same message for LOG messages too, for other types of backends, so
I think it's not that likely to change. But stilll not great.
> While we're at it, attached are a few more cleanups I noticed.
I assume you'll apply that yourself?
Commits with updated commit messages attached.
I wonder if we should apply the polishing of connect_ok()/connect_fails() and
the wait_connect() debuggability improvements to the backbranches? Keeping TAP
infrastructure as similar as possible between branches has proven worthwhile
IME.
Greetings,
Andres Freund
