On Tue, Nov 9, 2021 at 4:45 PM vignesh C <vignes...@gmail.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,
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,
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();
[1] commit f0b051e322d530a340e62f2ae16d99acdbcb3d05
Author: Jeff Davis <jda...@postgresql.org>
Date: Tue Oct 26 13:13:52 2021 -0700
Allow GRANT on pg_log_backend_memory_contexts().
Remove superuser check, allowing any user granted permissions on
pg_log_backend_memory_contexts() to log the memory contexts of any
backend.
Note that this could allow a privileged non-superuser to log the
memory contexts of a superuser backend, but as discussed, that does
not seem to be a problem.
Reviewed-by: Nathan Bossart, Bharath Rupireddy, Michael Paquier,
Kyotaro Horiguchi, Andres Freund
Discussion:
https://postgr.es/m/e5cf6684d17c8d1ef4904ae248605ccd6da03e72.ca...@j-davis.com
[2]
https://www.postgresql.org/message-id/CALj2ACU1nBzpacOK2q%3Da65S_4%2BOaz_rLTsU1Ri0gf7YUmnmhfQ%40mail.gmail.com
Regards,
Bharath Rupireddy.
