Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9206ced

Browse files
anarazelanarazel
committed
Clean up latch related code.
The larger part of this patch replaces usages of MyProc->procLatch with MyLatch. The latter works even early during backend startup, where MyProc->procLatch doesn't yet. While the affected code shouldn't run in cases where it's not initialized, it might get copied into places where it might. Using MyLatch is simpler and a bit faster to boot, so there's little point to stick with the previous coding. While doing so I noticed some weaknesses around newly introduced uses of latches that could lead to missed events, and an omitted CHECK_FOR_INTERRUPTS() call in worker_spi. As all the actual bugs are in v10 code, there doesn't seem to be sufficient reason to backpatch this. Author: Andres Freund Discussion: https://postgr.es/m/20170606195321.sjmenrfgl2nu6j63@alap3.anarazel.de https://postgr.es/m/20170606210405.sim3yl6vpudhmufo@alap3.anarazel.de Backpatch: -
1 parent e3a815d commit 9206ced

File tree

9 files changed

+56
-34
lines changed

9 files changed

+56
-34
lines changed

‎src/backend/access/transam/parallel.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,9 +527,9 @@ WaitForParallelWorkersToFinish(ParallelContext *pcxt)
527527
if (!anyone_alive)
528528
break;
529529

530-
WaitLatch(&MyProc->procLatch, WL_LATCH_SET, -1,
530+
WaitLatch(MyLatch, WL_LATCH_SET, -1,
531531
WAIT_EVENT_PARALLEL_FINISH);
532-
ResetLatch(&MyProc->procLatch);
532+
ResetLatch(MyLatch);
533533
}
534534

535535
if (pcxt->toc != NULL)

‎src/backend/libpq/pqmq.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,9 @@ mq_putmessage(char msgtype, const char *s, size_t len)
172172
if (result != SHM_MQ_WOULD_BLOCK)
173173
break;
174174

175-
WaitLatch(&MyProc->procLatch, WL_LATCH_SET, 0,
175+
WaitLatch(MyLatch, WL_LATCH_SET, 0,
176176
WAIT_EVENT_MQ_PUT_MESSAGE);
177-
ResetLatch(&MyProc->procLatch);
177+
ResetLatch(MyLatch);
178178
CHECK_FOR_INTERRUPTS();
179179
}
180180

‎src/backend/postmaster/bgworker.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,7 +1144,7 @@ WaitForBackgroundWorkerShutdown(BackgroundWorkerHandle *handle)
11441144
if (status == BGWH_STOPPED)
11451145
break;
11461146

1147-
rc = WaitLatch(&MyProc->procLatch,
1147+
rc = WaitLatch(MyLatch,
11481148
WL_LATCH_SET | WL_POSTMASTER_DEATH, 0,
11491149
WAIT_EVENT_BGWORKER_SHUTDOWN);
11501150

@@ -1154,7 +1154,7 @@ WaitForBackgroundWorkerShutdown(BackgroundWorkerHandle *handle)
11541154
break;
11551155
}
11561156

1157-
ResetLatch(&MyProc->procLatch);
1157+
ResetLatch(MyLatch);
11581158
}
11591159

11601160
return status;

‎src/backend/replication/libpqwalreceiver/libpqwalreceiver.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ libpqrcv_connect(const char *conninfo, bool logical, const char *appname,
176176
? WL_SOCKET_READABLE
177177
: WL_SOCKET_WRITEABLE);
178178

