From ae9d3e8646a250bc93fc50bded359c703baa9977 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 1 Aug 2024 17:24:12 +0300 Subject: [PATCH v3 08/12] Introduce a separate BackendType for dead-end children And replace postmaster.c's own "backend type" codes with BackendType XXX: While working on this, many times I accidentally did something like "foo |= B_SOMETHING" instead of "foo |= 1 << B_SOMETHING", when constructing arguments to SignalSomeChildren or CountChildren, and things broke in very subtle ways taking a long time to debug. The old constants that were already bitmasks avoided that. Maybe we need some macro magic or something to make this less error-prone. --- src/backend/postmaster/postmaster.c | 106 ++++++++++++------------- src/backend/utils/activity/pgstat_io.c | 3 + src/backend/utils/init/miscinit.c | 3 + src/include/miscadmin.h | 1 + 4 files changed, 56 insertions(+), 57 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index d9a2783fb6..2e917c5ea9 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -129,15 +129,11 @@ /* - * Possible types of a backend. Beyond being the possible bkend_type values in - * struct bkend, these are OR-able request flag bits for SignalSomeChildren() - * and CountChildren(). + * CountChildren and SignalSomeChildren use a uint32 bitmask argument to + * represent BackendTypes to count or signal. */ -#define BACKEND_TYPE_NORMAL 0x0001 /* normal backend */ -#define BACKEND_TYPE_AUTOVAC 0x0002 /* autovacuum worker process */ -#define BACKEND_TYPE_WALSND 0x0004 /* walsender process */ -#define BACKEND_TYPE_BGWORKER 0x0008 /* bgworker process */ -#define BACKEND_TYPE_ALL 0x000F /* OR of all the above */ +#define BACKEND_TYPE_ALL 0xffffffff +StaticAssertDecl(BACKEND_NUM_TYPES < 32, "too many backend types for uint32"); /* * List of active backends (or child processes anyway; we don't actually @@ -148,7 +144,7 @@ * As shown in the above set of backend types, this list includes not only * "normal" client sessions, but also autovacuum workers, walsenders, and * background workers. (Note that at the time of launch, walsenders are - * labeled BACKEND_TYPE_NORMAL; we relabel them to BACKEND_TYPE_WALSND + * labeled B_BACKEND; we relabel them to B_WAL_SENDER * upon noticing they've changed their PMChildFlags entry. Hence that check * must be done before any operation that needs to distinguish walsenders * from normal backends.) @@ -157,7 +153,8 @@ * the purpose of sending a friendly rejection message to a would-be client. * We must track them because they are attached to shared memory, but we know * they will never become live backends. dead_end children are not assigned a - * PMChildSlot. dead_end children have bkend_type NORMAL. + * PMChildSlot. dead_end children have bkend_type B_DEAD_END_BACKEND. + * FIXME: a dead-end backend can send query cancel? * * "Special" children such as the startup, bgwriter, autovacuum launcher, and * slot sync worker tasks are not in this list. They are tracked via StartupPID @@ -169,8 +166,7 @@ typedef struct bkend { pid_t pid; /* process id of backend */ int child_slot; /* PMChildSlot for this backend, if any */ - int bkend_type; /* child process flavor, see above */ - bool dead_end; /* is it going to send an error and quit? */ + BackendType bkend_type; /* child process flavor, see above */ RegisteredBgWorker *rw; /* bgworker info, if this is a bgworker */ bool bgworker_notify; /* gets bgworker start/stop notifications */ dlist_node elem; /* list link in BackendList */ @@ -410,12 +406,13 @@ static void report_fork_failure_to_client(ClientSocket *client_sock, int errnum) static CAC_state canAcceptConnections(int backend_type); static void signal_child(pid_t pid, int signal); static void sigquit_child(pid_t pid); -static bool SignalSomeChildren(int signal, int target); +static bool SignalSomeChildren(int signal, uint32 targetMask); static void TerminateChildren(int signal); -#define SignalChildren(sig) SignalSomeChildren(sig, BACKEND_TYPE_ALL) +#define SignalChildren(sig) \ + SignalSomeChildren(sig, BACKEND_TYPE_ALL & ~(1 << B_DEAD_END_BACKEND)) -static int CountChildren(int target); +static int CountChildren(uint32 targetMask); static Backend *assign_backendlist_entry(void); static void LaunchMissingBackgroundProcesses(void); static void maybe_start_bgworkers(void); @@ -1765,7 +1762,7 @@ canAcceptConnections(int backend_type) * bgworker_should_start_now() decided whether the DB state allows them. */ if (pmState != PM_RUN && pmState != PM_HOT_STANDBY && - backend_type != BACKEND_TYPE_BGWORKER) + backend_type != B_BG_WORKER) { if (Shutdown > NoShutdown) return CAC_SHUTDOWN; /* shutdown is pending */ @@ -1782,7 +1779,7 @@ canAcceptConnections(int backend_type) * "Smart shutdown" restrictions are applied only to normal connections, * not to autovac workers or bgworkers. */ - if (!connsAllowed && backend_type == BACKEND_TYPE_NORMAL) + if (!connsAllowed && backend_type == B_BACKEND) return CAC_SHUTDOWN; /* shutdown is pending */ /* @@ -1797,7 +1794,7 @@ canAcceptConnections(int backend_type) * The limit here must match the sizes of the per-child-process arrays; * see comments for MaxLivePostmasterChildren(). */ - if (CountChildren(BACKEND_TYPE_ALL) >= MaxLivePostmasterChildren()) + if (CountChildren(BACKEND_TYPE_ALL & ~(1 << B_DEAD_END_BACKEND)) >= MaxLivePostmasterChildren()) result = CAC_TOOMANY; return result; @@ -2554,11 +2551,11 @@ CleanupBackend(Backend *bp, bool crashed = false; /* Construct a process name for log message */ - if (bp->dead_end) + if (bp->bkend_type == B_DEAD_END_BACKEND) { procname = _("dead end backend"); } - else if (bp->bkend_type == BACKEND_TYPE_BGWORKER) + else if (bp->bkend_type == B_BG_WORKER) { snprintf(namebuf, MAXPGPATH, _("background worker \"%s\""), bp->rw->rw_worker.bgw_type); @@ -2596,7 +2593,7 @@ CleanupBackend(Backend *bp, * If the process attached to shared memory, check that it detached * cleanly. */ - if (!bp->dead_end) + if (bp->bkend_type != B_DEAD_END_BACKEND) { if (!ReleasePostmasterChildSlot(bp->child_slot)) { @@ -2628,7 +2625,7 @@ CleanupBackend(Backend *bp, /* * If it was a background worker, also update its RegisteredWorker entry. */ - if (bp->bkend_type == BACKEND_TYPE_BGWORKER) + if (bp->bkend_type == B_BG_WORKER) { RegisteredBgWorker *rw = bp->rw; @@ -2851,7 +2848,7 @@ PostmasterStateMachine(void) * This state ends when we have no normal client backends running. * Then we're ready to stop other children. */ - if (CountChildren(BACKEND_TYPE_NORMAL) == 0) + if (CountChildren(1 << B_BACKEND) == 0) pmState = PM_STOP_BACKENDS; } } @@ -2872,7 +2869,7 @@ PostmasterStateMachine(void) /* Signal all backend children except walsenders */ SignalSomeChildren(SIGTERM, - BACKEND_TYPE_ALL - BACKEND_TYPE_WALSND); + BACKEND_TYPE_ALL & ~(1 << B_WAL_SENDER | 1 << B_DEAD_END_BACKEND)); /* and the autovac launcher too */ if (AutoVacPID != 0) signal_child(AutoVacPID, SIGTERM); @@ -2914,7 +2911,7 @@ PostmasterStateMachine(void) * here. Walsenders and archiver are also disregarded, they will be * terminated later after writing the checkpoint record. */ - if (CountChildren(BACKEND_TYPE_ALL - BACKEND_TYPE_WALSND) == 0 && + if (CountChildren(BACKEND_TYPE_ALL & ~(1 << B_WAL_SENDER | 1 << B_DEAD_END_BACKEND)) == 0 && StartupPID == 0 && WalReceiverPID == 0 && WalSummarizerPID == 0 && @@ -2988,7 +2985,7 @@ PostmasterStateMachine(void) * left by now anyway; what we're really waiting for is walsenders and * archiver. */ - if (PgArchPID == 0 && CountChildren(BACKEND_TYPE_ALL) == 0) + if (PgArchPID == 0 && CountChildren(BACKEND_TYPE_ALL & ~(1 << B_DEAD_END_BACKEND)) == 0) { pmState = PM_WAIT_DEAD_END; } @@ -3285,10 +3282,10 @@ sigquit_child(pid_t pid) /* * Send a signal to the targeted children (but NOT special children; - * dead_end children are never signaled, either). + * dead_end children are never signaled, either XXX). */ static bool -SignalSomeChildren(int signal, int target) +SignalSomeChildren(int signal, uint32 targetMask) { dlist_iter iter; bool signaled = false; @@ -3297,24 +3294,21 @@ SignalSomeChildren(int signal, int target) { Backend *bp = dlist_container(Backend, elem, iter.cur); - if (bp->dead_end) - continue; - /* * Since target == BACKEND_TYPE_ALL is the most common case, we test * it first and avoid touching shared memory for every child. */ - if (target != BACKEND_TYPE_ALL) + if (targetMask != BACKEND_TYPE_ALL) { /* * Assign bkend_type for any recently announced WAL Sender * processes. */ - if (bp->bkend_type == BACKEND_TYPE_NORMAL && + if (bp->bkend_type == B_BACKEND && IsPostmasterChildWalSender(bp->child_slot)) - bp->bkend_type = BACKEND_TYPE_WALSND; + bp->bkend_type = B_WAL_SENDER; - if (!(target & bp->bkend_type)) + if ((targetMask & (1 << bp->bkend_type)) == 0) continue; } @@ -3387,17 +3381,22 @@ BackendStartup(ClientSocket *client_sock) } /* Pass down canAcceptConnections state */ - startup_data.canAcceptConnections = canAcceptConnections(BACKEND_TYPE_NORMAL); - bn->dead_end = (startup_data.canAcceptConnections != CAC_OK); + startup_data.canAcceptConnections = canAcceptConnections(B_BACKEND); bn->rw = NULL; /* * Unless it's a dead_end child, assign it a child slot number */ - if (!bn->dead_end) + if (startup_data.canAcceptConnections == CAC_OK) + { + bn->bkend_type = B_BACKEND; /* Can change later to WALSND */ bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); + } else + { + bn->bkend_type = B_DEAD_END_BACKEND; bn->child_slot = 0; + } /* Hasn't asked to be notified about any bgworkers yet */ bn->bgworker_notify = false; @@ -3410,7 +3409,7 @@ BackendStartup(ClientSocket *client_sock) /* in parent, fork failed */ int save_errno = errno; - if (!bn->dead_end) + if (bn->child_slot != 0) (void) ReleasePostmasterChildSlot(bn->child_slot); pfree(bn); errno = save_errno; @@ -3430,7 +3429,6 @@ BackendStartup(ClientSocket *client_sock) * of backends. */ bn->pid = pid; - bn->bkend_type = BACKEND_TYPE_NORMAL; /* Can change later to WALSND */ dlist_push_head(&BackendList, &bn->elem); return STATUS_OK; @@ -3664,11 +3662,10 @@ dummy_handler(SIGNAL_ARGS) } /* - * Count up number of child processes of specified types (dead_end children - * are always excluded). + * Count up number of child processes of specified types. */ static int -CountChildren(int target) +CountChildren(uint32 targetMask) { dlist_iter iter; int cnt = 0; @@ -3677,24 +3674,21 @@ CountChildren(int target) { Backend *bp = dlist_container(Backend, elem, iter.cur); - if (bp->dead_end) - continue; - /* * Since target == BACKEND_TYPE_ALL is the most common case, we test * it first and avoid touching shared memory for every child. */ - if (target != BACKEND_TYPE_ALL) + if (targetMask != BACKEND_TYPE_ALL) { /* * Assign bkend_type for any recently announced WAL Sender * processes. */ - if (bp->bkend_type == BACKEND_TYPE_NORMAL && + if (bp->bkend_type == B_BACKEND && IsPostmasterChildWalSender(bp->child_slot)) - bp->bkend_type = BACKEND_TYPE_WALSND; + bp->bkend_type = B_WAL_SENDER; - if (!(target & bp->bkend_type)) + if ((targetMask & (1 << bp->bkend_type)) == 0) continue; } @@ -3761,13 +3755,13 @@ StartAutovacuumWorker(void) * we have to check to avoid race-condition problems during DB state * changes. */ - if (canAcceptConnections(BACKEND_TYPE_AUTOVAC) == CAC_OK) + if (canAcceptConnections(B_AUTOVAC_WORKER) == CAC_OK) { bn = (Backend *) palloc_extended(sizeof(Backend), MCXT_ALLOC_NO_OOM); if (bn) { - /* Autovac workers are not dead_end and need a child slot */ - bn->dead_end = false; + /* Autovac workers need a child slot */ + bn->bkend_type = B_AUTOVAC_WORKER; bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); bn->bgworker_notify = false; bn->rw = NULL; @@ -3775,7 +3769,6 @@ StartAutovacuumWorker(void) bn->pid = StartChildProcess(B_AUTOVAC_WORKER); if (bn->pid > 0) { - bn->bkend_type = BACKEND_TYPE_AUTOVAC; dlist_push_head(&BackendList, &bn->elem); /* all OK */ return; @@ -3981,7 +3974,7 @@ assign_backendlist_entry(void) * only possible failure is CAC_TOOMANY, so we just log an error message * based on that rather than checking the error code precisely. */ - if (canAcceptConnections(BACKEND_TYPE_BGWORKER) != CAC_OK) + if (canAcceptConnections(B_BG_WORKER) != CAC_OK) { ereport(LOG, (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED), @@ -3999,8 +3992,7 @@ assign_backendlist_entry(void) } bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); - bn->bkend_type = BACKEND_TYPE_BGWORKER; - bn->dead_end = false; + bn->bkend_type = B_BG_WORKER; bn->bgworker_notify = false; return bn; diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 8af55989ee..9bad1040d6 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -312,6 +312,8 @@ pgstat_io_snapshot_cb(void) * * The following BackendTypes do not participate in the cumulative stats * subsystem or do not perform IO on which we currently track: +* - Dead-end backend because it is not connected to shared memory and +* doesn't do any IO * - Syslogger because it is not connected to shared memory * - Archiver because most relevant archiving IO is delegated to a * specialized command or module @@ -334,6 +336,7 @@ pgstat_tracks_io_bktype(BackendType bktype) switch (bktype) { case B_INVALID: + case B_DEAD_END_BACKEND: case B_ARCHIVER: case B_LOGGER: case B_WAL_RECEIVER: diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 537d92c0cf..ae8b1a4331 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -281,6 +281,9 @@ GetBackendTypeDesc(BackendType backendType) case B_BACKEND: backendDesc = "client backend"; break; + case B_DEAD_END_BACKEND: + backendDesc = "dead-end client backend"; + break; case B_BG_WORKER: backendDesc = "background worker"; break; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index ac16233b71..b21c4d43b9 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -337,6 +337,7 @@ typedef enum BackendType /* Backends and other backend-like processes */ B_BACKEND, + B_DEAD_END_BACKEND, B_AUTOVAC_LAUNCHER, B_AUTOVAC_WORKER, B_BG_WORKER, -- 2.39.2