Skip to content

Commit a7212be

Browse files
tglsfdctglsfdc
committed
Set cutoff xmin more aggressively when vacuuming a temporary table.
Since other sessions aren't allowed to look into a temporary table of our own session, we do not need to worry about the global xmin horizon when setting the vacuum XID cutoff. Indeed, if we're not inside a transaction block, we may set oldestXmin to be the next XID, because there cannot be any in-doubt tuples in a temp table, nor any tuples that are dead but still visible to some snapshot of our transaction. (VACUUM, of course, is never inside a transaction block; but we need to test that because CLUSTER shares the same code.) This approach allows us to always clean out a temp table completely during VACUUM, independently of concurrent activity. Aside from being useful in its own right, that simplifies building reproducible test cases. Discussion: https://postgr.es/m/3490536.1598629609@sss.pgh.pa.us
1 parent db864c3 commit a7212be

File tree

5 files changed

+70
-36
lines changed

5 files changed

+70
-36
lines changed

‎src/backend/access/heap/vacuumlazy.c

+1
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
471471
params->freeze_table_age,
472472
params->multixact_freeze_min_age,
473473
params->multixact_freeze_table_age,
474+
true, /* we must be a top-level command */
474475
&OldestXmin, &FreezeLimit, &xidFullScanLimit,
475476
&MultiXactCutoff, &mxactFullScanLimit);
476477

‎src/backend/commands/cluster.c

+17-11
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,13 @@ typedef struct
6767
} RelToCluster;
6868

6969

70-
static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
70+
static void rebuild_relation(Relation OldHeap, Oid indexOid,
71+
bool isTopLevel, bool verbose);
7172
static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
72-
bool verbose, bool *pSwapToastByContent,
73-
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
73+
bool isTopLevel, bool verbose,
74+
bool *pSwapToastByContent,
75+
TransactionId *pFreezeXid,
76+
MultiXactId *pCutoffMulti);
7477
static List *get_tables_to_cluster(MemoryContext cluster_context);
7578

7679

@@ -170,7 +173,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
170173
table_close(rel, NoLock);
171174

