Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9f540f8

Browse files
committed
Detect the deadlocks between backends and the startup process.
The deadlocks that the recovery conflict on lock is involved in canhappen between hot-standby backends and the startup process.If a backend takes an access exclusive lock on the table and whichfinally triggers the deadlock, that deadlock can be detectedas expected. On the other hand, previously, if the startup processtook an access exclusive lock and which finally triggered the deadlock,that deadlock could not be detected and could remain even afterdeadlock_timeout passed. This is a bug.The cause of this bug was that the code for handling the recoveryconflict on lock didn't take care of deadlock case at all. It assumedthat deadlocks involving the startup process and backends were ableto be detected by the deadlock detector invoked within backends.But this assumption was incorrect. The startup process also shouldhave invoked the deadlock detector if necessary.To fix this bug, this commit makes the startup process invokethe deadlock detector if deadlock_timeout is reached while handlingthe recovery conflict on lock. Specifically, in that case, the startupprocess requests all the backends holding the conflicting locks tocheck themselves for deadlocks.Back-patch to v9.6. v9.5 has also this bug, but per discussion we decidednot to back-patch the fix to v9.5. Because v9.5 doesn't have someinfrastructure codes (e.g.,37c5486) that this bug fix patch depends on.We can apply those codes for the back-patch, but since the next minorversion release is the final one for v9.5, it's risky to do that. If weunexpectedly introduce new bug to v9.5 by the back-patch, there is nochance to fix that. We determined that the back-patch to v9.5 would givemore risk than gain.Author: Fujii MasaoReviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro HoriguchiDiscussion:https://postgr.es/m/4041d6b6-cf24-a120-36fa-1294220f8243@oss.nttdata.com
1 parentc919ed1 commit9f540f8

File tree

5 files changed

+141
-32
lines changed

5 files changed

+141
-32
lines changed

