Re: Printing backtrace of postgres processes

On Wed, Feb 7, 2024 at 2:57 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Feb 07, 2024 at 02:04:39PM +0530, Bharath Rupireddy wrote:
> > Well, that 'ubuntu' is the default username there, I've changed it now
> > and kept the output short.
>
> I would keep it just at two or three lines, with a "For example, with
> lines like":

Done.

> > I've simplified the tests, now we don't need two separate output files
> > for tests. Please see the attached v27 patch.
>
> +  proname => 'pg_log_backtrace', provolatile => 'v', prorettype => 'bool',
>
> Hmm.  Would it be better to be in line with memory contexts logging
> and use pg_log_backend_backtrace()?

+1.

> One thing I was wondering is that
> there may be a good point in splitting the backtrace support into two
> functions (backends and auxiliary processes) that could be split with
> two execution ACLs across different roles.

-1 for that unless we have any requests. I mean, backtrace is a common
thing for all postgres processes, why different controls are needed?
I'd go with what pg_log_backend_memory_contexts does - it supports
both backends and auxiliary processes.

> +   PROCSIG_LOG_BACKTRACE,      /* ask backend to log the current backtrace */
>
> Incorrect order.

PROCSIG_XXX aren't alphabetically ordered, no?

> +-- Backtrace is not logged for auxiliary processes
>
> Not sure there's a point in keeping that in the tests for now.
>
> +    * XXX: It might be worth implementing it for auxiliary processes.
>
> Same, I would remove that.

Done.

> +static volatile sig_atomic_t backtrace_functions_loaded = false;
>
> Hmm, so you need that because of the fact that it would be called in a
> signal as backtrace(3) says:
> "If you need certain calls to these two functions to not allocate
> memory (in signal handlers, for example), you need to make sure libgcc
> is loaded beforehand".
>
> True that it is not interesting to only log something when having a
> CFI, this needs to be dynamic to get a precise state of things.

Right.

I've also fixed some test failures. Please see the attached v28 patch
set. 0002 extends pg_log_backend_backtrace to auxiliary processes,
just like pg_log_backend_memory_contexts (not focused on PID
de-duplication code yet).

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com