At Fri, 26 Mar 2021 12:26:31 +0900, Fujii Masao <masao.fu...@oss.nttdata.com>
wrote in
>
>
> On 2021/03/26 12:01, Kyotaro Horiguchi wrote:
> > At Thu, 25 Mar 2021 23:45:17 +0900, torikoshia
> > <torikos...@oss.nttdata.com> wrote in
> >> Attached new one.
>
> Regarding the argument max_children, isn't it better to set its
> default value,
> e.g., 100 like MemoryContextStats() uses?
> + ereport(WARNING,
> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("must be a superuser to log memory contexts")));
>
> IMO this type of error, i.e., permission error, should be treated as
> ERROR
> rather than WARNING, like pg_terminate_backend() does.
> + ereport(WARNING,
> + (errmsg("failed to send signal: %m")));
>
> Per message style rule, "could not send signal to process %d: %m" is
> better?
+1 x 3 for the above.
> > Thanks!
> > - SELECT * FROM pg_get_backend_memory_contexts();
> > + SELECT * FROM pg_get_backend_memory_contexts(0, 0);
> > However we can freely change the signature since it's new in 14, but I
> > see the (void) signature as useful. I vaguely thought of defining
> > pg_get_backend_memory_contexts(void) in pg_proc.dat then defining
> > (int, int) as a different function in system_views.sql. That allows
> > the function of the second signature to output nothing. You can make a
> > distinction between the two signatures by using PG_NARGS() in the C
> > function.
> > That being said, it's somewhat uneasy that two functions with the same
> > name returns different values. I'd like to hear others what they feel
> > like about the behavior.
>
> I think that it's confusing to merge two different features into one
> function.
> Isn't it better to leave pg_get_backend_memory_contexts() as it is,
> and
> make the log-memory-contexts feature as separate function, e.g.,
> pg_log_backend_memory_contexts()?
Yeah, that name looks better than pg_print_ba.. and I agree to
separate the two features. Sorry for wagging the discussion
back-and-forth.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
