Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes
Date
Msg-id 20211101.101207.2030923294047951899.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: enhance pg_log_backend_memory_contexts() to log memory contexts of auxiliary processes
List pgsql-hackers
At Fri, 29 Oct 2021 22:25:04 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in 
> On Mon, Oct 11, 2021 at 9:55 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Mon, Oct 11, 2021 at 8:21 AM torikoshia <torikoshia@oss.nttdata.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('logicalreplication 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



pgsql-hackers by date:

Previous
From: Mark Dilger
Date:
Subject: Re: Delegating superuser tasks to new security roles (Was: Granting control of SUSET gucs to non-superusers)
Next
From: Masahiko Sawada
Date:
Subject: Re: parallel vacuum comments