On Tue, Apr 5, 2022 at 9:18 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Wed, Mar 30, 2022 at 12:03 PM Justin Pryzby <pry...@telsasoft.com> wrote:
> > On Wed, Mar 30, 2022 at 11:53:52AM -0400, Greg Stark wrote:
> > > Sadly the cfbot is showing a patch conflict again. It's just a trivial
> > > conflict in the regression test schedule so I'm not going to update
> > > the status but it would be good to rebase it so we continue to get
> > > cfbot testing.
> >
> > Done.  No changes.
>
> + if (chk_auxiliary_proc)
> + ereport(WARNING,
> + errmsg("PID %d is not a PostgreSQL server process", pid));
> + else
> + ereport(WARNING,
> + errmsg("PID %d is not a PostgreSQL backend process", pid));
>
> This doesn't look right to me. I don't think that PostgreSQL server
> processes are one kind of thing and PostgreSQL backend processes are
> another kind of thing. I think they're two terms that are pretty
> nearly interchangeable, or maybe at best you want to argue that
> backend processes are some subset of server processes. I don't see
> this sort of thing adding any clarity.

I have changed it to "PID %d is not a PostgreSQL server process" which
is similar to the existing warning message in
pg_log_backend_memory_contexts.

> -static void
> +void
>  set_backtrace(ErrorData *edata, int num_skip)
>  {
>   StringInfoData errtrace;
> @@ -978,7 +978,18 @@ set_backtrace(ErrorData *edata, int num_skip)
>      "backtrace generation is not supported by this installation");
>  #endif
>
> - edata->backtrace = errtrace.data;
> + if (edata)
> + edata->backtrace = errtrace.data;
> + else
> + {
> + /*
> + * LOG_SERVER_ONLY is used to make sure the backtrace is never
> + * sent to client. We want to avoid messing up the other client
> + * session.
> + */
> + elog(LOG_SERVER_ONLY, "current backtrace:%s", errtrace.data);
> + pfree(errtrace.data);
> + }
>  }
>
> This looks like a grotty hack.

I have changed it so that the backtrace is set and returned to the
caller. The caller will take care of logging or setting it to the
error data context. Thoughts?

> - PGPROC    *proc = BackendPidGetProc(pid);
> -
> - /*
> - * BackendPidGetProc returns NULL if the pid isn't valid; but by the time
> - * we reach kill(), a process for which we get a valid proc here might
> - * have terminated on its own.  There's no way to acquire a lock on an
> - * arbitrary process to prevent that. But since so far all the callers of
> - * this mechanism involve some request for ending the process anyway, that
> - * it might end on its own first is not a problem.
> - *
> - * Note that proc will also be NULL if the pid refers to an auxiliary
> - * process or the postmaster (neither of which can be signaled via
> - * pg_signal_backend()).
> - */
> - if (proc == NULL)
> - {
> - /*
> - * This is just a warning so a loop-through-resultset will not abort
> - * if one backend terminated on its own during the run.
> - */
> - ereport(WARNING,
> - (errmsg("PID %d is not a PostgreSQL backend process", pid)));
> + PGPROC  *proc;
>
> + /* Users can only signal valid backend or an auxiliary process. */
> + proc = CheckPostgresProcessId(pid, false, NULL);
> + if (!proc)
>   return SIGNAL_BACKEND_ERROR;
> - }
>
> Incidentally changing the behavior of pg_signal_backend() doesn't seem
> like a great idea. We can do that as a separate commit, after
> considering whether documentation changes are needed. But it's not
> something that should get folded into a commit on another topic.

Agreed. I have kept the logic of pg_signal_backend as it is.
pg_log_backtrace and pg_log_backend_memory_contexts use the same
functionality to check and send signal. I have added a new function
"CheckProcSendDebugSignal" which has the common code implementation
and will be called by both pg_log_backtrace and
pg_log_backend_memory_contexts.

> + /*
> + * BackendPidGetProc() and AuxiliaryPidGetProc() return NULL if the pid
> + * isn't valid; but by the time we reach kill(), a process for which we
> + * get a valid proc here might have terminated on its own.  There's no way
> + * to acquire a lock on an arbitrary process to prevent that. But since
> + * this mechanism is usually used to debug a backend or an auxiliary
> + * process running and consuming lots of memory, that it might end on its
> + * own first without logging the requested info is not a problem.
> + */
>
> This comment made a lot more sense where it used to be than it does
> where it is now. I think more work is required here than just cutting
> and pasting.

