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

Commit118e99c

Browse files
committed
Fix low-probability loss of NOTIFY messages due to XID wraparound.
Up to now async.c has used TransactionIdIsInProgress() to detect whethera notify message's source transaction is still running. However, thatfunction has a quick-exit path that reports that XIDs before RecentXminare no longer running. If a listening backend is doing nothing butlistening, and not running any queries, there is nothing that will advanceits value of RecentXmin. Once 2 billion transactions elapse, theRecentXmin check causes active transactions to be reported as not running.If they aren't committed yet according to CLOG, async.c decides theyaborted and discards their messages. The timing for that is a bit tightbut it can happen when multiple backends are sending notifies concurrently.The net symptom therefore is that a sufficiently-long-survivinglisten-only backend starts to miss some fraction of NOTIFY traffic,but only under heavy load.The only function that updates RecentXmin is GetSnapshotData().A brute-force fix would therefore be to take a snapshot beforeprocessing incoming notify messages. But that would add cycles,as well as contention for the ProcArrayLock. We can be smarter:having taken the snapshot, let's use that to check for runningXIDs, and not call TransactionIdIsInProgress() at all. In thisway we reduce the number of ProcArrayLock acquisitions from oneper message to one per notify interrupt; that's the same underlight load but should be a benefit under heavy load. Light testingsays that this change is a wash performance-wise for normal loads.I looked around for other callers of TransactionIdIsInProgress()that might be at similar risk, and didn't find any; all of themare inside transactions that presumably have already taken asnapshot.Problem report and diagnosis by Marko Tiikkaja, patch by me.Back-patch to all supported branches, since it's been like thissince 9.0.Discussion:https://postgr.es/m/20170926182935.14128.65278@wrigleys.postgresql.org
1 parent46912d9 commit118e99c

File tree

3 files changed

+33
-15
lines changed

3 files changed

+33
-15
lines changed

‎src/backend/commands/async.c

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@
137137
#include"utils/builtins.h"
138138
#include"utils/memutils.h"
139139
#include"utils/ps_status.h"
140+
#include"utils/snapmgr.h"
140141
#include"utils/timestamp.h"
142+
#include"utils/tqual.h"
141143

142144

143145
/*
@@ -387,7 +389,8 @@ static bool SignalBackends(void);
387389
staticvoidasyncQueueReadAllNotifications(void);
388390
staticboolasyncQueueProcessPageEntries(volatileQueuePosition*current,
389391
QueuePositionstop,
390-
char*page_buffer);
392+
char*page_buffer,
393+
Snapshotsnapshot);
391394
staticvoidasyncQueueAdvanceTail(void);
392395
staticvoidProcessIncomingNotify(void);
393396
staticboolAsyncExistsPendingNotify(constchar*channel,constchar*payload);
@@ -798,7 +801,7 @@ PreCommit_Notify(void)
798801
}
799802
}
800803

801-
/* Queue any pending notifies */
804+
/* Queue any pending notifies(must happen after the above)*/
802805
if (pendingNotifies)
803806
{
804807
ListCell*nextNotify;
@@ -987,7 +990,9 @@ Exec_ListenPreCommit(void)
987990
* have already committed before we started to LISTEN.
988991
*
989992
* Note that we are not yet listening on anything, so we won't deliver any
990-
* notification to the frontend.
993+
* notification to the frontend. Also, although our transaction might
994+
* have executed NOTIFY, those message(s) aren't queued yet so we can't
995+
* see them in the queue.
991996
*
992997
* This will also advance the global tail pointer if possible.
993998
*/
@@ -1744,6 +1749,7 @@ asyncQueueReadAllNotifications(void)
17441749
volatileQueuePositionpos;
17451750
QueuePositionoldpos;
17461751
QueuePositionhead;
1752+
Snapshotsnapshot;
17471753
booladvanceTail;
17481754

17491755
/* page_buffer must be adequately aligned, so use a union */
@@ -1767,6 +1773,9 @@ asyncQueueReadAllNotifications(void)
17671773
return;
17681774
}
17691775