‎src/backend/storage/ipc/procarray.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2654,6 +2654,13 @@ GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid)
26542654
*/
26552655
pid_t
26562656
CancelVirtualTransaction(VirtualTransactionIdvxid,ProcSignalReasonsigmode)
2657+
{
2658+
returnSignalVirtualTransaction(vxid,sigmode, true);
2659+
}
2660+
2661+
pid_t
2662+
SignalVirtualTransaction(VirtualTransactionIdvxid,ProcSignalReasonsigmode,
2663+
boolconflictPending)
26572664
{
26582665
ProcArrayStruct*arrayP=procArray;
26592666
intindex;
@@ -2672,7 +2679,7 @@ CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
26722679
if (procvxid.backendId==vxid.backendId&&
26732680
procvxid.localTransactionId==vxid.localTransactionId)
26742681
{
2675-
proc->recoveryConflictPending=true;
2682+
proc->recoveryConflictPending=conflictPending;
26762683
pid=proc->pid;
26772684
if (pid!=0)
26782685
{

‎src/backend/storage/ipc/standby.c

Lines changed: 114 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ intmax_standby_streaming_delay = 30 * 1000;
4242

4343
staticHTAB*RecoveryLockLists;
4444

45+
/* Flags set by timeout handlers */
46+
staticvolatilesig_atomic_tgot_standby_deadlock_timeout= false;
47+
staticvolatilesig_atomic_tgot_standby_lock_timeout= false;
48+
4549
staticvoidResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId*waitlist,
4650
ProcSignalReasonreason,boolreport_waiting);
4751
staticvoidSendRecoveryConflictWithBufferPin(ProcSignalReasonreason);
@@ -391,8 +395,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
391395
* lock. As we are already queued to be granted the lock, no new lock
392396
* requests conflicting with ours will be granted in the meantime.
393397
*
394-
* Deadlocks involving the Startup process and an ordinary backend process
395-
* will be detected by the deadlock detector within the ordinary backend.
398+
* We also must check for deadlocks involving the Startup process and
399+
* hot-standby backend processes. If deadlock_timeout is reached in
400+
* this function, all the backends holding the conflicting locks are
401+
* requested to check themselves for deadlocks.
396402
*/
397403
void
398404
ResolveRecoveryConflictWithLock(LOCKTAGlocktag)
@@ -403,7 +409,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
403409

404410
ltime=GetStandbyLimitTime();
405411

406-
if (GetCurrentTimestamp() >=ltime)
412+
if (GetCurrentTimestamp() >=ltime&&ltime!=0)
407413
{
408414
/*
409415
* We're already behind, so clear a path as quickly as possible.
@@ -424,26 +430,85 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
424430
else
425431
{
426432
/*
427-
* Wait (or wait again) until ltime
433+
* Wait (or wait again) until ltime, and check for deadlocks as well
434+
* if we will be waiting longer than deadlock_timeout
428435
*/
429-
EnableTimeoutParamstimeouts[1];
436+
EnableTimeoutParamstimeouts[2];
437+
intcnt=0;
438+
439+
if (ltime!=0)
440+
{
441+
got_standby_lock_timeout= false;
442+
timeouts[cnt].id=STANDBY_LOCK_TIMEOUT;
443+
timeouts[cnt].type=TMPARAM_AT;
444+
timeouts[cnt].fin_time=ltime;
445+
cnt++;
446+
}
430447

431-
timeouts[0].id=STANDBY_LOCK_TIMEOUT;
432-
timeouts[0].type=TMPARAM_AT;
433-
timeouts[0].fin_time=ltime;
434-
enable_timeouts(timeouts,1);
448+
got_standby_deadlock_timeout= false;
449+
timeouts[cnt].id=STANDBY_DEADLOCK_TIMEOUT;
450+
timeouts[cnt].type=TMPARAM_AFTER;
451+
timeouts[cnt].delay_ms=DeadlockTimeout;
452+
cnt++;
453+
454+
enable_timeouts(timeouts,cnt);
435455
}
436456

437457
/* Wait to be signaled by the release of the Relation Lock */
438458
ProcWaitForSignal(PG_WAIT_LOCK |locktag.locktag_type);
439459

460+
/*
461+
* Exit if ltime is reached. Then all the backends holding conflicting
462+
* locks will be canceled in the next ResolveRecoveryConflictWithLock()
463+
* call.
464+
*/
465+
if (got_standby_lock_timeout)
466+
gotocleanup;
467+
468+
if (got_standby_deadlock_timeout)
469+
{
470+
VirtualTransactionId*backends;
471+
472+
backends=GetLockConflicts(&locktag,AccessExclusiveLock,NULL);
473+
474+
/* Quick exit if there's no work to be done */
475+
if (!VirtualTransactionIdIsValid(*backends))
476+
gotocleanup;
477+
478+
/*
479+
* Send signals to all the backends holding the conflicting locks, to
480+
* ask them to check themselves for deadlocks.
481+
*/
482+
while (VirtualTransactionIdIsValid(*backends))
483+
{
484+
SignalVirtualTransaction(*backends,
485+
PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
486+
false);
487+
backends++;
488+
}
489+
490+
/*
491+
* Wait again here to be signaled by the release of the Relation Lock,
492+
* to prevent the subsequent RecoveryConflictWithLock() from causing
493+
* deadlock_timeout and sending a request for deadlocks check again.
494+
* Otherwise the request continues to be sent every deadlock_timeout
495+
* until the relation locks are released or ltime is reached.
496+
*/
497+
got_standby_deadlock_timeout= false;
498+
ProcWaitForSignal(PG_WAIT_LOCK |locktag.locktag_type);
499+
}
500+
501+
cleanup:
502+
440503
/*
441504
* Clear any timeout requests established above. We assume here that the
442505
* Startup process doesn't have any other outstanding timeouts than those
443506
* used by this function. If that stops being true, we could cancel the
444507
* timeouts individually, but that'd be slower.
445508
*/
446509
disable_all_timeouts(false);
510+
got_standby_lock_timeout= false;
511+
got_standby_deadlock_timeout= false;
447512
}
448513

449514
/*
@@ -482,15 +547,7 @@ ResolveRecoveryConflictWithBufferPin(void)
482547

483548
ltime=GetStandbyLimitTime();
484549

485-
if (ltime==0)
486-
{
487-
/*
488-
* We're willing to wait forever for conflicts, so set timeout for
489-
* deadlock check only
490-
*/
491-
enable_timeout_after(STANDBY_DEADLOCK_TIMEOUT,DeadlockTimeout);
492-
}
493-
elseif (GetCurrentTimestamp() >=ltime)
550+
if (GetCurrentTimestamp() >=ltime&&ltime!=0)
494551
{
495552
/*
496553
* We're already behind, so clear a path as quickly as possible.
@@ -504,26 +561,55 @@ ResolveRecoveryConflictWithBufferPin(void)
504561
* waiting longer than deadlock_timeout
505562
*/
506563
EnableTimeoutParamstimeouts[2];
564+
intcnt=0;
507565

508-
timeouts[0].id=STANDBY_TIMEOUT;
509-
timeouts[0].type=TMPARAM_AT;
510-
timeouts[0].fin_time=ltime;
511-
timeouts[1].id=STANDBY_DEADLOCK_TIMEOUT;
512-
timeouts[1].type=TMPARAM_AFTER;
513-
timeouts[1].delay_ms=DeadlockTimeout;
514-
enable_timeouts(timeouts,2);
566+
if (ltime!=0)
567+
{
568+
timeouts[cnt].id=STANDBY_TIMEOUT;
569+
timeouts[cnt].type=TMPARAM_AT;
570+
timeouts[cnt].fin_time=ltime;
571+
cnt++;
572+
}
573+
574+
got_standby_deadlock_timeout= false;
575+
timeouts[cnt].id=STANDBY_DEADLOCK_TIMEOUT;
576+
timeouts[cnt].type=TMPARAM_AFTER;
577+
timeouts[cnt].delay_ms=DeadlockTimeout;
578+
cnt++;
579+
580+
enable_timeouts(timeouts,cnt);
515581
}
516582

