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

Commit57eb009

Browse files
committed
Allow snapshot references to still work during transaction abort.
In REPEATABLE READ (nee SERIALIZABLE) mode, an attempt to doGetTransactionSnapshot() between AbortTransaction and CleanupTransactionfailed, because GetTransactionSnapshot would recompute the transactionsnapshot (which is already wrong, given the isolation mode) and thenre-register it in the TopTransactionResourceOwner, leading to an Assertbecause the TopTransactionResourceOwner should be empty of resources afterAbortTransaction. This is the root cause of bug #6218 from YamamotoTakashi. While changing plancache.c to avoid requesting a snapshot whenhandling a ROLLBACK masks the problem, I think this is really a snapmgr.cbug: it's lower-level than the resource manager mechanism and should not beshutting itself down before we unwind resource manager resources. However,just postponing the release of the transaction snapshot until cleanup timedidn't work because of the circular dependency withTopTransactionResourceOwner. Fix by managing the internal reference tothat snapshot manually instead of depending on TopTransactionResourceOwner.This saves a few cycles as well as making the module layering morestraightforward. predicate.c's dependencies on TopTransactionResourceOwnergo away too.I think this is a longstanding bug, but there's no evidence that it's morethan a latent bug, so it doesn't seem worth any risk of back-patching.
1 parent16762b5 commit57eb009

File tree

5 files changed

+67
-57
lines changed

5 files changed

+67
-57
lines changed

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,9 +1906,6 @@ CommitTransaction(void)
19061906
/* Clean up the relation cache */
19071907
AtEOXact_RelationCache(true);
19081908

1909-
/* Clean up the snapshot manager */
1910-
AtEarlyCommit_Snapshot();
1911-
19121909
/*
19131910
* Make catalog changes visible to all backends. This has to happen after
19141911
* relcache references are dropped (see comments for
@@ -2158,9 +2155,6 @@ PrepareTransaction(void)
21582155
/* Clean up the relation cache */
21592156
AtEOXact_RelationCache(true);
21602157

2161-
/* Clean up the snapshot manager */
2162-
AtEarlyCommit_Snapshot();
2163-
21642158
/* notify doesn't need a postprepare call */
21652159

21662160
PostPrepare_PgStat();
@@ -2339,7 +2333,6 @@ AbortTransaction(void)
23392333
AtEOXact_ComboCid();
23402334
AtEOXact_HashTables(false);
23412335
AtEOXact_PgStat(false);
2342-
AtEOXact_Snapshot(false);
23432336
pgstat_report_xact_timestamp(0);
23442337
}
23452338

@@ -2368,6 +2361,7 @@ CleanupTransaction(void)
23682361
* do abort cleanup processing
23692362
*/
23702363
AtCleanup_Portals();/* now safe to release portal memory */
2364+
AtEOXact_Snapshot(false);/* and release the transaction's snapshots */
23712365

23722366
CurrentResourceOwner=NULL;/* and resource owner */
23732367
if (TopTransactionResourceOwner)

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@
146146
*PageIsPredicateLocked(Relation relation, BlockNumber blkno)
147147
*
148148
* predicate lock maintenance
149-
*RegisterSerializableTransaction(Snapshot snapshot)
149+
*GetSerializableTransactionSnapshot(Snapshot snapshot)
150150
*RegisterPredicateLockingXid(void)
151151
*PredicateLockRelation(Relation relation, Snapshot snapshot)
152152
*PredicateLockPage(Relation relation, BlockNumber blkno,
@@ -417,7 +417,7 @@ static void OldSerXidSetActiveSerXmin(TransactionId xid);
417417
staticuint32predicatelock_hash(constvoid*key,Sizekeysize);
418418
staticvoidSummarizeOldestCommittedSxact(void);
419419
staticSnapshotGetSafeSnapshot(Snapshotsnapshot);
420-
staticSnapshotRegisterSerializableTransactionInt(Snapshotsnapshot);
420+
staticSnapshotGetSerializableTransactionSnapshotInt(Snapshotsnapshot);
421421
staticboolPredicateLockExists(constPREDICATELOCKTARGETTAG*targettag);
422422
staticboolGetParentPredicateLockTag(constPREDICATELOCKTARGETTAG*tag,
423423
PREDICATELOCKTARGETTAG*parent);
@@ -1485,6 +1485,10 @@ SummarizeOldestCommittedSxact(void)
14851485
*without further checks. This requires waiting for concurrent
14861486
*transactions to complete, and retrying with a new snapshot if
14871487
*one of them could possibly create a conflict.
1488+
*
1489+
*As with GetSerializableTransactionSnapshot (which this is a subroutine
1490+
*for), the passed-in Snapshot pointer should reference a static data
1491+
*area that can safely be passed to GetSnapshotData.
14881492
*/
14891493
staticSnapshot
14901494
GetSafeSnapshot(SnapshotorigSnapshot)
@@ -1496,12 +1500,12 @@ GetSafeSnapshot(Snapshot origSnapshot)
14961500
while (true)
14971501
{
14981502
/*
1499-
*RegisterSerializableTransactionInt is going to call
1500-
* GetSnapshotData, so we need to provide it the static snapshotour
1501-
* caller passed to us.It returns a copy of that snapshot and
1502-
*registers it on TopTransactionResourceOwner.
1503+
*GetSerializableTransactionSnapshotInt is going to call
1504+
* GetSnapshotData, so we need to provide it the static snapshotarea
1505+
*ourcaller passed to us. The pointer returned is actually the same
1506+
*one passed to it, but we avoid assuming that here.
15031507
*/
1504-
snapshot=RegisterSerializableTransactionInt(origSnapshot);
1508+
snapshot=GetSerializableTransactionSnapshotInt(origSnapshot);
15051509

15061510
if (MySerializableXact==InvalidSerializableXact)
15071511
returnsnapshot;/* no concurrent r/w xacts; it's safe */
@@ -1535,8 +1539,6 @@ GetSafeSnapshot(Snapshot origSnapshot)
15351539
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
15361540
errmsg("deferrable snapshot was unsafe; trying a new one")));
15371541
ReleasePredicateLocks(false);
1538-
UnregisterSnapshotFromOwner(snapshot,
1539-
TopTransactionResourceOwner);
15401542
}
15411543

