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

Commit dd01834

Browse files
committed
Avoid repeating loads of frozen ID values.
Repeating loads of inplace-updated fields tends to cause bugs like the one from the previous commit. While there's no bug to fix in these code sites, adopt the load-once style. This improves the chance of future copy/paste finding the safe style. Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com
1 parent f65ab86 commit dd01834

File tree

4 files changed

+34
-21
lines changed

4 files changed

+34
-21
lines changed

src/backend/access/heap/heapam.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5907,7 +5907,6 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
59075907
Page page;
59085908
BlockNumber block;
59095909
Buffer buffer;
5910-
TransactionId prune_xid;
59115910

59125911
Assert(ItemPointerIsValid(tid));
59135912

@@ -5960,11 +5959,16 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
59605959
* TransactionXmin, so there's no race here).
59615960
*/
59625961
Assert(TransactionIdIsValid(TransactionXmin));
5963-
if (TransactionIdPrecedes(TransactionXmin, relation->rd_rel->relfrozenxid))
5964-
prune_xid = relation->rd_rel->relfrozenxid;
5965-
else
5966-
prune_xid = TransactionXmin;
5967-
PageSetPrunable(page, prune_xid);
5962+
{
5963+
TransactionId relfrozenxid = relation->rd_rel->relfrozenxid;
5964+
TransactionId prune_xid;
5965+
5966+
if (TransactionIdPrecedes(TransactionXmin, relfrozenxid))
5967+
prune_xid = relfrozenxid;
5968+
else
5969+
prune_xid = TransactionXmin;
5970+
PageSetPrunable(page, prune_xid);
5971+
}
59685972

59695973
/* store transaction information of xact deleting the tuple */
59705974
tp.t_data->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED);

src/backend/commands/cluster.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -919,18 +919,24 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
919919
* FreezeXid will become the table's new relfrozenxid, and that mustn't go
920920
* backwards, so take the max.
921921
*/
922-
if (TransactionIdIsValid(OldHeap->rd_rel->relfrozenxid) &&
923-
TransactionIdPrecedes(cutoffs.FreezeLimit,
924-
OldHeap->rd_rel->relfrozenxid))
925-
cutoffs.FreezeLimit = OldHeap->rd_rel->relfrozenxid;
922+
{
923+
TransactionId relfrozenxid = OldHeap->rd_rel->relfrozenxid;
924+
925+
if (TransactionIdIsValid(relfrozenxid) &&
926+
TransactionIdPrecedes(cutoffs.FreezeLimit, relfrozenxid))
927+
cutoffs.FreezeLimit = relfrozenxid;
928+
}
926929

927930
/*
928931
* MultiXactCutoff, similarly, shouldn't go backwards either.
929932
*/
930-
if (MultiXactIdIsValid(OldHeap->rd_rel->relminmxid) &&
931-
MultiXactIdPrecedes(cutoffs.MultiXactCutoff,
932-
OldHeap->rd_rel->relminmxid))
933-
cutoffs.MultiXactCutoff = OldHeap->rd_rel->relminmxid;
933+
{
934+
MultiXactId relminmxid = OldHeap->rd_rel->relminmxid;
935+
936+
if (MultiXactIdIsValid(relminmxid) &&
937+
MultiXactIdPrecedes(cutoffs.MultiXactCutoff, relminmxid))
938+
cutoffs.MultiXactCutoff = relminmxid;
939+
}
934940

935941
/*
936942
* Decide whether to use an indexscan or seqscan-and-optional-sort to scan

src/backend/commands/vacuum.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
12001200
aggressiveXIDCutoff = nextXID - freeze_table_age;
12011201
if (!TransactionIdIsNormal(aggressiveXIDCutoff))
12021202
aggressiveXIDCutoff = FirstNormalTransactionId;
1203-
if (TransactionIdPrecedesOrEquals(rel->rd_rel->relfrozenxid,
1203+
if (TransactionIdPrecedesOrEquals(cutoffs->relfrozenxid,
12041204
aggressiveXIDCutoff))
12051205
return true;
12061206

@@ -1221,7 +1221,7 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
12211221
aggressiveMXIDCutoff = nextMXID - multixact_freeze_table_age;
12221222
if (aggressiveMXIDCutoff < FirstMultiXactId)
12231223
aggressiveMXIDCutoff = FirstMultiXactId;
1224-
if (MultiXactIdPrecedesOrEquals(rel->rd_rel->relminmxid,
1224+
if (MultiXactIdPrecedesOrEquals(cutoffs->relminmxid,
12251225
aggressiveMXIDCutoff))
12261226
return true;
12271227

src/backend/postmaster/autovacuum.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2938,6 +2938,7 @@ relation_needs_vacanalyze(Oid relid,
29382938
int freeze_max_age;
29392939
int multixact_freeze_max_age;
29402940
TransactionId xidForceLimit;
2941+
TransactionId relfrozenxid;
29412942
MultiXactId multiForceLimit;
29422943

29432944
Assert(classForm != NULL);
@@ -2989,16 +2990,18 @@ relation_needs_vacanalyze(Oid relid,
29892990
xidForceLimit = recentXid - freeze_max_age;
29902991
if (xidForceLimit < FirstNormalTransactionId)
29912992
xidForceLimit -= FirstNormalTransactionId;
2992-
force_vacuum = (TransactionIdIsNormal(classForm->relfrozenxid) &&
2993-
TransactionIdPrecedes(classForm->relfrozenxid,
2994-
xidForceLimit));
2993+
relfrozenxid = classForm->relfrozenxid;
2994+
force_vacuum = (TransactionIdIsNormal(relfrozenxid) &&
2995+
TransactionIdPrecedes(relfrozenxid, xidForceLimit));
29952996
if (!force_vacuum)
29962997
{
2998+
MultiXactId relminmxid = classForm->relminmxid;
2999+
29973000
multiForceLimit = recentMulti - multixact_freeze_max_age;
29983001
if (multiForceLimit < FirstMultiXactId)
29993002
multiForceLimit -= FirstMultiXactId;
3000-
force_vacuum = MultiXactIdIsValid(classForm->relminmxid) &&
3001-
MultiXactIdPrecedes(classForm->relminmxid, multiForceLimit);
3003+
force_vacuum = MultiXactIdIsValid(relminmxid) &&
3004+
MultiXactIdPrecedes(relminmxid, multiForceLimit);
30023005
}
30033006
*wraparound = force_vacuum;
30043007

0 commit comments

Comments
 (0)