rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove set_latch_on_sigusr1 flag. 

This flag has proven to be a recipe for bugs, and it doesn't seem like
it can really buy anything in terms of performance.  So let's just
*always* set the process latch when we receive SIGUSR1 instead of
trying to do it only when needed.

Per my recent proposal on pgsql-hackers.
This commit is contained in:
Robert Haas 2015-10-09 14:31:04 -04:00
parent b7aac36245
commit db0f6cad48
6 changed files with 91 additions and 158 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
src/backend/postmaster/bgworker.c
View File

@ -954,45 +954,31 @@ WaitForBackgroundWorkerStartup(BackgroundWorkerHandle *handle, pid_t *pidp)
{
BgwHandleStatus status;
int rc;
bool save_set_latch_on_sigusr1;
save_set_latch_on_sigusr1 = set_latch_on_sigusr1;
set_latch_on_sigusr1 = true;
PG_TRY();
for (;;)
{
for (;;)
pid_t pid;
CHECK_FOR_INTERRUPTS();
status = GetBackgroundWorkerPid(handle, &pid);
if (status == BGWH_STARTED)
*pidp = pid;
if (status != BGWH_NOT_YET_STARTED)
break;
rc = WaitLatch(MyLatch,
WL_LATCH_SET | WL_POSTMASTER_DEATH, 0);
if (rc & WL_POSTMASTER_DEATH)
{
pid_t pid;
CHECK_FOR_INTERRUPTS();
status = GetBackgroundWorkerPid(handle, &pid);
if (status == BGWH_STARTED)
*pidp = pid;
if (status != BGWH_NOT_YET_STARTED)
break;
rc = WaitLatch(MyLatch,
WL_LATCH_SET | WL_POSTMASTER_DEATH, 0);
if (rc & WL_POSTMASTER_DEATH)
{
status = BGWH_POSTMASTER_DIED;
break;
}
ResetLatch(MyLatch);
status = BGWH_POSTMASTER_DIED;
break;
}
}
PG_CATCH();
{
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
PG_RE_THROW();
}
PG_END_TRY();
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
ResetLatch(MyLatch);
}
return status;
}
@ -1009,40 +995,26 @@ WaitForBackgroundWorkerShutdown(BackgroundWorkerHandle *handle)
{
BgwHandleStatus status;
int rc;
bool save_set_latch_on_sigusr1;
save_set_latch_on_sigusr1 = set_latch_on_sigusr1;
set_latch_on_sigusr1 = true;
PG_TRY();
for (;;)
{
for (;;)
{
pid_t pid;
pid_t pid;
CHECK_FOR_INTERRUPTS();
CHECK_FOR_INTERRUPTS();
status = GetBackgroundWorkerPid(handle, &pid);
if (status == BGWH_STOPPED)
return status;
status = GetBackgroundWorkerPid(handle, &pid);
if (status == BGWH_STOPPED)
return status;
rc = WaitLatch(&MyProc->procLatch,
WL_LATCH_SET | WL_POSTMASTER_DEATH, 0);
rc = WaitLatch(&MyProc->procLatch,
WL_LATCH_SET | WL_POSTMASTER_DEATH, 0);
if (rc & WL_POSTMASTER_DEATH)
return BGWH_POSTMASTER_DIED;
if (rc & WL_POSTMASTER_DEATH)
return BGWH_POSTMASTER_DIED;
ResetLatch(&MyProc->procLatch);
}
ResetLatch(&MyProc->procLatch);
}
PG_CATCH();
{
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
PG_RE_THROW();
}
PG_END_TRY();
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
return status;
}

11
src/backend/storage/ipc/procsignal.c
View File

@ -59,14 +59,6 @@ typedef struct
*/
#define NumProcSignalSlots (MaxBackends + NUM_AUXPROCTYPES)
/*
* If this flag is set, the process latch will be set whenever SIGUSR1
* is received. This is useful when waiting for a signal from the postmaster.
* Spurious wakeups must be expected. Make sure that the flag is cleared
* in the error path.
*/
bool set_latch_on_sigusr1;
static ProcSignalSlot *ProcSignalSlots = NULL;
static volatile ProcSignalSlot *MyProcSignalSlot = NULL;
@ -296,8 +288,7 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN))
RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
if (set_latch_on_sigusr1)
SetLatch(MyLatch);
SetLatch(MyLatch);
latch_sigusr1_handler();

78
src/backend/storage/ipc/shm_mq.c
View File