15421544
/*
@@ -1549,28 +1551,34 @@ GetSafeSnapshot(Snapshot origSnapshot)
15491551
}
15501552

15511553
/*
1552-
* Acquire and register a snapshot which can be used for this transaction..
1554+
* Acquire a snapshot that can be used for the current transaction.
1555+
*
15531556
* Make sure we have a SERIALIZABLEXACT reference in MySerializableXact.
15541557
* It should be current for this process and be contained in PredXact.
1558+
*
1559+
* The passed-in Snapshot pointer should reference a static data area that
1560+
* can safely be passed to GetSnapshotData. The return value is actually
1561+
* always this same pointer; no new snapshot data structure is allocated
1562+
* within this function.
15551563
*/
15561564
Snapshot
1557-
RegisterSerializableTransaction(Snapshotsnapshot)
1565+
GetSerializableTransactionSnapshot(Snapshotsnapshot)
15581566
{
15591567
Assert(IsolationIsSerializable());
15601568

15611569
/*
15621570
* A special optimization is available for SERIALIZABLE READ ONLY
15631571
* DEFERRABLE transactions -- we can wait for a suitable snapshot and
1564-
* thereby avoid all SSI overhead once it's running..
1572+
* thereby avoid all SSI overhead once it's running.
15651573
*/
15661574
if (XactReadOnly&&XactDeferrable)
15671575
returnGetSafeSnapshot(snapshot);
15681576

1569-
returnRegisterSerializableTransactionInt(snapshot);
1577+
returnGetSerializableTransactionSnapshotInt(snapshot);
15701578
}
15711579

15721580
staticSnapshot
1573-
RegisterSerializableTransactionInt(Snapshotsnapshot)
1581+
GetSerializableTransactionSnapshotInt(Snapshotsnapshot)
15741582
{
15751583
PGPROC*proc;
15761584
VirtualTransactionIdvxid;
@@ -1607,9 +1615,8 @@ RegisterSerializableTransactionInt(Snapshot snapshot)
16071615
}
16081616
}while (!sxact);
16091617

1610-
/* Getand register a snapshot */
1618+
/* Getthe snapshot */
16111619
snapshot=GetSnapshotData(snapshot);
1612-
snapshot=RegisterSnapshotOnOwner(snapshot,TopTransactionResourceOwner);
16131620

16141621
/*
16151622
* If there are no serializable transactions which are not read-only, we

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

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77
* persistent memory. When a snapshot is no longer in any of these lists
88
* (tracked by separate refcounts on each snapshot), its memory can be freed.
99
*
10+
* The FirstXactSnapshot, if any, is treated a bit specially: we increment its
11+
* regd_count and count it in RegisteredSnapshots, but this reference is not
12+
* tracked by a resource owner. We used to use the TopTransactionResourceOwner
13+
* to track this snapshot reference, but that introduces logical circularity
14+
* and thus makes it impossible to clean up in a sane fashion. It's better to
15+
* handle this reference as an internally-tracked registration, so that this
16+
* module is entirely lower-level than ResourceOwners.
17+
*
1018
* These arrangements let us reset MyProc->xmin when there are no snapshots
1119
* referenced by this transaction.(One possible improvement would be to be
1220
* able to advance Xmin when the snapshot with the earliest Xmin is no longer
@@ -97,11 +105,11 @@ static intRegisteredSnapshots = 0;
97105
boolFirstSnapshotSet= false;
98106

99107
/*
100-
*Remembers whether this transactionregistered a transaction snapshot at
101-
*start. We cannot trustFirstSnapshotSet in combination with
102-
*IsolationUsesXactSnapshot(), becauseGUC may be reset before us.
108+
*Remember the serializable transactionsnapshot, if any. We cannot trust
109+
* FirstSnapshotSet in combination with IsolationUsesXactSnapshot(), because
110+
* GUC may be reset before us, changing the value of IsolationUsesXactSnapshot.
103111
*/
104-
staticboolregistered_xact_snapshot=false;
112+
staticSnapshotFirstXactSnapshot=NULL;
105113