This function was called from pg_signal_backend earlier, this logic is
now moved to CheckProcSendDebugSignal which will be called only by
pg_log_backtrace and pg_log_backend_memory_contexts. This looks
appropriate in CheckProcSendDebugSignal with slight change to specify
it can consume lots of memory or a long running process.
Thoughts?

Attached v20 patch has the changes for the same.

Regards,
Vignesh
From cc3847b30e0b61218aebdf68e837d366a82ebad1 Mon Sep 17 00:00:00 2001
From: Vigneshwaran C <vignes...@gmail.com>
Date: Wed, 6 Apr 2022 10:45:56 +0530
Subject: [PATCH v20] Add function to log the backtrace of the specified
 postgres  process.

This commit adds pg_log_backtrace() function that requests to log
the backtrace of the specified backend or auxiliary process except
logger and statistic collector.

Only superusers are allowed to request to log the backtrace
because allowing any users to issue this request at an unbounded rate
would cause lots of log messages and which can lead to denial of service.

On receipt of the request, at the next CHECK_FOR_INTERRUPTS(),
the target backend logs its backtrace at LOG_SERVER_ONLY level,
so that the backtrace will appear in the server log but not
be sent to the client.

Bump catalog version.

Authors: Vignesh C, Bharath Rupireddy, Justin Pryzby
Reviewers: Bharath Rupireddy, Justin Pryzby, Fujii Masao, Atsushi Torikoshi, Dilip Kumar, Robert Haas, Andres Freund, Tom lane, Craig Ringer
Discussion: https://www.postgresql.org/message-id/CALDaNm3ZzmFS-=r7oduzj7y7bgqv+n06kqyft6c3xzdoknk...@mail.gmail.com
---
 doc/src/sgml/func.sgml                    | 62 +++++++++++++++++++++++
 src/backend/catalog/system_functions.sql  |  2 +
 src/backend/postmaster/autovacuum.c       |  4 ++
 src/backend/postmaster/checkpointer.c     |  4 ++
 src/backend/postmaster/interrupt.c        |  4 ++
 src/backend/postmaster/pgarch.c           | 10 ++--
 src/backend/postmaster/startup.c          |  4 ++
 src/backend/postmaster/walwriter.c        |  4 ++
 src/backend/storage/ipc/procarray.c       | 59 +++++++++++++++++++++
 src/backend/storage/ipc/procsignal.c      | 56 ++++++++++++++++++++
 src/backend/storage/ipc/signalfuncs.c     | 30 +++++++++++
 src/backend/tcop/postgres.c               |  4 ++
 src/backend/utils/adt/mcxtfuncs.c         | 49 ++----------------
 src/backend/utils/error/elog.c            | 21 ++++----
 src/backend/utils/init/globals.c          |  1 +
 src/include/catalog/pg_proc.dat           |  5 ++
 src/include/miscadmin.h                   |  1 +
 src/include/storage/procarray.h           |  1 +
 src/include/storage/procsignal.h          |  3 +-
 src/include/utils/elog.h                  |  2 +
 src/test/regress/expected/backtrace.out   | 49 ++++++++++++++++++
 src/test/regress/expected/backtrace_1.out | 55 ++++++++++++++++++++
 src/test/regress/parallel_schedule        |  2 +-
 src/test/regress/sql/backtrace.sql        | 33 ++++++++++++
 24 files changed, 403 insertions(+), 62 deletions(-)
 create mode 100644 src/test/regress/expected/backtrace.out
 create mode 100644 src/test/regress/expected/backtrace_1.out
 create mode 100644 src/test/regress/sql/backtrace.sql

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4001cb2bda..98b0aec71f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25365,6 +25365,30 @@ SELECT collation for ('foo' COLLATE "de_DE");
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_log_backtrace</primary>
+        </indexterm>
+        <function>pg_log_backtrace</function> ( <parameter>pid</parameter> <type>integer</type> )
+        <returnvalue>boolean</returnvalue>
+       </para>
+       <para>
+        Requests to log the backtrace of the backend with the
+        specified process ID.  This function can send the request to
+        backends and auxiliary processes except the logger and statistics
+        collector.  The backtraces will be logged at <literal>LOG</literal>
+        message level. They will appear in the server log based on the log
+        configuration set (See <xref linkend="runtime-config-logging"/> for
+        more information), but will not be sent to the client regardless of
+        <xref linkend="guc-client-min-messages"/>. A backtrace identifies
+        which function a process is currently executing and it may be useful
+        for developers to diagnose stuck processes and other problems. This
+        function is supported only if PostgreSQL was built with the ability to
+        capture backtraces, otherwise it will emit a warning.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
@@ -25585,6 +25609,44 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
     because it may generate a large number of log messages.
    </para>
 
