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 |
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 |