Re: Get memory contexts of an arbitrary backend process

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: masao(dot)fujii(at)oss(dot)nttdata(dot)com
Cc: torikoshia(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us, gkokolatos(at)protonmail(dot)com, kasahara(dot)tatsuhito(at)gmail(dot)com, craig(at)2ndquadrant(dot)com
Subject: Re: Get memory contexts of an arbitrary backend process
Date: 2021-03-26 03:53:58
Message-ID: 20210326.125358.504245283793809216.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 26 Mar 2021 12:26:31 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>
>
> On 2021/03/26 12:01, Kyotaro Horiguchi wrote:
> > At Thu, 25 Mar 2021 23:45:17 +0900, torikoshia
> > <torikoshia(at)oss(dot)nttdata(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2021-03-26 03:55:11 Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation
Previous Message Fujii Masao 2021-03-26 03:49:16 Re: Get memory contexts of an arbitrary backend process