+   <para>
+    <function>pg_log_backtrace</function> can be used to log the backtrace of
+    a backend process. For example:
+<programlisting>
+postgres=# select pg_log_backtrace(pg_backend_pid());
+ pg_log_backtrace
+------------------
+ t
+(1 row)
+</programlisting>
+The backtrace will be logged as specified by the logging configuration.
+For example:
+<screen>
+2021-01-27 11:33:50.247 IST [111735] LOG:  current backtrace:
+        postgres: postgresdba postgres [local] SELECT(set_backtrace+0x38) [0xae06c5]
+        postgres: postgresdba postgres [local] SELECT(ProcessInterrupts+0x788) [0x950c34]
+        postgres: postgresdba postgres [local] SELECT() [0x761e89]
+        postgres: postgresdba postgres [local] SELECT() [0x71bbda]
+        postgres: postgresdba postgres [local] SELECT() [0x71e380]
+        postgres: postgresdba postgres [local] SELECT(standard_ExecutorRun+0x1d6) [0x71c1fe]
+        postgres: postgresdba postgres [local] SELECT(ExecutorRun+0x55) [0x71c026]
+        postgres: postgresdba postgres [local] SELECT() [0x953fc5]
+        postgres: postgresdba postgres [local] SELECT(PortalRun+0x262) [0x953c7e]
+        postgres: postgresdba postgres [local] SELECT() [0x94db78]
+        postgres: postgresdba postgres [local] SELECT(PostgresMain+0x7d7) [0x951e72]
+        postgres: postgresdba postgres [local] SELECT() [0x896b2f]
+        postgres: postgresdba postgres [local] SELECT() [0x8964b5]
+        postgres: postgresdba postgres [local] SELECT() [0x892a79]
+        postgres: postgresdba postgres [local] SELECT(PostmasterMain+0x116b) [0x892350]
+        postgres: postgresdba postgres [local] SELECT() [0x795f72]
+        /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f2107bbd505]
+        postgres: postgresdba postgres [local] SELECT() [0x4842a9]
+</screen>
+    You can obtain 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).
+   </para>
+
   </sect2>
 
   <sect2 id="functions-admin-backup">
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 81bac6f581..eab437988a 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -709,6 +709,8 @@ REVOKE EXECUTE ON FUNCTION pg_ls_logicalmapdir() FROM PUBLIC;
 
 REVOKE EXECUTE ON FUNCTION pg_ls_replslotdir(text) FROM PUBLIC;
 
+REVOKE EXECUTE ON FUNCTION pg_log_backtrace(integer) FROM PUBLIC;
+
 --
 -- We also set up some things as accessible to standard roles.
 --
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 681ef91b81..a572412f2a 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -841,6 +841,10 @@ HandleAutoVacLauncherInterrupts(void)
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
 
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
+
 	/* Process sinval catchup interrupts that happened while sleeping */
 	ProcessCatchupInterrupt();
 }
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index a59c3cf020..9f1d163bec 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -585,6 +585,10 @@ HandleCheckpointerInterrupts(void)
 	/* Perform logging of memory contexts of this process */
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
+
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
 }
 
 /*
diff --git a/src/backend/postmaster/interrupt.c b/src/backend/postmaster/interrupt.c
index 3f412dad2e..84b1de4967 100644
--- a/src/backend/postmaster/interrupt.c
+++ b/src/backend/postmaster/interrupt.c
@@ -48,6 +48,10 @@ HandleMainLoopInterrupts(void)
 	/* Perform logging of memory contexts of this process */
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
+
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
 }
 
 /*
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index d916ed39a8..af55e39cc1 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -765,9 +765,9 @@ pgarch_die(int code, Datum arg)
  * Interrupt handler for WAL archiver process.
  *
  * This is called in the loops pgarch_MainLoop and pgarch_ArchiverCopyLoop.
- * It checks for barrier events, config update and request for logging of
- * memory contexts, but not shutdown request because how to handle
- * shutdown request is different between those loops.
+ * It checks for barrier events, config update, request for logging of
+ * memory contexts and backtraces, but not shutdown request because how to
+ * handle shutdown request is different between those loops.
  */
 static void
 HandlePgArchInterrupts(void)
