From 28652f322f7a345a0c06cc46a9d5659afdc7f62f Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 7 Jan 2025 20:22:36 -0500 Subject: [PATCH v2 2/7] 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 Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp --- src/backend/postmaster/postmaster.c | 55 ++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index de278b0850d..51ec097cd43 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 @@ -3255,6 +3255,37 @@ LaunchMissingBackgroundProcesses(void) maybe_start_bgworkers(); } +/* + * Return string representation of signal. + * + * Because this is only implemented in 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(state) case state: return #state + 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 + pg_unreachable(); +} + /* * Send a signal to a postmaster child process * @@ -3276,6 +3307,12 @@ signal_child(PMChild *pmchild, int signal) { pid_t pid = pmchild->pid; + ereport(DEBUG3, + (errmsg_internal("sending signal %d/%s to %s process %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 @@ -3297,19 +3334,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)); } @@ -3341,9 +3373,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.45.2.746.g06e570c0df.dirty