On Tue, 9 Mar 2021 at 17:21, Mark Dilger <mark.dil...@enterprisedb.com> wrote:
>
> For a prior discussion on this topic:
>
> https://www.postgresql.org/message-id/2e78013d0709130606l56539755wb9dbe17225ffe90a%40mail.gmail.com
Thanks for the reference! I note that that thread mentions the
old-style VACUUM FULL as a reason as to why it would be unsafe, which
I believe was removed quite a few versions ago (v9.0).
The only two existing mechanisms that I could find (in the access/heap
directory) that possibly could fail on shrunken line pointer arrays;
being xlog recovery (I do not have enough knowledge on recovery to
determine if that may touch pages that have shrunken line pointer
arrays, or if those situations won't exist due to never using dirtied
pages in recovery) and backwards table scans on non-MVCC snapshots
(which would be fixed in the attached patch).
With regards,
Matthias van de Meent
From ef3db93a172ceb2bcab5516336462117d3c99fd3 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekew...@gmail.com>
Date: Tue, 9 Mar 2021 14:42:52 +0100
Subject: [PATCH v2] Truncate a pages' line pointer array when it has trailing
unused ItemIds.
This will allow reuse of what is effectively free space for data as well as
new line pointers, instead of keeping it reserved for line pointers only.
An additional benefit is that the HasFreeLinePointers hint-bit optimization
now doesn't hint for free line pointers at the end of the array, slightly
increasing the specificity of where the free lines are; and saving us from
needing to search to the end of the array if all other entries are already
filled.
---
src/backend/access/heap/heapam.c | 9 ++++++++-
src/backend/storage/page/bufpage.c | 13 ++++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 3b435c107d..ae218cfac6 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -635,8 +635,15 @@ heapgettup(HeapScanDesc scan,
}
else
{
+ /*
+ * The previous returned tuple may have been vacuumed since the
+ * previous scan when we use a non-MVCC snapshot, so we must
+ * re-establish the lineoff <= PageGetMaxOffsetNumber(dp)
+ * invariant
+ */
lineoff = /* previous offnum */
- OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self)));
+ Min(lines,
+ OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self))));
}
/* page and lineoff now reference the physically previous tid */
diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index 9ac556b4ae..10d8f26ad0 100644
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -672,7 +672,11 @@ compactify_tuples(itemIdCompact itemidbase, int nitems, Page page, bool presorte
* PageRepairFragmentation
*
* Frees fragmented space on a page.
- * It doesn't remove unused line pointers! Please don't change this.
+ * It doesn't remove intermediate unused line pointers (that would mean
+ * moving ItemIds, and that would imply invalidating indexed values), but it
+ * does truncate the page->pd_linp array to the last unused line pointer, so
+ * that this space may also be reused for data, instead of only for line
+ * pointers.
*
* This routine is usable for heap pages only, but see PageIndexMultiDelete.
*
@@ -691,6 +695,7 @@ PageRepairFragmentation(Page page)
int nline,
nstorage,
nunused;
+ OffsetNumber lastUsed = InvalidOffsetNumber;
int i;
Size totallen;
bool presorted = true; /* For now */
@@ -724,6 +729,7 @@ PageRepairFragmentation(Page page)
lp = PageGetItemId(page, i);
if (ItemIdIsUsed(lp))
{
+ lastUsed = i;
if (ItemIdHasStorage(lp))
{
itemidptr->offsetindex = i - 1;
@@ -771,6 +777,11 @@ PageRepairFragmentation(Page page)
compactify_tuples(itemidbase, nstorage, page, presorted);
}
+ if (lastUsed != nline) {
+ ((PageHeader) page)->pd_lower = SizeOfPageHeaderData + (sizeof(ItemIdData) * lastUsed);
+ nunused = nunused - (nline - lastUsed);
+ }
+
/* Set hint bit for PageAddItem */
if (nunused > 0)
PageSetHasFreeLinePointers(page);
--
2.20.1