@@ -779,6 +779,10 @@ HandlePgArchInterrupts(void)
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
 
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
+
 	if (ConfigReloadPending)
 	{
 		char	   *archiveLib = pstrdup(XLogArchiveLibrary);
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index 29cf8f18e1..013d8fe68f 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -206,6 +206,10 @@ HandleStartupProcInterrupts(void)
 	/* Perform logging of memory contexts of this process */
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
+
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
 }
 
 
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 102fa2a089..52d5a82ecb 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -310,4 +310,8 @@ HandleWalWriterInterrupts(void)
 	/* Perform logging of memory contexts of this process */
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
+
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
 }
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 735763cc24..a8cc210975 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3161,6 +3161,65 @@ HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int type)
 	return result;
 }
 
+/*
+ * CheckProcSendDebugSignal -- check if the process with given pid is a backend
+ * or an auxiliary process and send the specified DEBUG signal.
+ *
+ * Returns true if sending the signal was successful, false otherwise
+ */
+bool
+CheckProcSendDebugSignal(int pid, ProcSignalReason signo)
+{
+	PGPROC	   *proc;
+	BackendId	backendId = InvalidBackendId;
+
+	proc = BackendPidGetProc(pid);
+
+	/*
+	 * See if the process with given pid is a backend or an auxiliary process.
+	 *
+	 * If the given process is a backend, use its backend id in
+	 * SendProcSignal() later to speed up the operation. Otherwise, don't do
+	 * that because auxiliary processes (except the startup process) don't
+	 * have a valid backend id.
+	 */
+	if (proc != NULL)
+		backendId = proc->backendId;
+	else
+		proc = AuxiliaryPidGetProc(pid);
+
+	/*
+	 * BackendPidGetProc() and AuxiliaryPidGetProc() return NULL if the pid
+	 * isn't valid; but by the time we reach kill(), a process for which we
+	 * get a valid proc here might have terminated on its own.  There's no way
+	 * to acquire a lock on an arbitrary process to prevent that. But since
+	 * this mechanism is usually used to debug a backend or an auxiliary
+	 * process running and consuming lots of memory or a long running process,
+	 * that it might end on its own first and its memory contexts are not
+	 * logged or backtrace not logged is not a problem.
+	 */
+	if (proc == NULL)
+	{
+		/*
+		 * This is just a warning so a loop-through-resultset will not abort
+		 * if one backend terminated on its own during the run.
+		 */
+		ereport(WARNING,
+				(errmsg("PID %d is not a PostgreSQL server process", pid)));
+		return false;
+	}
+
+	if (SendProcSignal(pid, signo, backendId) < 0)
+	{
+		/* Again, just a warning to allow loops */
+		ereport(WARNING,
+				(errmsg("could not send signal to process %d: %m", pid)));
+		return false;
+	}
+
+	return true;
+}
+
 /*
  * BackendPidGetProc -- get a backend's PGPROC given its PID
  *
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index f41563a0a4..5a54a1480d 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -603,6 +603,59 @@ ResetProcSignalBarrierBits(uint32 flags)
 	InterruptPending = true;
 }
 
+/*
+ * HandleLogBacktraceInterrupt - Handle receipt of an interrupt requesting to
+ * log a backtrace.
+ *
+ * All the actual work is deferred to ProcessLogBacktraceInterrupt(),
+ * because we cannot safely emit a log message inside the signal handler.
+ */
+static void
+HandleLogBacktraceInterrupt(void)
+{
+	InterruptPending = true;
+	LogBacktracePending = true;
+	/* latch will be set by procsignal_sigusr1_handler */
+}
+
+/*
+ * ProcessLogBacktraceInterrupt - Perform logging of backtrace of this
+ * backend process.
+ *
+ * Any backend that participates in ProcSignal signaling must arrange
+ * to call this function if we see LogBacktracePending set.
+ * CHECK_FOR_INTERRUPTS() or from process specific interrupt handlers.
+ */
+void
+ProcessLogBacktraceInterrupt(void)
+{
+	StringInfo errtrace = makeStringInfo();
+
+	LogBacktracePending = false;
+
+	/*
+	 * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+	 * connected client.
+	 */
+	ereport(LOG_SERVER_ONLY,
+			errhidestmt(true),
+			errhidecontext(true),
+			errmsg_internal("logging backtrace of PID %d", MyProcPid));
+
+	errtrace->data = set_backtrace(0);
+	if (!errtrace->data)
+		return;
+
+	/*
+	 * LOG_SERVER_ONLY is used to make sure the backtrace is never
+	 * sent to client. We want to avoid messing up the other client
+	 * session.
+	 */
+	elog(LOG_SERVER_ONLY, "current backtrace:%s", errtrace->data);
+	pfree(errtrace->data);
+	pfree(errtrace);
+}
+
 /*
  * CheckProcSignal - check to see if a particular reason has been
  * signaled, and clear the signal flag.  Should be called after receiving
@@ -652,6 +705,9 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
 	if (CheckProcSignal(PROCSIG_LOG_MEMORY_CONTEXT))
 		HandleLogMemoryContextInterrupt();
 
+	if (CheckProcSignal(PROCSIG_LOG_BACKTRACE))
+		HandleLogBacktraceInterrupt();
+
 	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_DATABASE))
 		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_DATABASE);
 
diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
index 6e310b14eb..1ed7ce7218 100644
--- a/src/backend/storage/ipc/signalfuncs.c
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -303,3 +303,33 @@ pg_rotate_logfile_v2(PG_FUNCTION_ARGS)
 	SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
 	PG_RETURN_BOOL(true);
 }
+
+/*
+ * pg_log_backtrace
+ *		Signal a backend or an auxiliary process to log its backtrace.
+ *
+ * By default, only superusers are allowed to signal to log the backtrace
+ * because allowing any user to issue this request at an unbounded
+ * rate would cause lots of log messages which can lead to denial of service.
+ * Additional roles can be permitted with GRANT.
+ *
+ * On receipt of this signal, a backend or an auxiliary process sets the flag
+ * in the signal handler, which causes the next CHECK_FOR_INTERRUPTS()
+ * or process-specific interrupt handler to log the backtrace.
+ */
+Datum
+pg_log_backtrace(PG_FUNCTION_ARGS)
+{
+	int			pid = PG_GETARG_INT32(0);
+	bool		result;
+
+#ifndef HAVE_BACKTRACE_SYMBOLS
+	ereport(WARNING,
+			errmsg("backtrace generation is not supported by this installation"),
+			errhint("You need to rebuild PostgreSQL using a library containing backtrace_symbols."));
+	PG_RETURN_BOOL(false);
+#endif
+
+	result = CheckProcSendDebugSignal(pid, PROCSIG_LOG_BACKTRACE);
+	PG_RETURN_BOOL(result);
+}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index ba2fcfeb4a..2b1d1610a7 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3380,6 +3380,10 @@ ProcessInterrupts(void)
 
 	if (LogMemoryContextPending)
 		ProcessLogMemoryContextInterrupt();