@ -962,63 +962,49 @@ static bool
shm_mq_wait_internal(volatile shm_mq *mq, PGPROC *volatile * ptr,
BackgroundWorkerHandle *handle)
{
bool save_set_latch_on_sigusr1;
bool result = false;
save_set_latch_on_sigusr1 = set_latch_on_sigusr1;
if (handle != NULL)
set_latch_on_sigusr1 = true;
PG_TRY();
for (;;)
{
for (;;)
BgwHandleStatus status;
pid_t pid;
bool detached;
/* Acquire the lock just long enough to check the pointer. */
SpinLockAcquire(&mq->mq_mutex);
detached = mq->mq_detached;
result = (*ptr != NULL);
SpinLockRelease(&mq->mq_mutex);
/* Fail if detached; else succeed if initialized. */
if (detached)
{
BgwHandleStatus status;
pid_t pid;
bool detached;
result = false;
break;
}
if (result)
break;
/* Acquire the lock just long enough to check the pointer. */
SpinLockAcquire(&mq->mq_mutex);
detached = mq->mq_detached;
result = (*ptr != NULL);
SpinLockRelease(&mq->mq_mutex);
/* Fail if detached; else succeed if initialized. */
if (detached)
if (handle != NULL)
{
/* Check for unexpected worker death. */
status = GetBackgroundWorkerPid(handle, &pid);
if (status != BGWH_STARTED && status != BGWH_NOT_YET_STARTED)
{
result = false;
break;
}
if (result)
break;
if (handle != NULL)
{
/* Check for unexpected worker death. */
status = GetBackgroundWorkerPid(handle, &pid);
if (status != BGWH_STARTED && status != BGWH_NOT_YET_STARTED)
{
result = false;
break;
}
}
/* Wait to be signalled. */
WaitLatch(MyLatch, WL_LATCH_SET, 0);
/* An interrupt may have occurred while we were waiting. */
CHECK_FOR_INTERRUPTS();
/* Reset the latch so we don't spin. */
ResetLatch(MyLatch);
}
/* Wait to be signalled. */
WaitLatch(MyLatch, WL_LATCH_SET, 0);
/* An interrupt may have occurred while we were waiting. */
CHECK_FOR_INTERRUPTS();
/* Reset the latch so we don't spin. */
ResetLatch(MyLatch);
}
PG_CATCH();
{
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
PG_RE_THROW();
}
PG_END_TRY();
return result;
}

4
src/backend/tcop/postgres.c
View File

@ -2825,9 +2825,7 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
/*
* Set the process latch. This function essentially emulates signal
* handlers like die() and StatementCancelHandler() and it seems prudent
* to behave similarly as they do. Alternatively all plain backend code
* waiting on that latch, expecting to get interrupted by query cancels et
* al., would also need to set set_latch_on_sigusr1.
* to behave similarly as they do.
*/
SetLatch(MyLatch);

1
src/include/storage/procsignal.h
View File

@ -55,6 +55,5 @@ extern int SendProcSignal(pid_t pid, ProcSignalReason reason,
BackendId backendId);
extern void procsignal_sigusr1_handler(SIGNAL_ARGS);
extern PGDLLIMPORT bool set_latch_on_sigusr1;
#endif /* PROCSIGNAL_H */

65
src/test/modules/test_shm_mq/setup.c
View File

@ -255,51 +255,38 @@ static void
wait_for_workers_to_become_ready(worker_state *wstate,
volatile test_shm_mq_header *hdr)
{
bool save_set_latch_on_sigusr1;
bool result = false;
save_set_latch_on_sigusr1 = set_latch_on_sigusr1;
set_latch_on_sigusr1 = true;
PG_TRY();
for (;;)
{
for (;;)
int workers_ready;
/* If all the workers are ready, we have succeeded. */
SpinLockAcquire(&hdr->mutex);
workers_ready = hdr->workers_ready;
SpinLockRelease(&hdr->mutex);
if (workers_ready >= wstate->nworkers)
{
int workers_ready;
/* If all the workers are ready, we have succeeded. */
SpinLockAcquire(&hdr->mutex);
workers_ready = hdr->workers_ready;
SpinLockRelease(&hdr->mutex);
if (workers_ready >= wstate->nworkers)
{
result = true;
break;
}
/* If any workers (or the postmaster) have died, we have failed. */
if (!check_worker_status(wstate))
{
result = false;
break;
}
/* Wait to be signalled. */
WaitLatch(MyLatch, WL_LATCH_SET, 0);
/* An interrupt may have occurred while we were waiting. */
CHECK_FOR_INTERRUPTS();
/* Reset the latch so we don't spin. */
ResetLatch(MyLatch);
result = true;
break;
}
/* If any workers (or the postmaster) have died, we have failed. */
if (!check_worker_status(wstate))
{
result = false;
break;
}
/* Wait to be signalled. */
WaitLatch(MyLatch, WL_LATCH_SET, 0);
/* An interrupt may have occurred while we were waiting. */
CHECK_FOR_INTERRUPTS();
/* Reset the latch so we don't spin. */
ResetLatch(MyLatch);
}
PG_CATCH();
{
set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
PG_RE_THROW();
}
PG_END_TRY();
if (!result)
ereport(ERROR,