On Fri, Jan 26, 2024 at 4:11 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
Thanks for reviewing.
> > + You can get the file name and line number from the logged details by using
> > + gdb/addr2line in linux platforms (users must ensure gdb/addr2line is
> > + already installed).
>
> This doesn't read great. I mean, what is gdb/addr2line? I think you
> mean either GDB or addr2line; this could be clearer.
Wrapped them in <productname> tag and reworded the comment a bit.
> > * internal backtrace support functions in the backtrace. This requires that
> > - * this and related functions are not inlined.
> > + * this and related functions are not inlined. If the edata pointer is valid,
> > + * backtrace information will be set in edata.
> > */
> > -static void
> > +void
> > set_backtrace(ErrorData *edata, int num_skip)
> > {
> > StringInfoData errtrace;
>
> This seems like a terrible API choice, and the comment change is no
> good. I suggest you need to create a function that deals only with a
> StringInfo, maybe
> append_backtrace(StringInfo buf, int num_skip)
> which is used by set_backtrace to print the backtrace in
> edata->backtrace, and a new function log_backtrace() that does the
> ereport(LOG_SERVER_ONLY) thing.
You probably were looking at v21, the above change isn't there in
versions after that. Can you please review the latest version v26
attached here?
We might want this patch extended to the newly added walsummarizer
process which I'll do so in the next version.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