+
+	/* Perform logging of backtrace of this process */
+	if (LogBacktracePending)
+		ProcessLogBacktraceInterrupt();
 }
 
 
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index bb7cc94024..6dffe63943 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -145,51 +145,8 @@ Datum
 pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
 {
 	int			pid = PG_GETARG_INT32(0);
-	PGPROC	   *proc;
-	BackendId	backendId = InvalidBackendId;
+	bool		result;
 
-	proc = BackendPidGetProc(pid);
-
-	/*
-	 * See if the process with given pid is a backend or an auxiliary process.
-	 *
-	 * If the given process is a backend, use its backend id in
-	 * SendProcSignal() later to speed up the operation. Otherwise, don't do
-	 * that because auxiliary processes (except the startup process) don't
-	 * have a valid backend id.
-	 */
-	if (proc != NULL)
-		backendId = proc->backendId;
-	else
-		proc = AuxiliaryPidGetProc(pid);
-
-	/*
-	 * BackendPidGetProc() and AuxiliaryPidGetProc() return NULL if the pid
-	 * isn't valid; but by the time we reach kill(), a process for which we
-	 * get a valid proc here might have terminated on its own.  There's no way
-	 * to acquire a lock on an arbitrary process to prevent that. But since
-	 * this mechanism is usually used to debug a backend or an auxiliary
-	 * process running and consuming lots of memory, that it might end on its
-	 * own first and its memory contexts are not logged is not a problem.
-	 */
-	if (proc == NULL)
-	{
-		/*
-		 * This is just a warning so a loop-through-resultset will not abort
-		 * if one backend terminated on its own during the run.
-		 */
-		ereport(WARNING,
-				(errmsg("PID %d is not a PostgreSQL server process", pid)));
-		PG_RETURN_BOOL(false);
-	}
-
-	if (SendProcSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT, backendId) < 0)
-	{
-		/* Again, just a warning to allow loops */
-		ereport(WARNING,
-				(errmsg("could not send signal to process %d: %m", pid)));
-		PG_RETURN_BOOL(false);
-	}
-
-	PG_RETURN_BOOL(true);
+	result = CheckProcSendDebugSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT);
+	PG_RETURN_BOOL(result);
 }
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 7402696986..97ec2a5065 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -172,7 +172,6 @@ static char formatted_log_time[FORMATTED_TS_LEN];
 
 
 static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
