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
