From 792d4986844c6cd310d19ec2ecb4eafba7b60e1f Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Thu, 13 Aug 2026 16:37:01 +0200 Subject: [PATCH v2 1/2] Make data checksums launcher cancel its worker at SIGINT Make sure the launcher cancels the currently running worker by calling TerminateBackgroundWorker when it receives SIGINT. In order to handle cases where SIGINT arrives while tje launcher is waiting for a worker to start or exit, implement a version of WaitForBackgroundWorkerStartup and WaitForBackgroundWorkerShutdown which checks the abort_requested signalling. Backpatch to v19 where online checksums were introduced. Author: Fujii Masao Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CAHGQGwEQ1-+iPQnUpTXYiHmzSz9ufFVkOK4kL_uyTdYt7jgg0Q@mail.gmail.com Backpatch-through: 19 --- src/backend/postmaster/datachecksum_state.c | 112 ++++++++++++++++++-- 1 file changed, 101 insertions(+), 11 deletions(-) diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c index be308a282d5..7dfe7c812bb 100644 --- a/src/backend/postmaster/datachecksum_state.c +++ b/src/backend/postmaster/datachecksum_state.c @@ -393,6 +393,10 @@ static DataChecksumsWorkerResult ProcessDatabase(DataChecksumsWorkerDatabase *db static bool ProcessAllDatabases(void); static bool ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrategy strategy); static void ResetDataChecksumsProgressCounters(void); +static BgwHandleStatus WaitForDataChecksumsWorkerState(BackgroundWorkerHandle *handle, + bool wait_for_startup, + pid_t *pidp, + uint32 wait_event); static void launcher_cancel_handler(SIGNAL_ARGS); static void WaitForAllTransactionsToFinish(void); @@ -861,6 +865,73 @@ ProcessSingleRelationByOid(Oid relationId, BufferAccessStrategy strategy) return !aborted; } +/* + * WaitForDataChecksumsWorkerState + * Wait for a data checksums worker to start or stop. + * + * This is like WaitForBackgroundWorkerStartup() and + * WaitForBackgroundWorkerShutdown(), except that it also reacts to SIGINT + * received by the launcher. The launcher owns the overall checksum + * operation, so canceling it should stop the worker it has registered or is + * currently running. + * + * If wait_for_startup is true, wait until the worker is no longer in + * BGWH_NOT_YET_STARTED state, like WaitForBackgroundWorkerStartup(). If it + * is false, wait until the worker reaches BGWH_STOPPED state, like + * WaitForBackgroundWorkerShutdown(). + * + * pidp is set to the worker's PID when startup succeeds, if it is not NULL. + */ +static BgwHandleStatus +WaitForDataChecksumsWorkerState(BackgroundWorkerHandle *handle, + bool wait_for_startup, + pid_t *pidp, + uint32 wait_event) +{ + BgwHandleStatus status; + bool termination_requested = false; + + for (;;) + { + int rc; + pid_t pid; + + CHECK_FOR_INTERRUPTS(); + + status = GetBackgroundWorkerPid(handle, &pid); + if (status == BGWH_STARTED && pidp) + *pidp = pid; + + if (abort_requested && !termination_requested) + { + TerminateBackgroundWorker(handle); + termination_requested = true; + } + + /* + * Startup waits for the worker to leave BGWH_NOT_YET_STARTED, while + * shutdown waits for it to reach BGWH_STOPPED. + */ + if (status == BGWH_STOPPED || + (wait_for_startup && status == BGWH_STARTED)) + break; + + rc = WaitLatch(MyLatch, + WL_LATCH_SET | WL_POSTMASTER_DEATH, 0, + wait_event); + + if (rc & WL_POSTMASTER_DEATH) + { + status = BGWH_POSTMASTER_DIED; + break; + } + + ResetLatch(MyLatch); + } + + return status; +} + /* * ProcessDatabase * Enable data checksums in a single database. @@ -921,9 +992,20 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db) return DATACHECKSUMSWORKER_FAILED; } - status = WaitForBackgroundWorkerStartup(bgw_handle, &pid); + /* + * While this expects to wait for BGWORKER_STARTUP it may return _STOPPED + * if the worker was terminated in the meantime so we must check status. + */ + status = WaitForDataChecksumsWorkerState(bgw_handle, true, &pid, + WAIT_EVENT_BGWORKER_STARTUP); if (status == BGWH_STOPPED) { + if (abort_requested) + { + result = DATACHECKSUMSWORKER_ABORTED; + goto done; + } + /* * If the worker managed to start, and stop, before we got to waiting * for it we can see a STOPPED status here without it being a failure. @@ -981,7 +1063,8 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db) "Waiting for worker in database %s (pid %ld)", db->dbname, (long) pid); pgstat_report_activity(STATE_RUNNING, activity); - status = WaitForBackgroundWorkerShutdown(bgw_handle); + status = WaitForDataChecksumsWorkerState(bgw_handle, false, NULL, + WAIT_EVENT_BGWORKER_SHUTDOWN); if (status == BGWH_POSTMASTER_DIED) ereport(FATAL, errcode(ERRCODE_ADMIN_SHUTDOWN), @@ -1004,6 +1087,11 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db) if (result == DATACHECKSUMSWORKER_FAILED && !DatabaseExists(db->dboid)) result = DATACHECKSUMSWORKER_DROPDB; + CHECK_FOR_LAUNCHER_ABORT_REQUEST(); + if (abort_requested) + result = DATACHECKSUMSWORKER_ABORTED; + +done: if (result == DATACHECKSUMSWORKER_ABORTED) ereport(LOG, errmsg("data checksums processing was aborted in database \"%s\"", @@ -1056,9 +1144,9 @@ launcher_exit(int code, Datum arg) /* * launcher_cancel_handler * - * Internal routine for reacting to SIGINT and flagging the worker to abort. - * The worker won't be interrupted immediately but will check for abort flag - * between each block in a relation. + * Internal routine for reacting to SIGINT and flagging the launcher to abort. + * If a worker is registered or running, the launcher will request worker + * termination from its normal control flow. */ static void launcher_cancel_handler(SIGNAL_ARGS) @@ -1068,10 +1156,8 @@ launcher_cancel_handler(SIGNAL_ARGS) abort_requested = true; /* - * There is no sleeping in the main loop, the flag will be checked - * periodically in ProcessSingleRelationFork. The worker does however - * sleep when waiting for concurrent transactions to end so we still need - * to set the latch. + * Wake the launcher if it is waiting for transactions to finish or for a + * worker to start up or shut down. */ SetLatch(MyLatch); @@ -1226,8 +1312,9 @@ again: if (!ProcessAllDatabases()) { /* - * If the target state changed during processing then it's not a - * failure, so restart processing instead. + * If processing was canceled, or the target state changed during + * processing, then it's not a failure. In the latter case, the + * launcher will restart processing with the new target state. */ CHECK_FOR_LAUNCHER_ABORT_REQUEST(); if (abort_requested) @@ -1314,6 +1401,8 @@ ProcessAllDatabases(void) /* Get a list of all databases to process */ WaitForAllTransactionsToFinish(); + if (abort_requested) + return false; DatabaseList = BuildDatabaseList(); /* @@ -1370,6 +1459,7 @@ ProcessAllDatabases(void) else if (result == DATACHECKSUMSWORKER_ABORTED || abort_requested) { /* Abort flag set, so exit the whole process */ + FreeDatabaseList(DatabaseList); return false; } else if (result == DATACHECKSUMSWORKER_DROPDB) -- 2.39.3 (Apple Git-146)