106114

107115
staticSnapshotCopySnapshot(Snapshotsnapshot);
@@ -125,23 +133,27 @@ GetTransactionSnapshot(void)
125133
if (!FirstSnapshotSet)
126134
{
127135
Assert(RegisteredSnapshots==0);
136+
Assert(FirstXactSnapshot==NULL);
128137

129138
/*
130139
* In transaction-snapshot mode, the first snapshot must live until
131140
* end of xact regardless of what the caller does with it, so we must
132-
* register it internally here and unregister it at end of xact.
141+
* make a copy of it rather than returning CurrentSnapshotData
142+
* directly.
133143
*/
134144
if (IsolationUsesXactSnapshot())
135145
{
146+
/* First, create the snapshot in CurrentSnapshotData */
136147
if (IsolationIsSerializable())
137-
CurrentSnapshot=RegisterSerializableTransaction(&CurrentSnapshotData);
148+
CurrentSnapshot=GetSerializableTransactionSnapshot(&CurrentSnapshotData);
138149
else
139-
{
140150
CurrentSnapshot=GetSnapshotData(&CurrentSnapshotData);
141-
CurrentSnapshot=RegisterSnapshotOnOwner(CurrentSnapshot,
142-
TopTransactionResourceOwner);
143-
}
144-
registered_xact_snapshot= true;
151+
/* Make a saved copy */
152+
CurrentSnapshot=CopySnapshot(CurrentSnapshot);
153+
FirstXactSnapshot=CurrentSnapshot;
154+
/* Mark it as "registered" in FirstXactSnapshot */
155+
FirstXactSnapshot->regd_count++;
156+
RegisteredSnapshots++;
145157
}
146158
else
147159
CurrentSnapshot=GetSnapshotData(&CurrentSnapshotData);
@@ -522,33 +534,30 @@ AtSubAbort_Snapshot(int level)
522534
SnapshotResetXmin();
523535
}
524536

525-
/*
526-
* AtEarlyCommit_Snapshot
527-
*
528-
* Snapshot manager's cleanup function, to be called on commit, before
529-
* doing resowner.c resource release.
530-
*/
531-
void
532-
AtEarlyCommit_Snapshot(void)
533-
{
534-
/*
535-
* In transaction-snapshot mode we must unregister our private refcount to
536-
* the transaction-snapshot.
537-
*/
538-
if (registered_xact_snapshot)
539-
UnregisterSnapshotFromOwner(CurrentSnapshot,
540-
TopTransactionResourceOwner);
541-
registered_xact_snapshot= false;
542-
543-
}
544-
545537
/*
546538
* AtEOXact_Snapshot
547539
*Snapshot manager's cleanup function for end of transaction
548540
*/
549541
void
550542
AtEOXact_Snapshot(boolisCommit)
551543
{
544+
/*
545+
* In transaction-snapshot mode we must release our privately-managed
546+
* reference to the transaction snapshot. We must decrement
547+
* RegisteredSnapshots to keep the check below happy. But we don't bother
548+
* to do FreeSnapshot, for two reasons: the memory will go away with
549+
* TopTransactionContext anyway, and if someone has left the snapshot
550+
* stacked as active, we don't want the code below to be chasing through
551+
* a dangling pointer.
552+
*/
553+
if (FirstXactSnapshot!=NULL)
554+
{
555+
Assert(FirstXactSnapshot->regd_count>0);
556+
Assert(RegisteredSnapshots>0);
557+
RegisteredSnapshots--;
558+
}
559+
FirstXactSnapshot=NULL;
560+
552561
/* On commit, complain about leftover snapshots */
553562
if (isCommit)
554563
{
@@ -574,5 +583,6 @@ AtEOXact_Snapshot(bool isCommit)
574583
SecondarySnapshot=NULL;
575584

576585
FirstSnapshotSet= false;
577-
registered_xact_snapshot= false;
586+
587+
SnapshotResetXmin();
578588
}

‎src/include/storage/predicate.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ extern void CheckPointPredicate(void);
4242
externboolPageIsPredicateLocked(Relationrelation,BlockNumberblkno);
4343

4444
/* predicate lock maintenance */
45-
externSnapshotRegisterSerializableTransaction(Snapshotsnapshot);
45+
externSnapshotGetSerializableTransactionSnapshot(Snapshotsnapshot);
4646
externvoidRegisterPredicateLockingXid(TransactionIdxid);
4747
externvoidPredicateLockRelation(Relationrelation,Snapshotsnapshot);
4848
externvoidPredicateLockPage(Relationrelation,BlockNumberblkno,Snapshotsnapshot);

‎src/include/utils/snapmgr.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ extern void UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner);
4040

4141
externvoidAtSubCommit_Snapshot(intlevel);
4242
externvoidAtSubAbort_Snapshot(intlevel);
43-
externvoidAtEarlyCommit_Snapshot(void);
4443
externvoidAtEOXact_Snapshot(boolisCommit);
4544

4645
#endif/* SNAPMGR_H */

0 commit comments

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