diff options
author | Robert Haas <rhaas@postgresql.org> | 2015-03-19 10:56:34 -0400 |
---|---|---|
committer | Robert Haas <rhaas@postgresql.org> | 2015-03-19 11:08:54 -0400 |
commit | 76d07a2a0633836a82c43beb8ed3b08a64dc3d46 (patch) | |
tree | 1addda1116a05fa6da347f4327409857f6ee1f84 /src | |
parent | 904e8b627c2547e93112080aac9381bbf39e8e99 (diff) | |
download | postgresql-76d07a2a0633836a82c43beb8ed3b08a64dc3d46.tar.gz postgresql-76d07a2a0633836a82c43beb8ed3b08a64dc3d46.zip |
Fix status reporting for terminated bgworkers that were never started.
Previously, GetBackgroundWorkerPid() would return BGWH_NOT_YET_STARTED
if the slot used for the worker registration had not been reused by
unrelated activity, and BGWH_STOPPED if it had. Either way, a process
that had requested notification when the state of one of its
background workers changed did not receive such notifications. Fix
things so that GetBackgroundWorkerPid() always returns BGWH_STOPPED in
this situation, so that we do not erroneously give waiters the
impression that the worker will eventually be started; and send
notifications just as we would if the process terminated after having
been started, so that it's possible to wait for the postmaster to
process a worker termination request without polling.
Discovered by Amit Kapila during testing of parallel sequential scan.
Analysis and fix by me. Back-patch to 9.4; there may not be anyone
relying on this interface yet, but if anyone is, the new behavior is a
clear improvement.
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/postmaster/bgworker.c | 25 |
1 files changed, 24 insertions, 1 deletions
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 85a3b3a0773..c22066fad2a 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -245,14 +245,37 @@ BackgroundWorkerStateChange(void) rw->rw_terminate = true; if (rw->rw_pid != 0) kill(rw->rw_pid, SIGTERM); + else + { + /* Report never-started, now-terminated worker as dead. */ + ReportBackgroundWorkerPID(rw); + } } continue; } - /* If it's already flagged as do not restart, just release the slot. */ + /* + * If the worker is marked for termination, we don't need to add it + * to the registered workers list; we can just free the slot. + * However, if bgw_notify_pid is set, the process that registered the + * worker may need to know that we've processed the terminate request, + * so be sure to signal it. + */ if (slot->terminate) { + int notify_pid; + + /* + * We need a memory barrier here to make sure that the load of + * bgw_notify_pid completes before the store to in_use. + */ + notify_pid = slot->worker.bgw_notify_pid; + pg_memory_barrier(); + slot->pid = 0; slot->in_use = false; + if (notify_pid != 0) + kill(notify_pid, SIGUSR1); + continue; } |