Re: Printing backtrace of postgres processes

On Wed, Nov 10, 2021 at 12:17 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Tue, Nov 9, 2021 at 4:45 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > Thanks for reporting this, the attached v9 patch has the changes for the same.
>
> Thanks for the v9 patch. I have some comments:
>
> 1) I think we are moving away from if (!superuser()) checks, see the
> commit [1]. The goal is to let the GRANT-REVOKE system deal with who
> is supposed to run these system functions. Since pg_print_backtrace
> also writes the info to server logs,

Modified

> 2) I think we need to have LOG_SERVER_ONLY instead of LOG to avoid
> bakctrace being sent to the connected client. This will be good from
> security perspective as well since we don't send backtrace over the
> wire to the client.
> + PrintBacktracePending = false;
> + ereport(LOG,
> + (errmsg("logging backtrace of PID %d", MyProcPid)));
>
> for pg_log_backend_memory_contexts:
> + /*
> + * Use LOG_SERVER_ONLY to prevent the memory contexts
> from being sent
> + * to the connected client.
> + *
> + * We don't buffer the information about all memory
> contexts in a
> + * backend into StringInfo and log it as one message.
> Otherwise which
> + * may require the buffer to be enlarged very much and
> lead to OOM
> + * error since there can be a large number of memory
> contexts in a
> + * backend. Instead, we log one message per memory context.
> + */
> + ereport(LOG_SERVER_ONLY,

Modified

> 3) I think we need to extend this function to the auxiliary processes
> too, because users might be interested to see what these processes are
> doing and where they are currently stuck via their backtraces, see the
> proposal for pg_log_backend_memory_contexts at [2]. I think you need
> to add below code in couple of other places such as
> HandleCheckpointerInterrupts, HandleMainLoopInterrupts,
> HandlePgArchInterrupts, HandleStartupProcInterrupts,
> HandleWalWriterInterrupts.
>
> + /* Process printing backtrace */
> + if (PrintBacktracePending)
> + ProcessPrintBacktraceInterrupt();

Created 0002 patch to handle this.

Thanks for the comments, the attached v10 patch has the fixes for the same.

Regards,
Vignesh