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

Commit525b09a

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 parent6d2ef1c commit525b09a

File tree

3 files changed

+34
-15
lines changed

3 files changed

+34
-15
lines changed

‎src/backend/commands/async.c

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@
133133
#include"utils/builtins.h"
134134
#include"utils/memutils.h"
135135
#include"utils/ps_status.h"
136+
#include"utils/snapmgr.h"
136137
#include"utils/timestamp.h"
138+
#include"utils/tqual.h"
137139

138140

139141
/*
@@ -388,7 +390,8 @@ static bool SignalBackends(void);
388390
staticvoidasyncQueueReadAllNotifications(void);
389391
staticboolasyncQueueProcessPageEntries(volatileQueuePosition*current,
390392
QueuePositionstop,
391-
char*page_buffer);
393+
char*page_buffer,
394+
Snapshotsnapshot);
392395
staticvoidasyncQueueAdvanceTail(void);
393396
staticvoidProcessIncomingNotify(void);
394397
staticvoidNotifyMyFrontEnd(constchar*channel,
@@ -799,7 +802,7 @@ PreCommit_Notify(void)
799802
}
800803
}
801804

802-
/* Queue any pending notifies */
805+
/* Queue any pending notifies(must happen after the above)*/
803806
if (pendingNotifies)
804807
{
805808
ListCell*nextNotify;
@@ -988,7 +991,9 @@ Exec_ListenPreCommit(void)
988991
* have already committed before we started to LISTEN.
989992
*
990993
* Note that we are not yet listening on anything, so we won't deliver any
991-
* notification to the frontend.
994+
* notification to the frontend. Also, although our transaction might
995+
* have executed NOTIFY, those message(s) aren't queued yet so we can't
996+
* see them in the queue.
992997
*
993998
* This will also advance the global tail pointer if possible.
994999
*/
@@ -1837,6 +1842,7 @@ asyncQueueReadAllNotifications(void)
18371842
volatileQueuePositionpos;
18381843
QueuePositionoldpos;
18391844
QueuePositionhead;
1845+
Snapshotsnapshot;
18401846
booladvanceTail;
18411847

18421848
/* page_buffer must be adequately aligned, so use a union */
@@ -1860,6 +1866,9 @@ asyncQueueReadAllNotifications(void)
18601866
return;
18611867
}
18621868

1869+
/* Get snapshot we'll use to decide which xacts are still in progress */
1870+
snapshot=RegisterSnapshot(GetLatestSnapshot());
1871+
18631872
/*----------
18641873
* Note that we deliver everything that we see in the queue and that
18651874
* matches our _current_ listening state.
@@ -1947,7 +1956,8 @@ asyncQueueReadAllNotifications(void)
19471956
* while sending the notifications to the frontend.
19481957
*/
19491958
reachedStop=asyncQueueProcessPageEntries(&pos,head,
1950-
page_buffer.buf);
1959+
page_buffer.buf,
1960+
snapshot);
19511961
}while (!reachedStop);
19521962
}
19531963
PG_CATCH();
@@ -1975,6 +1985,9 @@ asyncQueueReadAllNotifications(void)
19751985
/* If we were the laziest backend, try to advance the tail pointer */
19761986
if (advanceTail)
19771987
asyncQueueAdvanceTail();
1988+
1989+
/* Done with snapshot */
1990+
UnregisterSnapshot(snapshot);
19781991
}
19791992

19801993
/*
@@ -1996,7 +2009,8 @@ asyncQueueReadAllNotifications(void)
19962009
staticbool
19972010
asyncQueueProcessPageEntries(volatileQueuePosition*current,
19982011
QueuePositionstop,
1999-
char*page_buffer)
2012+
char*page_buffer,
2013+
Snapshotsnapshot)
20002014
{
20012015
boolreachedStop= false;
20022016
boolreachedEndOfPage;
@@ -2021,7 +2035,7 @@ asyncQueueProcessPageEntries(volatile QueuePosition *current,
20212035
/* Ignore messages destined for other databases */
20222036
if (qe->dboid==MyDatabaseId)
20232037
{
2024-
if (TransactionIdIsInProgress(qe->xid))
2038+
if (XidInMVCCSnapshot(qe->xid,snapshot))
20252039
{
20262040
/*
20272041
* The source transaction is still in progress, so we can't
@@ -2032,10 +2046,15 @@ asyncQueueProcessPageEntries(volatile QueuePosition *current,
20322046
* this advance-then-back-up behavior when dealing with an
20332047
* uncommitted message.)
20342048
*
2035-
* Note that we must test TransactionIdIsInProgress before we
2036-
* test TransactionIdDidCommit, else we might return a message
2037-
* from a transaction that is not yet visible to snapshots;
2038-
* compare the comments at the head of tqual.c.
2049+
* Note that we must test XidInMVCCSnapshot before we test
2050+
* TransactionIdDidCommit, else we might return a message from
2051+
* a transaction that is not yet visible to snapshots; compare
2052+
* the comments at the head of tqual.c.
2053+
*
2054+
* Also, while our own xact won't be listed in the snapshot,
2055+
* we need not check for TransactionIdIsCurrentTransactionId
2056+
* because our transaction cannot (yet) have queued any
2057+
* messages.
20392058
*/
20402059
*current=thisentry;
20412060
reachedStop= true;

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
7272
SnapshotDataSnapshotAnyData= {HeapTupleSatisfiesAny};
7373
SnapshotDataSnapshotToastData= {HeapTupleSatisfiesToast};
7474

75-
/* local functions */
76-
staticboolXidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot);
7775

7876

7977
/*
@@ -1282,10 +1280,11 @@ HeapTupleIsSurelyDead(HeapTupleHeader tuple, TransactionId OldestXmin)
12821280
*
12831281
* Note: GetSnapshotData never stores either top xid or subxids of our own
12841282
* backend into a snapshot, so these xids will not be reported as "running"
1285-
* by this function. This is OK for current uses, because we actually only
1286-
* apply this for known-committed XIDs.
1283+
* by this function. This is OK for current uses, because we always check
1284+
* TransactionIdIsCurrentTransactionId first, except when it's known the
1285+
* XID could not be ours anyway.
12871286
*/
1288-
staticbool
1287+
bool
12891288
XidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot)
12901289
{
12911290
uint32i;

‎src/include/utils/tqual.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTupleHeader tuple,
8585
TransactionIdOldestXmin,Bufferbuffer);
8686
externboolHeapTupleIsSurelyDead(HeapTupleHeadertuple,
8787
TransactionIdOldestXmin);
88+
externboolXidInMVCCSnapshot(TransactionIdxid,Snapshotsnapshot);
8889

8990
externvoidHeapTupleSetHintBits(HeapTupleHeadertuple,Bufferbuffer,
9091
uint16infomask,TransactionIdxid);

0 commit comments

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