1776+
/* Get snapshot we'll use to decide which xacts are still in progress */
1777+
snapshot=RegisterSnapshot(GetLatestSnapshot());
1778+
17701779
/*----------
17711780
* Note that we deliver everything that we see in the queue and that
17721781
* matches our _current_ listening state.
@@ -1854,7 +1863,8 @@ asyncQueueReadAllNotifications(void)
18541863
* while sending the notifications to the frontend.
18551864
*/
18561865
reachedStop=asyncQueueProcessPageEntries(&pos,head,
1857-
page_buffer.buf);
1866+
page_buffer.buf,
1867+
snapshot);
18581868
}while (!reachedStop);
18591869
}
18601870
PG_CATCH();
@@ -1882,6 +1892,9 @@ asyncQueueReadAllNotifications(void)
18821892
/* If we were the laziest backend, try to advance the tail pointer */
18831893
if (advanceTail)
18841894
asyncQueueAdvanceTail();
1895+
1896+
/* Done with snapshot */
1897+
UnregisterSnapshot(snapshot);
18851898
}
18861899

18871900
/*
@@ -1903,7 +1916,8 @@ asyncQueueReadAllNotifications(void)
19031916
staticbool
19041917
asyncQueueProcessPageEntries(volatileQueuePosition*current,
19051918
QueuePositionstop,
1906-
char*page_buffer)
1919+
char*page_buffer,
1920+
Snapshotsnapshot)
19071921
{
19081922
boolreachedStop= false;
19091923
boolreachedEndOfPage;
@@ -1928,7 +1942,7 @@ asyncQueueProcessPageEntries(volatile QueuePosition *current,
19281942
/* Ignore messages destined for other databases */
19291943
if (qe->dboid==MyDatabaseId)
19301944
{
1931-
if (TransactionIdIsInProgress(qe->xid))
1945+
if (XidInMVCCSnapshot(qe->xid,snapshot))
19321946
{
19331947
/*
19341948
* The source transaction is still in progress, so we can't
@@ -1939,10 +1953,15 @@ asyncQueueProcessPageEntries(volatile QueuePosition *current,
19391953
* this advance-then-back-up behavior when dealing with an
19401954
* uncommitted message.)
19411955
*
1942-
* Note that we must test TransactionIdIsInProgress before we
1943-
* test TransactionIdDidCommit, else we might return a message
1944-
* from a transaction that is not yet visible to snapshots;
1945-
* compare the comments at the head of tqual.c.
1956+
* Note that we must test XidInMVCCSnapshot before we test
1957+
* TransactionIdDidCommit, else we might return a message from
1958+
* a transaction that is not yet visible to snapshots; compare
1959+
* the comments at the head of tqual.c.
1960+
*
1961+
* Also, while our own xact won't be listed in the snapshot,
1962+
* we need not check for TransactionIdIsCurrentTransactionId
1963+
* because our transaction cannot (yet) have queued any
1964+
* messages.
19461965
*/
19471966
*current=thisentry;
19481967
reachedStop= true;

‎src/backend/utils/time/tqual.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@
8181
SnapshotDataSnapshotSelfData= {HeapTupleSatisfiesSelf};
8282
SnapshotDataSnapshotAnyData= {HeapTupleSatisfiesAny};
8383

84-
/* local functions */
85-
staticboolXidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot);
8684

8785
/*
8886
* SetHintBits()
@@ -1479,10 +1477,10 @@ HeapTupleIsSurelyDead(HeapTuple htup, TransactionId OldestXmin)
14791477
* Note: GetSnapshotData never stores either top xid or subxids of our own
14801478
* backend into a snapshot, so these xids will not be reported as "running"
14811479
* by this function. This is OK for current uses, because we always check
1482-
* TransactionIdIsCurrentTransactionId first, exceptforknown-committed
1483-
*XIDs which could not be ours anyway.
1480+
* TransactionIdIsCurrentTransactionId first, exceptwhen it'sknown the
1481+
*XID could not be ours anyway.
14841482
*/
1485-
staticbool
1483+
bool
14861484
XidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot)
14871485
{
14881486
uint32i;

‎src/include/utils/tqual.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTuple htup,
7878
TransactionIdOldestXmin,Bufferbuffer);
7979
externboolHeapTupleIsSurelyDead(HeapTuplehtup,
8080
TransactionIdOldestXmin);
81+
externboolXidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot);
8182

8283
externvoidHeapTupleSetHintBits(HeapTupleHeadertuple,Bufferbuffer,
8384
uint16infomask,TransactionIdxid);

0 commit comments

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