On Thu, Feb 08, 2024 at 12:30:00AM +0530, Bharath Rupireddy wrote:
> I've missed adding LoadBacktraceFunctions() in InitAuxiliaryProcess
> for 0002 patch. Please find the attached v29 patch set. Sorry for the
> noise.
I've been torturing the patch with \watch and loops calling the
function while doing a sequential scan of pg_stat_activity, and that
was stable while doing a pgbench and an installcheck-world in
parallel, with some infinite loops and some spinlocks I should not
have taken.
+ if (backtrace_functions_loaded)
+ return;
I was really wondering about this point, particularly regarding the
fact that this would load libgcc for all the backends when they start,
unconditionally. One thing could be to hide that behind a postmaster
GUC disabled by default, but then we'd come back to using gdb on a
live server, which is no fun on a customer environment because of the
extra dependencies, which may not, or just cannot, be installed. So
yeah, I think that I'm OK about that.
+ * procsignal.h
+ * Backtrace-related routines
This one is incorrect.
In HandleLogBacktraceInterrupt(), we don't use backtrace_symbols() and
rely on backtrace_symbols_fd() to avoid doing malloc() in the signal
handler as mentioned in [1] back in 2022. Perhaps the part about the
fact that we don't use backtrace_symbols() should be mentioned
explicitely in a comment rather than silently implied? That's
a very important point.
Echoing with upthread, and we've been more lax with superuser checks
and assignment of custom roles in recent years, I agree with the
approach of the patch to make that superuser by default. Then users
can force their own policy as they want with an EXECUTE ACL on the SQL
function.
As a whole, I'm pretty excited about being able to rely on that
without the need to use gdb to get a live trace. Does anybody have
objections and/or comments, particularly about the superuser and the
load part at backend startup? This thread has been going on for so
long that it would be good to let 1 week for folks to react before
doing anything. See v29 for references.
[1]: https://www.postgresql.org/message-id/CALj2ACUNZVB0cQovvKBd53-upsMur8j-5_K=-fg86uAa+WYEWg@mail.gmail.com
--
Michael