aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorRobert Haas <rhaas@postgresql.org>2015-10-09 14:31:04 -0400
committerRobert Haas <rhaas@postgresql.org>2015-10-09 14:31:04 -0400
commitdb0f6cad4884bd4c835156d3a720d9a79dbd63a9 (patch)
treee16029e0e2a001eb34096daeb3c8100e2afb0a1d /src
parentb7aac36245261eba9eb7d18561ce44220b361959 (diff)
downloadpostgresql-db0f6cad4884bd4c835156d3a720d9a79dbd63a9.tar.gz
postgresql-db0f6cad4884bd4c835156d3a720d9a79dbd63a9.zip
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.
Diffstat (limited to 'src')
-rw-r--r--src/backend/postmaster/bgworker.c82
-rw-r--r--src/backend/storage/ipc/procsignal.c11
-rw-r--r--src/backend/storage/ipc/shm_mq.c76
-rw-r--r--src/backend/tcop/postgres.c4
-rw-r--r--src/include/storage/procsignal.h1
-rw-r--r--src/test/modules/test_shm_mq/setup.c65
6 files changed, 86 insertions, 153 deletions
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 68c9505809e..c38d486a20e 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -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;
+ pid_t pid;
- CHECK_FOR_INTERRUPTS();
+ 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);
+ status = GetBackgroundWorkerPid(handle, &pid);
+ if (status == BGWH_STARTED)
+ *pidp = pid;
+ if (status != BGWH_NOT_YET_STARTED)
+ break;
- if (rc & WL_POSTMASTER_DEATH)
- {
- status = BGWH_POSTMASTER_DIED;
- break;
- }
+ rc = WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_POSTMASTER_DEATH, 0);
- ResetLatch(MyLatch);
+ if (rc & WL_POSTMASTER_DEATH)
+ {
+ status = BGWH_POSTMASTER_DIED;
+ break;
}
+
+ ResetLatch(MyLatch);
}
- 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;
}
@@ -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);
- }
- }
- PG_CATCH();
- {
- set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
- PG_RE_THROW();
+ ResetLatch(&MyProc->procLatch);
}
- PG_END_TRY();
- set_latch_on_sigusr1 = save_set_latch_on_sigusr1;
return status;
}
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 0abde43565b..acfb4e9be82 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -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();
diff --git a/src/backend/storage/ipc/shm_mq.c b/src/backend/storage/ipc/shm_mq.c
index c78f1650e6a..80956ce4304 100644
--- a/src/backend/storage/ipc/shm_mq.c
+++ b/src/backend/storage/ipc/shm_mq.c
@@ -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)
+ {
+ result = false;
+ break;
+ }
+ if (result)
+ break;
+
+ if (handle != NULL)
{
- 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)
+ /* 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);
+ /* Wait to be signalled. */
+ WaitLatch(MyLatch, WL_LATCH_SET, 0);
- /* An interrupt may have occurred while we were waiting. */
- CHECK_FOR_INTERRUPTS();
+ /* 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();
+ /* Reset the latch so we don't spin. */
+ ResetLatch(MyLatch);
}
- PG_END_TRY();
return result;
}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index aee13aec757..d30fe35c14f 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -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);
diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h
index af1a0cd71f2..50ffb13e1f2 100644
--- a/src/include/storage/procsignal.h
+++ b/src/include/storage/procsignal.h
@@ -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 */
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index 7f2f5fd3ff7..dedd72f8838 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -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,