-static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
 static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
 static void write_console(const char *line, int len);
 static const char *process_log_prefix_padding(const char *p, int *padding);
@@ -546,7 +545,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
 		edata->funcname &&
 		backtrace_functions &&
 		matches_backtrace_functions(edata->funcname))
-		set_backtrace(edata, 2);
+		edata->backtrace = set_backtrace(2);
 
 	/*
 	 * Call any context callback functions.  Errors occurring in callback
@@ -932,7 +931,7 @@ errbacktrace(void)
 	CHECK_STACK_DEPTH();
 	oldcontext = MemoryContextSwitchTo(edata->assoc_context);
 
-	set_backtrace(edata, 1);
+	edata->backtrace = set_backtrace(1);
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
@@ -941,13 +940,13 @@ errbacktrace(void)
 }
 
 /*
- * Compute backtrace data and add it to the supplied ErrorData.  num_skip
- * specifies how many inner frames to skip.  Use this to avoid showing the
- * internal backtrace support functions in the backtrace.  This requires that
- * this and related functions are not inlined.
+ * Compute backtrace data and return it.  num_skip specifies how many inner
+ * frames to skip.  Use this to avoid showing the internal backtrace support
+ * functions in the backtrace.  This requires that this and related functions
+ * are not inlined.
  */
-static void
-set_backtrace(ErrorData *edata, int num_skip)
+char *
+set_backtrace(int num_skip)
 {
 	StringInfoData errtrace;
 
@@ -962,7 +961,7 @@ set_backtrace(ErrorData *edata, int num_skip)
 		nframes = backtrace(buf, lengthof(buf));
 		strfrms = backtrace_symbols(buf, nframes);
 		if (strfrms == NULL)
-			return;
+			return NULL;
 
 		for (int i = num_skip; i < nframes; i++)
 			appendStringInfo(&errtrace, "\n%s", strfrms[i]);
@@ -973,7 +972,7 @@ set_backtrace(ErrorData *edata, int num_skip)
 						   "backtrace generation is not supported by this installation");
 #endif
 
