> On 4 Apr 2024, at 23:24, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> A minimum fix that seems to make this work better is as attached,
> but I feel like somebody ought to examine all the IPC::Run::timer
> and IPC::Run::timeout calls and see which ones are mistaken.
> It's a little scary to convert a timeout to a timer because of
> the hazard that someplace that would need to be checking for
> is_expired isn't.
Skimming this and a few callsites it seems reasonable to use a timer instead of
a timeout, but careful study is needed to make sure we're not introducing
anything subtly wrong in the other direction.
> Also, the debug printout code at the bottom of check_completion
> is quite useless, because control can never reach it since
> BackgroundPsql::query_until will "die" on failure. I think that
> that code worked when written, and I'm suspicious that 664d75753
> broke it, but I've not dug deeply into the history.
AFAICT, in the previous coding the interactive_psql object would use a timer or
timeout based on what the caller provided, and check_completion used a timer so
the debug logging probably worked as written.
--
Daniel Gustafsson