Skip to content

Navigation Menu

Sign in

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
Appearance settings

Commit be504a3

Browse filesBrowse files
anarazelanarazel
committed
Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids
When vacuum_defer_cleanup_age is bigger than the current xid, including the epoch, the subtraction of vacuum_defer_cleanup_age would lead to a wrapped around xid. While that normally is not a problem, the subsequent conversion to a 64bit xid results in a 64bit-xid very far into the future. As that xid is used as a horizon to detect whether rows versions are old enough to be removed, that allows removal of rows that are still visible (i.e. corruption). If vacuum_defer_cleanup_age was never changed from the default, there is no chance of this bug occurring. This bug was introduced in dc7420c. A lesser version of it exists in 12-13, introduced by fb5344c, affecting only GiST. The 12-13 version of the issue can, in rare cases, lead to pages in a gist index getting recycled too early, potentially causing index entries to be found multiple times. The fix is fairly simple - don't allow vacuum_defer_cleanup_age to retreat further than FirstNormalTransactionId. Patches to make similar bugs easier to find, by adding asserts to the 64bit xid infrastructure, have been proposed, but are not suitable for backpatching. Currently there are no tests for vacuum_defer_cleanup_age. A patch introducing infrastructure to make writing a test easier has been posted to the list. Reported-by: Michail Nikolaev <michail.nikolaev@gmail.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de Backpatch: 12-, but impact/fix is smaller for 12-13
1 parent a4e0033 commit be504a3
Copy full SHA for be504a3

File tree

1 file changed

+73
-12
lines changed
Filter options

1 file changed

+73
-12
lines changed

‎src/backend/storage/ipc/procarray.c

Copy file name to clipboardExpand all lines: src/backend/storage/ipc/procarray.c
+73-12
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,9 @@ static inline void ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId l
367367
static void ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid);
368368
static void MaintainLatestCompletedXid(TransactionId latestXid);
369369
static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
370+
static void TransactionIdRetreatSafely(TransactionId *xid,
371+
int retreat_by,
372+
FullTransactionId rel);
370373

371374
static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
372375
TransactionId xid);
@@ -1888,17 +1891,35 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
18881891
* so guc.c should limit it to no more than the xidStopLimit threshold
18891892
* in varsup.c. Also note that we intentionally don't apply
18901893
* vacuum_defer_cleanup_age on standby servers.
1894+
*
1895+
* Need to use TransactionIdRetreatSafely() instead of open-coding the
1896+
* subtraction, to prevent creating an xid before
1897+
* FirstNormalTransactionId.
18911898
*/
1892-
h->oldest_considered_running =
1893-
TransactionIdRetreatedBy(h->oldest_considered_running,
1894-
vacuum_defer_cleanup_age);
1895-
h->shared_oldest_nonremovable =
1896-
TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
1897-
vacuum_defer_cleanup_age);
1898-
h->data_oldest_nonremovable =
1899-
TransactionIdRetreatedBy(h->data_oldest_nonremovable,
1900-
vacuum_defer_cleanup_age);
1901-
/* defer doesn't apply to temp relations */
1899+
Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
1900+
h->shared_oldest_nonremovable));
1901+
Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
1902+
h->data_oldest_nonremovable));
1903+
1904+
if (vacuum_defer_cleanup_age > 0)
1905+
{
1906+
TransactionIdRetreatSafely(&h->oldest_considered_running,
1907+
vacuum_defer_cleanup_age,
1908+
h->latest_completed);
1909+
TransactionIdRetreatSafely(&h->shared_oldest_nonremovable,
1910+
vacuum_defer_cleanup_age,
1911+
h->latest_completed);
1912+
TransactionIdRetreatSafely(&h->data_oldest_nonremovable,
1913+
vacuum_defer_cleanup_age,
1914+
h->latest_completed);
1915+
/* defer doesn't apply to temp relations */
1916+
1917+
1918+
Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
1919+
h->shared_oldest_nonremovable));
1920+
Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
1921+
h->data_oldest_nonremovable));
1922+
}
19021923
}
19031924

19041925
/*
@@ -2470,8 +2491,10 @@ GetSnapshotData(Snapshot snapshot)
24702491
oldestfxid = FullXidRelativeTo(latest_completed, oldestxid);
24712492

24722493
/* apply vacuum_defer_cleanup_age */
2473-
def_vis_xid_data =
2474-
TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age);
2494+
def_vis_xid_data = xmin;
2495+
TransactionIdRetreatSafely(&def_vis_xid_data,
2496+
vacuum_defer_cleanup_age,
2497+
oldestfxid);
24752498

24762499
/* Check whether there's a replication slot requiring an older xmin. */
24772500
def_vis_xid_data =
@@ -4295,6 +4318,44 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
42954318
return GlobalVisTestIsRemovableXid(state, xid);
42964319
}
42974320

4321+
/*
4322+
* Safely retract *xid by retreat_by, store the result in *xid.
4323+
*
4324+
* Need to be careful to prevent *xid from retreating below
4325+
* FirstNormalTransactionId during epoch 0. This is important to prevent
4326+
* generating xids that cannot be converted to a FullTransactionId without
4327+
* wrapping around.
4328+
*
4329+
* If retreat_by would lead to a too old xid, FirstNormalTransactionId is
4330+
* returned instead.
4331+
*/
4332+
static void
4333+
TransactionIdRetreatSafely(TransactionId *xid, int retreat_by, FullTransactionId rel)
4334+
{
4335+
TransactionId original_xid = *xid;
4336+
FullTransactionId fxid;
4337+
uint64 fxid_i;
4338+
4339+
Assert(TransactionIdIsNormal(original_xid));
4340+
Assert(retreat_by >= 0); /* relevant GUCs are stored as ints */
4341+
AssertTransactionIdInAllowableRange(original_xid);
4342+
4343+
if (retreat_by == 0)
4344+
return;
4345+
4346+
fxid = FullXidRelativeTo(rel, original_xid);
4347+
fxid_i = U64FromFullTransactionId(fxid);
4348+
4349+
if ((fxid_i - FirstNormalTransactionId) <= retreat_by)
4350+
*xid = FirstNormalTransactionId;
4351+
else
4352+
{
4353+
*xid = TransactionIdRetreatedBy(original_xid, retreat_by);
4354+
Assert(TransactionIdIsNormal(*xid));
4355+
Assert(NormalTransactionIdPrecedes(*xid, original_xid));
4356+
}
4357+
}
4358+
42984359
/*
42994360
* Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
43004361
* is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.