At Fri, 9 Apr 2021 10:59:44 +0900, Michael Paquier <mich...@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