517583
/* Wait to be signaled by UnpinBuffer() */
518584
ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
519585

586+
if (got_standby_deadlock_timeout)
587+
{
588+
/*
589+
* Send out a request for hot-standby backends to check themselves for
590+
* deadlocks.
591+
*
592+
* XXX The subsequent ResolveRecoveryConflictWithBufferPin() will wait
593+
* to be signaled by UnpinBuffer() again and send a request for
594+
* deadlocks check if deadlock_timeout happens. This causes the
595+
* request to continue to be sent every deadlock_timeout until the
596+
* buffer is unpinned or ltime is reached. This would increase the
597+
* workload in the startup process and backends. In practice it may
598+
* not be so harmful because the period that the buffer is kept pinned
599+
* is basically no so long. But we should fix this?
600+
*/
601+
SendRecoveryConflictWithBufferPin(
602+
PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
603+
}
604+
520605
/*
521606
* Clear any timeout requests established above. We assume here that the
522607
* Startup process doesn't have any other timeouts than what this function
523608
* uses. If that stops being true, we could cancel the timeouts
524609
* individually, but that'd be slower.
525610
*/
526611
disable_all_timeouts(false);
612+
got_standby_deadlock_timeout= false;
527613
}
528614

529615
staticvoid
@@ -583,13 +669,12 @@ CheckRecoveryConflictDeadlock(void)
583669

584670
/*
585671
* StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT
586-
* occurs before STANDBY_TIMEOUT. Send out a request for hot-standby
587-
* backends to check themselves for deadlocks.
672+
* occurs before STANDBY_TIMEOUT.
588673
*/
589674
void
590675
StandbyDeadLockHandler(void)
591676
{
592-
SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
677+
got_standby_deadlock_timeout= true;
593678
}
594679

595680
/*
@@ -608,11 +693,11 @@ StandbyTimeoutHandler(void)
608693

609694
/*
610695
* StandbyLockTimeoutHandler() will be called if STANDBY_LOCK_TIMEOUT is exceeded.
611-
* This doesn't need to do anything, simply waking up is enough.
612696
*/
613697
void
614698
StandbyLockTimeoutHandler(void)
615699
{
700+
got_standby_lock_timeout= true;
616701
}
617702

618703
/*

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,6 +1783,9 @@ CheckDeadLockAlert(void)
17831783
* Have to set the latch again, even if handle_sig_alarm already did. Back
17841784
* then got_deadlock_timeout wasn't yet set... It's unlikely that this
17851785
* ever would be a problem, but setting a set latch again is cheap.
1786+
*
1787+
* Note that, when this function runs inside procsignal_sigusr1_handler(),
1788+
* the handler function sets the latch again after the latch is set here.
17861789
*/
17871790
SetLatch(MyLatch);
17881791
errno=save_errno;

‎src/backend/tcop/postgres.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,11 +2852,23 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
28522852
casePROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
28532853

28542854
/*
2855-
* If we aren't blocking the Startup process there is nothing
2856-
* more to do.
2855+
* If PROCSIG_RECOVERY_CONFLICT_BUFFERPIN is requested but we
2856+
* aren't blocking the Startup process there is nothing more
2857+
* to do.
2858+
*
2859+
* When PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK is
2860+
* requested, if we're waiting for locks and the startup
2861+
* process is not waiting for buffer pin (i.e., also waiting
2862+
* for locks), we set the flag so that ProcSleep() will check
2863+
* for deadlocks.
28572864
*/
28582865
if (!HoldingBufferPinThatDelaysRecovery())
2866+
{
2867+
if (reason==PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK&&
2868+
GetStartupBufferPinWaitBufId()<0)
2869+
CheckDeadLockAlert();
28592870
return;
2871+
}
28602872

28612873
MyProc->recoveryConflictPending= true;
28622874

‎src/include/storage/procarray.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
105105
int*nvxids);
106106
externVirtualTransactionId*GetConflictingVirtualXIDs(TransactionIdlimitXmin,OiddbOid);
107107
externpid_tCancelVirtualTransaction(VirtualTransactionIdvxid,ProcSignalReasonsigmode);
108+
externpid_tSignalVirtualTransaction(VirtualTransactionIdvxid,ProcSignalReasonsigmode,
109+
boolconflictPending);
108110

109111
externboolMinimumActiveBackends(intmin);
110112
externintCountDBBackends(Oiddatabaseid);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp