Re: Printing backtrace of postgres processes - Mailing list pgsql-hackers

At Thu, 14 Apr 2022 10:33:50 +0530, vignesh C <vignesh21@gmail.com> wrote in 
> On Wed, Apr 6, 2022 at 12:29 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Tue, Apr 5, 2022 at 9:18 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > > This looks like a grotty hack.
> >
> > I have changed it so that the backtrace is set and returned to the
> > caller. The caller will take care of logging or setting it to the
> > error data context. Thoughts?

The point here I think is that the StringInfo is useless here in the
first place.  Addition to that, it is outside the use of StringInfo
hammering in a string pointer into it.

By the way, as Andres suggested very early in this thread, backrace()
can be called from signal handlers with some preparation.  So we can
run backtrace() in HandleLogBacktraceInterrupt() and backtrace_symbols
in ProceeLogBacktraceInterrupt().  This make this a bit complex, but
the outcome should be more informative.


> > > Incidentally changing the behavior of pg_signal_backend() doesn't seem
> > > like a great idea. We can do that as a separate commit, after
> > > considering whether documentation changes are needed. But it's not
> > > something that should get folded into a commit on another topic.
> >
> > Agreed. I have kept the logic of pg_signal_backend as it is.
> > pg_log_backtrace and pg_log_backend_memory_contexts use the same
> > functionality to check and send signal. I have added a new function
> > "CheckProcSendDebugSignal" which has the common code implementation
> > and will be called by both pg_log_backtrace and
> > pg_log_backend_memory_contexts.

The name looks like too specific than what it actually does, and
rather doesn't manifest what it does.
SendProcSignalBackendOrAuxproc() might be more eescriptive.

Or we can provide BackendOrAuxiliaryPidGetProc(int pid, BackendId &backendid).

> > > + /*
> > > + * BackendPidGetProc() and AuxiliaryPidGetProc() return NULL if the pid
> > > + * isn't valid; but by the time we reach kill(), a process for which we
> > > + * get a valid proc here might have terminated on its own.  There's no way
> > > + * to acquire a lock on an arbitrary process to prevent that. But since
> > > + * this mechanism is usually used to debug a backend or an auxiliary
> > > + * process running and consuming lots of memory, that it might end on its
> > > + * own first without logging the requested info is not a problem.
> > > + */
> > >
> > > This comment made a lot more sense where it used to be than it does
> > > where it is now. I think more work is required here than just cutting
> > > and pasting.
> >
> > This function was called from pg_signal_backend earlier, this logic is
> > now moved to CheckProcSendDebugSignal which will be called only by
> > pg_log_backtrace and pg_log_backend_memory_contexts. This looks
> > appropriate in CheckProcSendDebugSignal with slight change to specify
> > it can consume lots of memory or a long running process.
> > Thoughts?

For example. do you see the following part correct as well for
pg_log_backtrace()?

> + * this mechanism is usually used to debug a backend or an auxiliary
> + * process running and consuming lots of memory, that it might end on its

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center