179-
rc = WaitLatchOrSocket(&MyProc->procLatch,
179+
rc = WaitLatchOrSocket(MyLatch,
180180
WL_POSTMASTER_DEATH |
181181
WL_LATCH_SET | io_flag,
182182
PQsocket(conn->streamConn),
@@ -190,7 +190,7 @@ libpqrcv_connect(const char *conninfo, bool logical, const char *appname,
190190
/* Interrupted? */
191191
if (rc & WL_LATCH_SET)
192192
{
193-
ResetLatch(&MyProc->procLatch);
193+
ResetLatch(MyLatch);
194194
CHECK_FOR_INTERRUPTS();
195195
}
196196

@@ -574,21 +574,22 @@ libpqrcv_PQexec(PGconn *streamConn, const char *query)
574574
* the signal arrives in the middle of establishment of
575575
* replication connection.
576576
*/
577-
ResetLatch(&MyProc->procLatch);
578-
rc = WaitLatchOrSocket(&MyProc->procLatch,
577+
rc = WaitLatchOrSocket(MyLatch,
579578
WL_POSTMASTER_DEATH | WL_SOCKET_READABLE |
580579
WL_LATCH_SET,
581580
PQsocket(streamConn),
582581
0,
583582
WAIT_EVENT_LIBPQWALRECEIVER);
583+
584+
/* Emergency bailout? */
584585
if (rc & WL_POSTMASTER_DEATH)
585586
exit(1);
586587

587-
/* interrupted */
588+
/* Interrupted? */
588589
if (rc & WL_LATCH_SET)
589590
{
591+
ResetLatch(MyLatch);
590592
CHECK_FOR_INTERRUPTS();
591-
continue;
592593
}
593594
if (PQconsumeInput(streamConn) == 0)
594595
return NULL; /* trouble */

‎src/backend/replication/logical/launcher.c

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,15 @@ WaitForReplicationWorkerAttach(LogicalRepWorker *worker,
208208
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
209209
1000L, WAIT_EVENT_BGWORKER_STARTUP);
210210

211+
/* emergency bailout if postmaster has died */
211212
if (rc & WL_POSTMASTER_DEATH)
212213
proc_exit(1);
213214

214-
ResetLatch(MyLatch);
215+
if (rc & WL_LATCH_SET)
216+
{
217+
ResetLatch(MyLatch);
218+
CHECK_FOR_INTERRUPTS();
219+
}
215220
}
216221

217222
return;
@@ -440,18 +445,20 @@ logicalrep_worker_stop(Oid subid, Oid relid)
440445

441446
LWLockRelease(LogicalRepWorkerLock);
442447

443-
CHECK_FOR_INTERRUPTS();
444-
445448
/* Wait for signal. */
446-
rc = WaitLatch(&MyProc->procLatch,
449+
rc = WaitLatch(MyLatch,
447450
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
448451
1000L, WAIT_EVENT_BGWORKER_STARTUP);
449452

450453
/* emergency bailout if postmaster has died */
451454
if (rc & WL_POSTMASTER_DEATH)
452455
proc_exit(1);
453456

454-
ResetLatch(&MyProc->procLatch);
457+
if (rc & WL_LATCH_SET)
458+
{
459+
ResetLatch(MyLatch);
460+
CHECK_FOR_INTERRUPTS();
461+
}
455462

456463
/* Check worker status. */
457464
LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
@@ -492,15 +499,19 @@ logicalrep_worker_stop(Oid subid, Oid relid)
492499
CHECK_FOR_INTERRUPTS();
493500

494501
/* Wait for more work. */
495-
rc = WaitLatch(&MyProc->procLatch,
502+
rc = WaitLatch(MyLatch,
496503
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
497504
1000L, WAIT_EVENT_BGWORKER_SHUTDOWN);
498505

499506
/* emergency bailout if postmaster has died */
500507
if (rc & WL_POSTMASTER_DEATH)
501508
proc_exit(1);
502509

503-
ResetLatch(&MyProc->procLatch);
510+
if (rc & WL_LATCH_SET)
511+
{
512+
ResetLatch(MyLatch);
513+
CHECK_FOR_INTERRUPTS();
514+
}
504515
}
505516
}
506517

@@ -876,7 +887,7 @@ ApplyLauncherMain(Datum main_arg)
876887
}
877888

878889
/* Wait for more work. */
879-
rc = WaitLatch(&MyProc->procLatch,
890+
rc = WaitLatch(MyLatch,
880891
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
881892
wait_time,
882893
WAIT_EVENT_LOGICAL_LAUNCHER_MAIN);
@@ -885,13 +896,17 @@ ApplyLauncherMain(Datum main_arg)
885896
if (rc & WL_POSTMASTER_DEATH)
886897
proc_exit(1);
887898

899+
if (rc & WL_LATCH_SET)
900+
{
901+
ResetLatch(MyLatch);
902+
CHECK_FOR_INTERRUPTS();
903+
}
904+
888905
if (got_SIGHUP)
889906
{
890907
got_SIGHUP = false;
891908
ProcessConfigFile(PGC_SIGHUP);
892909
}
893-
894-
ResetLatch(&MyProc->procLatch);
895910
}
896911

897912
LogicalRepCtx->launcher_pid = 0;

‎src/backend/replication/logical/tablesync.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,15 @@ wait_for_relation_state_change(Oid relid, char expected_state)
191191
if (!worker)
192192
return false;
193193

194-
rc = WaitLatch(&MyProc->procLatch,
194+
rc = WaitLatch(MyLatch,
195195
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
196196
1000L, WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
197197

198198
/* emergency bailout if postmaster has died */
199199
if (rc & WL_POSTMASTER_DEATH)
200200
proc_exit(1);
201201

202-
ResetLatch(&MyProc->procLatch);
202+
ResetLatch(MyLatch);
203203
}
204204

205205
return false;
@@ -236,15 +236,15 @@ wait_for_worker_state_change(char expected_state)
236236
if (MyLogicalRepWorker->relstate == expected_state)
237237
return true;
238238

239-
rc = WaitLatch(&MyProc->procLatch,
239+
rc = WaitLatch(MyLatch,
240240
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
241241
1000L, WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
242242

243243
/* emergency bailout if postmaster has died */
244244
if (rc & WL_POSTMASTER_DEATH)
245245
proc_exit(1);
246246

247-
ResetLatch(&MyProc->procLatch);
247+
ResetLatch(MyLatch);
248248
}
249249

250250
return false;
@@ -604,7 +604,7 @@ copy_read_data(void *outbuf, int minread, int maxread)
604604
/*
605605
* Wait for more data or latch.
606606
*/
607-
rc = WaitLatchOrSocket(&MyProc->procLatch,
607+
rc = WaitLatchOrSocket(MyLatch,
608608
WL_SOCKET_READABLE | WL_LATCH_SET |
609609
WL_TIMEOUT | WL_POSTMASTER_DEATH,
610610
fd, 1000L, WAIT_EVENT_LOGICAL_SYNC_DATA);
@@ -613,7 +613,7 @@ copy_read_data(void *outbuf, int minread, int maxread)
613613
if (rc & WL_POSTMASTER_DEATH)
614614
proc_exit(1);
615615

616-
ResetLatch(&MyProc->procLatch);
616+
ResetLatch(MyLatch);
617617
}
618618

619619
return bytesread;

‎src/backend/replication/logical/worker.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,7 +1146,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
11461146
/*
11471147
* Wait for more data or latch.
11481148
*/
1149-
rc = WaitLatchOrSocket(&MyProc->procLatch,
1149+
rc = WaitLatchOrSocket(MyLatch,
11501150
WL_SOCKET_READABLE | WL_LATCH_SET |
11511151
WL_TIMEOUT | WL_POSTMASTER_DEATH,
11521152
fd, NAPTIME_PER_CYCLE,
@@ -1156,6 +1156,12 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
11561156
if (rc & WL_POSTMASTER_DEATH)
11571157
proc_exit(1);
11581158

1159+
if (rc & WL_LATCH_SET)
1160+
{
1161+
ResetLatch(MyLatch);
1162+
CHECK_FOR_INTERRUPTS();
1163+
}
1164+
11591165
if (got_SIGHUP)
11601166
{
11611167
got_SIGHUP = false;
@@ -1209,8 +1215,6 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
12091215

12101216
send_feedback(last_received, requestReply, requestReply);
12111217
}
1212-
1213-
ResetLatch(&MyProc->procLatch);
12141218
}
12151219
}
12161220

‎src/backend/storage/lmgr/condition_variable.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,14 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv)
6868
{
6969
cv_wait_event_set = CreateWaitEventSet(TopMemoryContext, 1);
7070
AddWaitEventToSet(cv_wait_event_set, WL_LATCH_SET, PGINVALID_SOCKET,
71-
&MyProc->procLatch, NULL);
71+
MyLatch, NULL);
7272
}
7373

7474
/*
7575
* Reset my latch before adding myself to the queue and before entering
7676
* the caller's predicate loop.
7777
*/
78-
ResetLatch(&MyProc->procLatch);
78+
ResetLatch(MyLatch);
7979

8080
/* Add myself to the wait queue. */
8181
SpinLockAcquire(&cv->mutex);
@@ -135,7 +135,7 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
135135
WaitEventSetWait(cv_wait_event_set, -1, &event, 1, wait_event_info);
136136

137137
/* Reset latch before testing whether we can return. */
138-
ResetLatch(&MyProc->procLatch);
138+
ResetLatch(MyLatch);
139139

140140
/*
141141
* If this process has been taken out of the wait list, then we know

‎src/test/modules/worker_spi/worker_spi.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ worker_spi_main(Datum main_arg)
235235
if (rc & WL_POSTMASTER_DEATH)
236236
proc_exit(1);
237237

238+
CHECK_FOR_INTERRUPTS();
239+
238240
/*
239241
* In case of a SIGHUP, just reload the configuration.
240242
*/

0 commit comments

Comments
 (0)