Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: bharath(dot)rupireddyforpostgres(at)gmail(dot)com
Cc: torikoshia(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes
Date: 2021-11-01 01:12:07
Message-ID: 20211101.101207.2030923294047951899.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 29 Oct 2021 22:25:04 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
> On Mon, Oct 11, 2021 at 9:55 AM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > On Mon, Oct 11, 2021 at 8:21 AM torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
> > > If we can use debuggers, it's possible to know the memory contexts e.g.
> > > using MemoryContextStats().
> > > So IMHO if it's necessary to know memory contexts without attaching gdb
> > > for other than client backends(probably this means using under
> > > production environment), this enhancement would be pay.
> >
> > Thanks for providing your thoughts. Knowing memory usage of auxiliary
> > processes is as important as backends (user session processes) without
> > attaching debugger in production environments.
> >
> > There are some open points as mentioned in my first mail in this
> > thread, I will start working on this patch once we agree on them.
>
> I'm attaching the v1 patch that enables
> pg_log_backend_memory_contexts() to log memory contexts of auxiliary
> processes. Please review it.
>
> Here's the CF entry - https://commitfest.postgresql.org/35/3385/

After the patch applied the function looks like this

proc = BackendPidGetProc(pid);
if (proc == NULL)
<try aux processes>
<set is_aux_proc>
if (proc == NULL)
<error>
if (!is_aux_proc)
<set local backend id>
SendProcSignal(.., the backend id);

is_aux_proc lookslike making the code complex. I think we can remove
it.

+ /* Only regular backends will have valid backend id, auxiliary processes don't. */
+ if (!is_aux_proc)
+ backendId = proc->backendId;

I think the reason we need to do this is not that aux processes have
the invalid backend id (=InvalidBackendId) but that "some" auxiliary
processes may have a broken proc->backendId in regard to
SendProcSignal (we know that's the startup for now.).

+SELECT pg_log_backend_memory_contexts(memcxt_get_proc_pid('autovacuum launcher'+SELECT pg_log_backend_memory_contexts(memcxt_get_proc_pid('logical replication launcher'));
...

Maybe we can reduce (a quite bit of) run time of the test by
loopingover the processes but since the test only checks if the
function doesn't fail to send a signal, I'm not sure we need to
perform the test for all of the processes here. On the other hand,
the test is missing the most significant target of the startup
process.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-11-01 01:44:34 Re: parallel vacuum comments
Previous Message Mark Dilger 2021-11-01 00:24:05 Re: Delegating superuser tasks to new security roles (Was: Granting control of SUSET gucs to non-superusers)