mirror of
				https://github.com/postgres/postgres.git
				synced 2025-11-03 09:13:20 +03:00 
			
		
		
		
	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.
This commit is contained in:
		@@ -245,14 +245,37 @@ BackgroundWorkerStateChange(void)
 | 
				
			|||||||
				rw->rw_terminate = true;
 | 
									rw->rw_terminate = true;
 | 
				
			||||||
				if (rw->rw_pid != 0)
 | 
									if (rw->rw_pid != 0)
 | 
				
			||||||
					kill(rw->rw_pid, SIGTERM);
 | 
										kill(rw->rw_pid, SIGTERM);
 | 
				
			||||||
 | 
									else
 | 
				
			||||||
 | 
									{
 | 
				
			||||||
 | 
										/* Report never-started, now-terminated worker as dead. */
 | 
				
			||||||
 | 
										ReportBackgroundWorkerPID(rw);
 | 
				
			||||||
 | 
									}
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			continue;
 | 
								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)
 | 
							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;
 | 
								slot->in_use = false;
 | 
				
			||||||
 | 
								if (notify_pid != 0)
 | 
				
			||||||
 | 
									kill(notify_pid, SIGUSR1);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			continue;
 | 
								continue;
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user