Re: Printing backtrace of postgres processes

At Fri, 29 Jan 2021 19:10:24 +0530, vignesh C <vignesh21@gmail.com> wrote in 
> Attached v5 patch has the fixes for the same.

PMSIGNAL_BACKTRACE_EMIT is not used anywhere?
(the commit message seems stale.)

+++ b/src/bin/pg_ctl/t/005_backtrace.pl

pg_ctl doesn't (or no longer?) seem relevant to this patch.


+	eval {
+		$current_logfiles = slurp_file($node->data_dir . '/current_logfiles');
+	};
+	last unless $@;

Is there any reason not just to do "while (! -e <filenmae)) { usleep(); }" ?


+logging_collector = on

I don't see a reason for turning on logging collector here.


+gdb ./postgres
+GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-115.el7
+Copyright (C) 2013 Free Software Foundation, Inc.
+License GPLv3+: GNU GPL version 3 or later <literal>&lt;</literal>http://gnu.org/licenses/gpl.html<literal>&gt;</literal>
+This is free software: you are free to change and redistribute it.
+There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
+and "show warranty" for details.
+This GDB was configured as "x86_64-redhat-linux-gnu".
+For bug reporting instructions, please see:
+<literal>&lt;</literal>http://www.gnu.org/software/gdb/bugs/<literal>&gt;</literal>...
+Reading symbols from /home/postgresdba/inst/bin/postgres...done.

Almost all of the banner lines seem to be useless here.


 #define SIGNAL_BACKEND_SUCCESS 0
+#define BACKEND_VALIDATION_SUCCESS 0
 #define SIGNAL_BACKEND_ERROR 1
 #define SIGNAL_BACKEND_NOPERMISSION 2
 #define SIGNAL_BACKEND_NOSUPERUSER 3

Even though I can share the feeling that SIGNAL_BACKEND_SUCCESS is a
bit odd to represent "sending signal is allowed", I feel that it's
better using the existing symbol than using the new symbol.


+validate_backend_pid(int pid)

The function needs a comment. The name is somewhat
confusing. check_privilege_to_send_singal()?

Maybe the return value of the function should be changed to an enum,
and its callers should use switch-case to handle the value.


+			if (!SendProcSignal(bt_pid, PROCSIG_BACKTRACE_PRINT, InvalidBackendId))
+				PG_RETURN_BOOL(true);
+			else
+				ereport(WARNING,
+						(errmsg("failed to send signal to postmaster: %m")));
+		}
+
+		PG_RETURN_BOOL(result);

The variable "result" seems useless.


+	elog(LOG_SERVER_ONLY, "current backtrace:%s", errtrace.data);
+
+	errno = save_errno;
+}

You need to release the resouces held by the errtrace. And the
errtrace is a bit pointless. Why isn't it "backtrace"?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center