Re: Printing backtrace of postgres processes

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Printing backtrace of postgres processes
Date: 2024-02-07 15:30:00
Message-ID: CALj2ACW3JuxHYn2bmgrdcR_D-QUwNk2Z_fxEqQ+=e6oNei8rkg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 7, 2024 at 2:57 PM Michael Paquier <michael(at)paquier(dot)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

Attachment Content-Type Size
v28-0002-Extend-backtrace-logging-function-to-auxiliary-p.patch application/octet-stream 6.0 KB
v28-0001-Add-function-to-log-backtrace-of-a-backend.patch application/octet-stream 16.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-02-07 15:41:38 Re: Change GUC hashtable to use simplehash?
Previous Message Tom Lane 2024-02-07 15:26:30 Re: pg_get_expr locking