At Fri, 9 Apr 2021 10:59:44 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Fri, Apr 09, 2021 at 06:53:21AM +0530, Bharath Rupireddy wrote:
> > I didn't think of the warning cases, my bad. How about using SET
> > client_min_messages = 'ERROR'; before we call
> > pg_wait_for_backend_termination? We can only depend on the return
> > value of pg_wait_for_backend_termination, when true we can exit. This
> > way the buildfarm will not see warnings. Thoughts?
>
> You could do that, but I would also bet that this is going to get
> forgotten in the future if this gets extended in more SQL tests that
> are output-sensitive, in or out of core. Honestly, I can get behind a
> warning in pg_wait_for_backend_termination() to inform that the
> process poked at is not a PostgreSQL one, because it offers new and
> useful information to the user. But, and my apologies for sounding a
> bit noisy, I really don't get why pg_wait_until_termination() has any
> need to do that. From what I can see, it provides the following
> information:
> - A PID, that we already know from the caller or just from
> pg_stat_activity.
> - A timeout, already known as well.
> - The fact that the process did not terminate, information given by
> the "false" status, only used in this case.
>
> So there is no new information here to the user, only a duplicate of
> what's already known to the caller of this function. I see more
> advantages in removing this WARNING rather than keeping it.
FWIW I agree to Michael. I faintly remember that I thought the same
while reviewing but it seems that I forgot to write a comment like
that. It's a work of the caller, concretely the existing callers and
any possible script that calls the function.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center