172175
/* Do the job. */
173-
cluster_rel(tableOid, indexOid, stmt->options);
176+
cluster_rel(tableOid, indexOid, stmt->options, isTopLevel);
174177
}
175178
else
176179
{
@@ -219,7 +222,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
219222
PushActiveSnapshot(GetTransactionSnapshot());
220223
/* Do the job. */
221224
cluster_rel(rvtc->tableOid, rvtc->indexOid,
222-
stmt->options | CLUOPT_RECHECK);
225+
stmt->options | CLUOPT_RECHECK,
226+
isTopLevel);
223227
PopActiveSnapshot();
224228
CommitTransactionCommand();
225229
}
@@ -250,7 +254,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
250254
* and error messages should refer to the operation as VACUUM not CLUSTER.
251255
*/
252256
void
253-
cluster_rel(Oid tableOid, Oid indexOid, int options)
257+
cluster_rel(Oid tableOid, Oid indexOid, int options, bool isTopLevel)
254258
{
255259
Relation OldHeap;
256260
bool verbose = ((options & CLUOPT_VERBOSE) != 0);
@@ -400,7 +404,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
400404
TransferPredicateLocksToHeapRelation(OldHeap);
401405

402406
/* rebuild_relation does all the dirty work */
403-
rebuild_relation(OldHeap, indexOid, verbose);
407+
rebuild_relation(OldHeap, indexOid, isTopLevel, verbose);
404408

405409
/* NB: rebuild_relation does table_close() on OldHeap */
406410

@@ -545,11 +549,12 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
545549
*
546550
* OldHeap: table to rebuild --- must be opened and exclusive-locked!
547551
* indexOid: index to cluster by, or InvalidOid to rewrite in physical order.
552+
* isTopLevel: should be passed down from ProcessUtility.
548553
*
549554
* NB: this routine closes OldHeap at the right time; caller should not.
550555
*/
551556
static void
552-
rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
557+
rebuild_relation(Relation OldHeap, Oid indexOid, bool isTopLevel, bool verbose)
553558
{
554559
Oid tableOid = RelationGetRelid(OldHeap);
555560
Oid tableSpace = OldHeap->rd_rel->reltablespace;
@@ -577,7 +582,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
577582
AccessExclusiveLock);
578583

579584
/* Copy the heap data into the new table in the desired order */
580-
copy_table_data(OIDNewHeap, tableOid, indexOid, verbose,
585+
copy_table_data(OIDNewHeap, tableOid, indexOid, isTopLevel, verbose,
581586
&swap_toast_by_content, &frozenXid, &cutoffMulti);
582587

583588
/*
@@ -728,7 +733,8 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
728733
* *pCutoffMulti receives the MultiXactId used as a cutoff point.
729734
*/
730735
static void
731-
copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
736+
copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
737+
bool isTopLevel, bool verbose,
732738
bool *pSwapToastByContent, TransactionId *pFreezeXid,
733739
MultiXactId *pCutoffMulti)
734740
{
@@ -826,7 +832,7 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
826832
* Since we're going to rewrite the whole table anyway, there's no reason
827833
* not to be aggressive about this.
828834
*/
829-
vacuum_set_xid_limits(OldHeap, 0, 0, 0, 0,
835+
vacuum_set_xid_limits(OldHeap, 0, 0, 0, 0, isTopLevel,
830836
&OldestXmin, &FreezeXid, NULL, &MultiXactCutoff,
831837
NULL);
832838

‎src/backend/commands/vacuum.c

+49-24
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,9 @@ get_all_vacuum_rels(int options)
907907
/*
908908
* vacuum_set_xid_limits() -- compute oldestXmin and freeze cutoff points
909909
*
910+
* Input parameters are the target relation, applicable freeze age settings,
911+
* and isTopLevel which should be passed down from ProcessUtility.
912+
*
910913
* The output parameters are:
911914
* - oldestXmin is the cutoff value used to distinguish whether tuples are
912915
* DEAD or RECENTLY_DEAD (see HeapTupleSatisfiesVacuum).
@@ -931,6 +934,7 @@ vacuum_set_xid_limits(Relation rel,
931934
int freeze_table_age,
932935
int multixact_freeze_min_age,
933936
int multixact_freeze_table_age,
937+
bool isTopLevel,
934938
TransactionId *oldestXmin,
935939
TransactionId *freezeLimit,
936940
TransactionId *xidFullScanLimit,
@@ -946,32 +950,53 @@ vacuum_set_xid_limits(Relation rel,
946950
MultiXactId mxactLimit;
947951
MultiXactId safeMxactLimit;
948952

949-
/*
950-
* We can always ignore processes running lazy vacuum. This is because we
951-
* use these values only for deciding which tuples we must keep in the
952-
* tables. Since lazy vacuum doesn't write its XID anywhere (usually no
953-
* XID assigned), it's safe to ignore it. In theory it could be
954-
* problematic to ignore lazy vacuums in a full vacuum, but keep in mind
955-
* that only one vacuum process can be working on a particular table at
956-
* any time, and that each vacuum is always an independent transaction.
957-
*/
958-
*oldestXmin = GetOldestNonRemovableTransactionId(rel);
959-
960-
if (OldSnapshotThresholdActive())
953+
if (RELATION_IS_LOCAL(rel) && !IsInTransactionBlock(isTopLevel))
954+
{
955+
/*
956+
* If we are processing a temp relation (which by prior checks must be
957+
* one belonging to our session), and we are not inside any
958+
* transaction block, then there can be no tuples in the rel that are
959+
* still in-doubt, nor can there be any that are dead but possibly
960+
* still interesting to some snapshot our session holds. We don't
961+
* need to care whether other sessions could see such tuples, either.
962+
* So we can aggressively set the cutoff xmin to be the nextXid.
963+
*/
964+
*oldestXmin = ReadNewTransactionId();
965+
}
966+
else
961967
{
962-
TransactionId limit_xmin;
963-
TimestampTz limit_ts;
968+
/*
969+
* Otherwise, calculate the cutoff xmin normally.
970+
*
971+
* We can always ignore processes running lazy vacuum. This is
972+
* because we use these values only for deciding which tuples we must
973+
* keep in the tables. Since lazy vacuum doesn't write its XID
974+
* anywhere (usually no XID assigned), it's safe to ignore it. In
975+
* theory it could be problematic to ignore lazy vacuums in a full
976+
* vacuum, but keep in mind that only one vacuum process can be
977+
* working on a particular table at any time, and that each vacuum is
978+
* always an independent transaction.
979+
*/
980+
*oldestXmin = GetOldestNonRemovableTransactionId(rel);
964981

965-
if (TransactionIdLimitedForOldSnapshots(*oldestXmin, rel, &limit_xmin, &limit_ts))
982+
if (OldSnapshotThresholdActive())
966983
{
967-
/*
968-
* TODO: We should only set the threshold if we are pruning on the
969-
* basis of the increased limits. Not as crucial here as it is for
970-
* opportunistic pruning (which often happens at a much higher
971-
* frequency), but would still be a significant improvement.
972-
*/
973-
SetOldSnapshotThresholdTimestamp(limit_ts, limit_xmin);
974-
*oldestXmin = limit_xmin;
984+
TransactionId limit_xmin;
985+
TimestampTz limit_ts;
986+
987+
if (TransactionIdLimitedForOldSnapshots(*oldestXmin, rel,
988+
&limit_xmin, &limit_ts))
989+
{
990+
/*
991+
* TODO: We should only set the threshold if we are pruning on
992+
* the basis of the increased limits. Not as crucial here as
993+
* it is for opportunistic pruning (which often happens at a
994+
* much higher frequency), but would still be a significant
995+
* improvement.
996+
*/
997+
SetOldSnapshotThresholdTimestamp(limit_ts, limit_xmin);
998+
*oldestXmin = limit_xmin;
999+
}
9751000
}
9761001
}
9771002

@@ -1905,7 +1930,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
19051930
cluster_options |= CLUOPT_VERBOSE;
19061931

19071932
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
1908-
cluster_rel(relid, InvalidOid, cluster_options);
1933+
cluster_rel(relid, InvalidOid, cluster_options, true);
19091934
}
19101935
else
19111936
table_relation_vacuum(onerel, params, vac_strategy);

‎src/include/commands/cluster.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020

2121
extern void cluster(ClusterStmt *stmt, bool isTopLevel);
22-
extern void cluster_rel(Oid tableOid, Oid indexOid, int options);
22+
extern void cluster_rel(Oid tableOid, Oid indexOid, int options,
23+
bool isTopLevel);
2324
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
2425
bool recheck, LOCKMODE lockmode);
2526
extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);

‎src/include/commands/vacuum.h

+1
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ extern void vacuum_set_xid_limits(Relation rel,
267267
int freeze_min_age, int freeze_table_age,
268268
int multixact_freeze_min_age,
269269
int multixact_freeze_table_age,
270+
bool isTopLevel,
270271
TransactionId *oldestXmin,
271272
TransactionId *freezeLimit,
272273
TransactionId *xidFullScanLimit,

0 commit comments

Comments
 (0)