-	edata->backtrace = errtrace.data;
+	return errtrace.data;
 }
 
 /*
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 3419c099b2..2dd2cc3dc6 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -36,6 +36,7 @@ volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false;
 volatile sig_atomic_t IdleSessionTimeoutPending = false;
 volatile sig_atomic_t ProcSignalBarrierPending = false;
 volatile sig_atomic_t LogMemoryContextPending = false;
+volatile sig_atomic_t LogBacktracePending = false;
 volatile uint32 InterruptHoldoffCount = 0;
 volatile uint32 QueryCancelHoldoffCount = 0;
 volatile uint32 CritSectionCount = 0;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 25304430f4..f32662b0f8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11839,4 +11839,9 @@
   prorettype => 'bytea', proargtypes => 'pg_brin_minmax_multi_summary',
   prosrc => 'brin_minmax_multi_summary_send' },
 
+# function to get the backtrace of server process
+{ oid => '6105', descr => 'log backtrace of server process',
+  proname => 'pg_log_backtrace', provolatile => 'v', prorettype => 'bool',
+  proargtypes => 'int4', prosrc => 'pg_log_backtrace' },
+
 ]
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0abc3ad540..2309874d47 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -94,6 +94,7 @@ extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending;
 extern PGDLLIMPORT volatile sig_atomic_t IdleSessionTimeoutPending;
 extern PGDLLIMPORT volatile sig_atomic_t ProcSignalBarrierPending;
 extern PGDLLIMPORT volatile sig_atomic_t LogMemoryContextPending;
+extern PGDLLIMPORT volatile sig_atomic_t LogBacktracePending;
 
 extern PGDLLIMPORT volatile sig_atomic_t CheckClientConnectionPending;
 extern PGDLLIMPORT volatile sig_atomic_t ClientConnectionLost;
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 1b2cfac5ad..a60b6f82f5 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -67,6 +67,7 @@ extern PGPROC *BackendPidGetProc(int pid);
 extern PGPROC *BackendPidGetProcWithLock(int pid);
 extern int	BackendXidGetPid(TransactionId xid);
 extern bool IsBackendPid(int pid);
+extern bool CheckProcSendDebugSignal(int pid, ProcSignalReason signo);
 
 extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
 												   bool excludeXmin0, bool allDbs, int excludeVacuum,
diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h
index ee636900f3..af3954d564 100644
--- a/src/include/storage/procsignal.h
+++ b/src/include/storage/procsignal.h
@@ -35,6 +35,7 @@ typedef enum
 	PROCSIG_WALSND_INIT_STOPPING,	/* ask walsenders to prepare for shutdown  */
 	PROCSIG_BARRIER,			/* global barrier interrupt  */
 	PROCSIG_LOG_MEMORY_CONTEXT, /* ask backend to log the memory contexts */
+	PROCSIG_LOG_BACKTRACE,		/* ask backend to log the current backtrace */
 
 	/* Recovery conflict reasons */
 	PROCSIG_RECOVERY_CONFLICT_DATABASE,
@@ -65,7 +66,7 @@ extern int	SendProcSignal(pid_t pid, ProcSignalReason reason,
 extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type);
 extern void WaitForProcSignalBarrier(uint64 generation);
 extern void ProcessProcSignalBarrier(void);
-
+extern void ProcessLogBacktraceInterrupt(void);
 extern void procsignal_sigusr1_handler(SIGNAL_ARGS);
 
 #endif							/* PROCSIGNAL_H */
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 3eb8de3966..9a7d20903c 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -467,4 +467,6 @@ extern void set_syslog_parameters(const char *ident, int facility);
  */
 extern void write_stderr(const char *fmt,...) pg_attribute_printf(1, 2);
 
+extern pg_noinline char *set_backtrace(int num_skip);
+
 #endif							/* ELOG_H */
