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 04:17:21
Message-ID: 20210326.131721.583838520933682740.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 26 Mar 2021 12:49:16 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
> On 2021/03/26 12:26, Fujii Masao wrote:
> > 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?
>
> + mcxtLogData->maxChildrenPerContext = max_children;
> +
> + if(!SendProcSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT,
> proc->backendId))
> + PG_RETURN_BOOL(true);
>
> Do we need memory barrier here? Otherwise, there is a race condition
> where maxChildrenPerContext has not been set yet when the target
> backend
> that received signal read that shared variable. No?
>
> + Note that when multiple
> + <function>pg_get_backend_memory_contexts</function> called in
> + succession or simultaneously, <parameter>max_children</parameter>
> can
> + be the one of another
> + <function>pg_get_backend_memory_contexts</function>.
> + </para></entry>
>
> This might not be an issue in practice as Horiguchi-san said upthread.
> But this looks a bit ugly and might be footgun later. The current
> approach
> using shared memory to pass this information to the target backends
> might be overkill, and too complicated to polish the design at the
> stage
> just before feature freeze. So I'd withdraw my previous comment and
> am OK to use the hard-coded value as max_children at the first version
> of this feature. Thought?

The dumper function silently suppresses logging when there are too
many children. Suppressing a part of output when the user didn't told
anything looks like just a misbehavior (even if it is written in the
documentation..), especially when the suppressed contexts occupy the
majority of memory usage. I think the fixed-limit of lines works if
the lines are in descending order of memory usage.

At least we need an additional line to inform the suppression.

"some contexts are omitted"
"n child contexts: total_bytes = ..."

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2021-03-26 04:28:36 Re: Get memory contexts of an arbitrary backend process
Previous Message Thomas Munro 2021-03-26 04:14:44 Re: increase size of pg_commit_ts buffers