On Wed, Feb 7, 2024 at 9:00 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Feb 7, 2024 at 2:57 PM Michael Paquier <mich...@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).
I've missed adding LoadBacktraceFunctions() in InitAuxiliaryProcess for 0002 patch. Please find the attached v29 patch set. Sorry for the noise. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v29-0001-Add-function-to-log-backtrace-of-a-backend.patch
Description: Binary data
v29-0002-Extend-backtrace-logging-function-for-auxiliary-.patch
Description: Binary data