diff --git a/src/test/regress/expected/backtrace.out b/src/test/regress/expected/backtrace.out
new file mode 100644
index 0000000000..2184a99483
--- /dev/null
+++ b/src/test/regress/expected/backtrace.out
@@ -0,0 +1,49 @@
+--
+-- pg_log_backtrace()
+--
+-- Backtraces are logged and not returned to the function.
+-- Furthermore, their contents can vary depending on the timing. However,
+-- we can at least verify that the code doesn't fail, and that the
+-- permissions are set properly.
+--
+SELECT pg_log_backtrace(pg_backend_pid());
+ pg_log_backtrace 
+------------------
+ t
+(1 row)
+
+SELECT pg_log_backtrace(pid) FROM pg_stat_activity
+  WHERE backend_type = 'checkpointer';
+ pg_log_backtrace 
+------------------
+ t
+(1 row)
+
+CREATE ROLE regress_log_backtrace;
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- no
+ has_function_privilege 
+------------------------
+ f
+(1 row)
+
+GRANT EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  TO regress_log_backtrace;
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- yes
+ has_function_privilege 
+------------------------
+ t
+(1 row)
+
+SET ROLE regress_log_backtrace;
+SELECT pg_log_backtrace(pg_backend_pid());
+ pg_log_backtrace 
+------------------
+ t
+(1 row)
+
+RESET ROLE;
+REVOKE EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  FROM regress_log_backtrace;
+DROP ROLE regress_log_backtrace;
diff --git a/src/test/regress/expected/backtrace_1.out b/src/test/regress/expected/backtrace_1.out
new file mode 100644
index 0000000000..fe9523f89d
--- /dev/null
+++ b/src/test/regress/expected/backtrace_1.out
@@ -0,0 +1,55 @@
+--
+-- pg_log_backtrace()
+--
+-- Backtraces are logged and not returned to the function.
+-- Furthermore, their contents can vary depending on the timing. However,
+-- we can at least verify that the code doesn't fail, and that the
+-- permissions are set properly.
+--
+SELECT pg_log_backtrace(pg_backend_pid());
+WARNING:  backtrace generation is not supported by this installation
+HINT:  You need to rebuild PostgreSQL using a library containing backtrace_symbols.
+ pg_log_backtrace 
+------------------
+ f
+(1 row)
+
+SELECT pg_log_backtrace(pid) FROM pg_stat_activity
+  WHERE backend_type = 'checkpointer';
+WARNING:  backtrace generation is not supported by this installation
+HINT:  You need to rebuild PostgreSQL using a library containing backtrace_symbols.
+ pg_log_backtrace 
+------------------
+ f
+(1 row)
+
+CREATE ROLE regress_log_backtrace;
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- no
+ has_function_privilege 
+------------------------
+ f
+(1 row)
+
+GRANT EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  TO regress_log_backtrace;
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- yes
+ has_function_privilege 
+------------------------
+ t
+(1 row)
+
+SET ROLE regress_log_backtrace;
+SELECT pg_log_backtrace(pg_backend_pid());
+WARNING:  backtrace generation is not supported by this installation
+HINT:  You need to rebuild PostgreSQL using a library containing backtrace_symbols.
+ pg_log_backtrace 
+------------------
+ f
+(1 row)
+
+RESET ROLE;
+REVOKE EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  FROM regress_log_backtrace;
+DROP ROLE regress_log_backtrace;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 5030d19c03..489f543a83 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -86,7 +86,7 @@ test: brin_bloom brin_multi
 # psql depends on create_am
 # amutils depends on geometry, create_index_spgist, hash_index, brin
 # ----------
-test: create_table_like alter_generic alter_operator misc async dbsize merge misc_functions sysviews tsrf tid tidscan tidrangescan collate.icu.utf8 incremental_sort create_role
+test: create_table_like alter_generic alter_operator misc async dbsize merge misc_functions sysviews tsrf tid tidscan tidrangescan collate.icu.utf8 incremental_sort create_role backtrace
 
 # collate.*.utf8 tests cannot be run in parallel with each other
 test: rules psql psql_crosstab amutils stats_ext collate.linux.utf8
diff --git a/src/test/regress/sql/backtrace.sql b/src/test/regress/sql/backtrace.sql
new file mode 100644
index 0000000000..d74b1016ae
--- /dev/null
+++ b/src/test/regress/sql/backtrace.sql
@@ -0,0 +1,33 @@
+--
+-- pg_log_backtrace()
+--
+-- Backtraces are logged and not returned to the function.
+-- Furthermore, their contents can vary depending on the timing. However,
+-- we can at least verify that the code doesn't fail, and that the
+-- permissions are set properly.
+--
+
+SELECT pg_log_backtrace(pg_backend_pid());
+
+SELECT pg_log_backtrace(pid) FROM pg_stat_activity
+  WHERE backend_type = 'checkpointer';
+
+CREATE ROLE regress_log_backtrace;
+
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- no
+
+GRANT EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  TO regress_log_backtrace;
+
+SELECT has_function_privilege('regress_log_backtrace',
+  'pg_log_backtrace(integer)', 'EXECUTE'); -- yes
+
+SET ROLE regress_log_backtrace;
+SELECT pg_log_backtrace(pg_backend_pid());
+RESET ROLE;
+
+REVOKE EXECUTE ON FUNCTION pg_log_backtrace(integer)
+  FROM regress_log_backtrace;
+
+DROP ROLE regress_log_backtrace;
-- 
2.32.0

Reply via email to