At Fri, 26 Mar 2021 13:17:21 +0900 (JST), Kyotaro Horiguchi <horikyota....@gmail.com> wrote in > At Fri, 26 Mar 2021 12:49:16 +0900, Fujii Masao <masao.fu...@oss.nttdata.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 > > >> <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? > > > > + 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 = ..." Sorry I missed that is already implemented. So my opnion is I agree with limiting with a fixed-number, and preferablly sorted in descending order of... totalspace/nblocks? regards. -- Kyotaro Horiguchi NTT Open Source Software Center
