From 40d4031abd0da3d84543b050e1ced2da775a3274 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 10 Jan 2025 11:08:17 -0500 Subject: [PATCH] postmaster: Improve logging of signals sent by postmaster Previously many, in some cases important, signals we never logged. In other cases the signal name was only included numerically. As part of this, change the debug log level the signal is logged at to DEBUG3, previously some where DEBUG2, some DEBUG4. Also move from direct use of kill() to signal the av launcher to signal_child(). There doesn't seem to be a reason for directly using kill(). Reviewed-by: Heikki Linnakangas Reviewed-by: Bertrand Drouvot Reviewed-by: Nazir Bilal Yavuz Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp --- src/backend/postmaster/postmaster.c | 56 ++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 3a9ad89783..c65288f5c1 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1693,7 +1693,7 @@ ServerLoop(void) { avlauncher_needs_signal = false; if (AutoVacLauncherPMChild != NULL) - kill(AutoVacLauncherPMChild->pid, SIGUSR2); + signal_child(AutoVacLauncherPMChild, SIGUSR2); } #ifdef HAVE_PTHREAD_IS_THREADED_NP @@ -3256,6 +3256,38 @@ LaunchMissingBackgroundProcesses(void) maybe_start_bgworkers(); } +/* + * Return string representation of signal. + * + * Because this is only implemented for signals we already rely on in this + * file we don't need to deal with unimplemented or same-numeric-value signals + * (as we'd e.g. have to for EWOULDBLOCK / EAGAIN). + */ +static const char * +pm_signame(int signal) +{ +#define PM_TOSTR_CASE(sym) case sym: return #sym + switch (signal) + { + PM_TOSTR_CASE(SIGABRT); + PM_TOSTR_CASE(SIGCHLD); + PM_TOSTR_CASE(SIGHUP); + PM_TOSTR_CASE(SIGINT); + PM_TOSTR_CASE(SIGKILL); + PM_TOSTR_CASE(SIGQUIT); + PM_TOSTR_CASE(SIGTERM); + PM_TOSTR_CASE(SIGUSR1); + PM_TOSTR_CASE(SIGUSR2); + default: + /* all signals sent by postmaster should be listed here */ + Assert(false); + return "(unknown)"; + } +#undef PM_TOSTR_CASE + + return ""; /* silence compiler */ +} + /* * Send a signal to a postmaster child process * @@ -3277,6 +3309,12 @@ signal_child(PMChild *pmchild, int signal) { pid_t pid = pmchild->pid; + ereport(DEBUG3, + (errmsg_internal("sending signal %d/%s to %s process with pid %d", + signal, pm_signame(signal), + GetBackendTypeDesc(pmchild->bkend_type), + (int) pmchild->pid))); + if (kill(pid, signal) < 0) elog(DEBUG3, "kill(%ld,%d) failed: %m", (long) pid, signal); #ifdef HAVE_SETSID @@ -3298,19 +3336,14 @@ signal_child(PMChild *pmchild, int signal) /* * Convenience function for killing a child process after a crash of some - * other child process. We log the action at a higher level than we would - * otherwise do, and we apply send_abort_for_crash to decide which signal - * to send. Normally it's SIGQUIT -- and most other comments in this file - * are written on the assumption that it is -- but developers might prefer - * to use SIGABRT to collect per-child core dumps. + * other child process. We apply send_abort_for_crash to decide which signal + * to send. Normally it's SIGQUIT -- and most other comments in this file are + * written on the assumption that it is -- but developers might prefer to use + * SIGABRT to collect per-child core dumps. */ static void sigquit_child(PMChild *pmchild) { - ereport(DEBUG2, - (errmsg_internal("sending %s to process %d", - (send_abort_for_crash ? "SIGABRT" : "SIGQUIT"), - (int) pmchild->pid))); signal_child(pmchild, (send_abort_for_crash ? SIGABRT : SIGQUIT)); } @@ -3342,9 +3375,6 @@ SignalChildren(int signal, BackendTypeMask targetMask) if (!btmask_contains(targetMask, bp->bkend_type)) continue; - ereport(DEBUG4, - (errmsg_internal("sending signal %d to %s process %d", - signal, GetBackendTypeDesc(bp->bkend_type), (int) bp->pid))); signal_child(bp, signal); signaled = true; } -- 2.39.5