On Mon, Feb 26, 2018 at 10:20 PM, Masahiko Sawada <sawada.m...@gmail.com> wrote:
> Thank you for updating patches!
>
> 0001 patch looks good to me except for the following unnecessary empty lines.
>
> +                * If there are no indexes then we should periodically
> vacuum the FSM
> +                * on huge relations to make free space visible early.
> +                */
> +               else if (nindexes == 0 && fsm_updated_pages >
> vacuum_fsm_every_pages)
> +               {
> +                       /* Vacuum the Free Space Map */
> +                       FreeSpaceMapVacuum(onerel, max_freespace);
> +                       fsm_updated_pages = 0;
> +                       max_freespace = 0;
> +               }
> +
> +
> +               /*
>
> @@ -913,10 +967,14 @@ lazy_scan_heap(Relation onerel, int options,
> LVRelStats *vacrelstats,
>
> vmbuffer, InvalidTransactionId,
>
> VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
>                                 END_CRIT_SECTION();
> +
>                         }
>
> 0002 patch looks good to me.
>
> For 0003 patch, I think fsm_set() function name could lead misreading
> because it actually not only sets the value but also returns the
> value. fsm_set_and_get() might be better?

Attached is the patchset modified as requested
From bc79add4171beb1c8a0a714436cb4d7c27d6a2fc Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Fri, 28 Jul 2017 21:31:59 -0300
Subject: [PATCH 1/3] Vacuum: Update FSM more frequently

Make Vacuum update the FSM more frequently, to avoid the case where
autovacuum fails to reach the point where it updates the FSM in
highly contended tables.

Introduce a tree pruning threshold to FreeSpaceMapVacuum that avoids
recursing into branches that already contain enough free space, to
avoid having to traverse the whole FSM and thus induce quadratic
costs. Intermediate FSM vacuums are only supposed to make enough
free space visible to avoid extension until the final (non-partial)
FSM vacuum.

Run partial FSM vacuums after each index pass, which is a point
at which whole ranges of the heap have been thorougly cleaned, and
we can expect no further updates to those ranges of the FSM save
for concurrent activity. When there are no indexes, and thus no
index passes, run partial FSM vacuums every 8GB of dirtied pages
or 1/8th of the relation, whichever is highest. This allows some
partial work to be made visible without incurring quadratic cost.

In any case, FSM are small in relation to the table, so even when
quadratic cost is involved, it should not be problematic. Index
passes already incur quadratic cost, and the addition of the FSM
is unlikely to be measurable.

Run a partial FSM vacuum with a low pruning threshold right at
the beginning of the VACUUM to finish up any work left over from
prior canceled vacuum runs, something that is common in highly
contended tables when running only autovacuum.
---
 src/backend/access/brin/brin.c            |  2 +-
 src/backend/access/brin/brin_pageops.c    | 10 ++--
 src/backend/commands/vacuumlazy.c         | 81 ++++++++++++++++++++++++++++---
 src/backend/storage/freespace/freespace.c | 27 +++++++++--
 src/backend/storage/freespace/indexfsm.c  |  2 +-
 src/include/storage/freespace.h           |  2 +-
 6 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 68b3371..24d6df7 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -1461,5 +1461,5 @@ brin_vacuum_scan(Relation idxrel, BufferAccessStrategy strategy)
 	 * the way to the top.
 	 */
 	if (vacuum_fsm)
-		FreeSpaceMapVacuum(idxrel);
+		FreeSpaceMapVacuum(idxrel, 0);
 }
diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 60a7025..d4c7a87 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -136,7 +136,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
 				brin_initialize_empty_new_buffer(idxrel, newbuf);
 			UnlockReleaseBuffer(newbuf);
 			if (extended)
-				FreeSpaceMapVacuum(idxrel);
+				FreeSpaceMapVacuum(idxrel, 0);
 		}
 		return false;
 	}
@@ -156,7 +156,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
 				brin_initialize_empty_new_buffer(idxrel, newbuf);
 			UnlockReleaseBuffer(newbuf);
 			if (extended)
-				FreeSpaceMapVacuum(idxrel);
+				FreeSpaceMapVacuum(idxrel, 0);
 		}
 		return false;
 	}
@@ -211,7 +211,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
 		LockBuffer(oldbuf, BUFFER_LOCK_UNLOCK);
 
 		if (extended)
-			FreeSpaceMapVacuum(idxrel);
+			FreeSpaceMapVacuum(idxrel, 0);
 
 		return true;
 	}
@@ -313,7 +313,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
 		{
 			Assert(BlockNumberIsValid(newblk));
 			RecordPageWithFreeSpace(idxrel, newblk, freespace);
-			FreeSpaceMapVacuum(idxrel);
+			FreeSpaceMapVacuum(idxrel, 0);
 		}
 
 		return true;
@@ -457,7 +457,7 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
 	LockBuffer(revmapbuf, BUFFER_LOCK_UNLOCK);
 
 	if (extended)
-		FreeSpaceMapVacuum(idxrel);
+		FreeSpaceMapVacuum(idxrel, 0);
 
 	return off;
 }
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index cf7f5e1..a529d55 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -85,6 +85,20 @@
 #define VACUUM_TRUNCATE_LOCK_TIMEOUT			5000	/* ms */
 
 /*
+ * When a table has no indexes, vacuum the FSM at most every 1/Nth
+ * of the relation has been vacuumed to prevent bloat during long-running
+ * vacuums. This specifies the N.
+ */
+#define VACUUM_FSM_EVERY_FRACTION 8
+
+/*
+ * When a table has no indexes, vacuum the FSM at most every this
+ * many dirty pages. With a default page size of 8kb, this value
+ * basically means 8GB of dirtied pages.
+ */
+#define VACUUM_FSM_EVERY_PAGES 1048576
+
+/*
  * Guesstimation of number of dead tuples per page.  This is used to
  * provide an upper limit to memory allocated when vacuuming small
  * tables.
@@ -146,7 +160,7 @@ static BufferAccessStrategy vac_strategy;
 static void lazy_scan_heap(Relation onerel, int options,
 			   LVRelStats *vacrelstats, Relation *Irel, int nindexes,
 			   bool aggressive);
-static void lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats);
+static Size lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats);
 static bool lazy_check_needs_freeze(Buffer buf, bool *hastup);
 static void lazy_vacuum_index(Relation indrel,
 				  IndexBulkDeleteResult **stats,
@@ -287,7 +301,7 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
 								 PROGRESS_VACUUM_PHASE_FINAL_CLEANUP);
 
 	/* Vacuum the Free Space Map */
-	FreeSpaceMapVacuum(onerel);
+	FreeSpaceMapVacuum(onerel, 0);
 
 	/*
 	 * Update statistics in pg_class.
@@ -470,7 +484,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 	TransactionId relfrozenxid = onerel->rd_rel->relfrozenxid;
 	TransactionId relminmxid = onerel->rd_rel->relminmxid;
 	BlockNumber empty_pages,
-				vacuumed_pages;
+				vacuumed_pages,
+				fsm_updated_pages,
+				vacuum_fsm_every_pages;
 	double		num_tuples,
 				tups_vacuumed,
 				nkeep,
@@ -480,6 +496,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 	PGRUsage	ru0;
 	Buffer		vmbuffer = InvalidBuffer;
 	BlockNumber next_unskippable_block;
+	Size		max_freespace = 0;
 	bool		skipping_blocks;
 	xl_heap_freeze_tuple *frozen;
 	StringInfoData buf;
@@ -504,7 +521,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 						get_namespace_name(RelationGetNamespace(onerel)),
 						relname)));
 
-	empty_pages = vacuumed_pages = 0;
+	empty_pages = vacuumed_pages = fsm_updated_pages = 0;
 	num_tuples = tups_vacuumed = nkeep = nunused = 0;
 
 	indstats = (IndexBulkDeleteResult **)
@@ -517,6 +534,16 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 	vacrelstats->nonempty_pages = 0;
 	vacrelstats->latestRemovedXid = InvalidTransactionId;
 
+	/*
+	 * Vacuum the FSM a few times in the middle if the relation is big
+	 * and has no indexes. Once every some amount of dirtied pages, or
+	 * fraction of the relation, whatever is bigger, to avoid quadratic cost.
+	 * If it has indexes, this is ignored, and the FSM is vacuumed after
+	 * each index pass.
+	 */
+	vacuum_fsm_every_pages = nblocks / VACUUM_FSM_EVERY_FRACTION;
+	vacuum_fsm_every_pages = Max(vacuum_fsm_every_pages, VACUUM_FSM_EVERY_PAGES);
+
 	lazy_space_alloc(vacrelstats, nblocks);
 	frozen = palloc(sizeof(xl_heap_freeze_tuple) * MaxHeapTuplesPerPage);
 
@@ -746,7 +773,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			pgstat_progress_update_multi_param(2, hvp_index, hvp_val);
 
 			/* Remove tuples from heap */
-			lazy_vacuum_heap(onerel, vacrelstats);
+			freespace = lazy_vacuum_heap(onerel, vacrelstats);
+			if (freespace > max_freespace)
+				max_freespace = freespace;
 
 			/*
 			 * Forget the now-vacuumed tuples, and press on, but be careful
@@ -756,12 +785,32 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			vacrelstats->num_dead_tuples = 0;
 			vacrelstats->num_index_scans++;
 
+			/*
+			 * Vacuum the Free Space Map to make the changes we made visible.
+			 * Don't recurse into branches with more than max_freespace,
+			 * as we can't set it higher already.
+			 */
+			FreeSpaceMapVacuum(onerel, max_freespace);
+			max_freespace = 0;
+
 			/* Report that we are once again scanning the heap */
 			pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
 										 PROGRESS_VACUUM_PHASE_SCAN_HEAP);
 		}
 
 		/*
+		 * If there are no indexes then we should periodically vacuum the FSM
+		 * on huge relations to make free space visible early.
+		 */
+		else if (nindexes == 0 && fsm_updated_pages > vacuum_fsm_every_pages)
+		{
+			/* Vacuum the Free Space Map */
+			FreeSpaceMapVacuum(onerel, max_freespace);
+			fsm_updated_pages = 0;
+			max_freespace = 0;
+		}
+
+		/*
 		 * Pin the visibility map page in case we need to mark the page
 		 * all-visible.  In most cases this will be very cheap, because we'll
 		 * already have the correct page pinned anyway.  However, it's
@@ -877,7 +926,11 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			MarkBufferDirty(buf);
 			UnlockReleaseBuffer(buf);
 
+			fsm_updated_pages++;
 			RecordPageWithFreeSpace(onerel, blkno, freespace);
+			if (freespace > max_freespace)
+				max_freespace = freespace;
+
 			continue;
 		}
 
@@ -916,7 +969,10 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			}
 
 			UnlockReleaseBuffer(buf);
+			fsm_updated_pages++;
 			RecordPageWithFreeSpace(onerel, blkno, freespace);
+			if (freespace > max_freespace)
+				max_freespace = freespace;
 			continue;
 		}
 
@@ -1170,6 +1226,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 			 */
 			vacrelstats->num_dead_tuples = 0;
 			vacuumed_pages++;
+			fsm_updated_pages++;
 		}
 
 		freespace = PageGetHeapFreeSpace(page);
@@ -1272,7 +1329,11 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 		 * taken if there are no indexes.)
 		 */
 		if (vacrelstats->num_dead_tuples == prev_dead_count)
+		{
 			RecordPageWithFreeSpace(onerel, blkno, freespace);
+			if (freespace > max_freespace)
+				max_freespace = freespace;
+		}
 	}
 
 	/* report that everything is scanned and vacuumed */
@@ -1392,13 +1453,16 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
  *		space on their pages.  Pages not having dead tuples recorded from
  *		lazy_scan_heap are not visited at all.
  *
+ *		Returns the maximum amount of free space on vacuumed pages.
+ *
  * Note: the reason for doing this as a second pass is we cannot remove
  * the tuples until we've removed their index entries, and we want to
  * process index entry removal in batches as large as possible.
  */
-static void
+static Size
 lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 {
+	Size		max_freespace = 0;
 	int			tupindex;
 	int			npages;
 	PGRUsage	ru0;
@@ -1433,6 +1497,9 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 		page = BufferGetPage(buf);
 		freespace = PageGetHeapFreeSpace(page);
 
+		if (freespace > max_freespace)
+			max_freespace = freespace;
+
 		UnlockReleaseBuffer(buf);
 		RecordPageWithFreeSpace(onerel, tblk, freespace);
 		npages++;
@@ -1449,6 +1516,8 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 					RelationGetRelationName(onerel),
 					tupindex, npages),
 			 errdetail_internal("%s", pg_rusage_show(&ru0))));
+
+	return max_freespace;
 }
 
 /*
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index dd8ae52..ff79d71 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -108,7 +108,7 @@ static Size fsm_space_cat_to_avail(uint8 cat);
 static int fsm_set_and_search(Relation rel, FSMAddress addr, uint16 slot,
 				   uint8 newValue, uint8 minValue);
 static BlockNumber fsm_search(Relation rel, uint8 min_cat);
-static uint8 fsm_vacuum_page(Relation rel, FSMAddress addr, bool *eof);
+static uint8 fsm_vacuum_page(Relation rel, FSMAddress addr, uint8 threshold, bool *eof);
 static BlockNumber fsm_get_lastblckno(Relation rel, FSMAddress addr);
 static void fsm_update_recursive(Relation rel, FSMAddress addr, uint8 new_cat);
 
@@ -374,9 +374,15 @@ FreeSpaceMapTruncateRel(Relation rel, BlockNumber nblocks)
 
 /*
  * FreeSpaceMapVacuum - scan and fix any inconsistencies in the FSM
+ *
+ * If threshold is nonzero, a partial scan is made, skipping branches
+ * that already contain that much free space. Such a can be faster
+ * if many branches can be skipped, but it can't guarantee all
+ * inconsistencies will be fixed, only those under branches with less
+ * free space recorded than the threshold.
  */
 void
-FreeSpaceMapVacuum(Relation rel)
+FreeSpaceMapVacuum(Relation rel, Size threshold)
 {
 	bool		dummy;
 
@@ -384,7 +390,7 @@ FreeSpaceMapVacuum(Relation rel)
 	 * Traverse the tree in depth-first order. The tree is stored physically
 	 * in depth-first order, so this should be pretty I/O efficient.
 	 */
-	fsm_vacuum_page(rel, FSM_ROOT_ADDRESS, &dummy);
+	fsm_vacuum_page(rel, FSM_ROOT_ADDRESS, fsm_space_avail_to_cat(threshold), &dummy);
 }
 
 /******** Internal routines ********/
@@ -783,9 +789,12 @@ fsm_search(Relation rel, uint8 min_cat)
 
 /*
  * Recursive guts of FreeSpaceMapVacuum
+ *
+ * If threshold is nonzero, a partial scan is made, skipping branches
+ * that already contain that much free space recorded.
  */
 static uint8
-fsm_vacuum_page(Relation rel, FSMAddress addr, bool *eof_p)
+fsm_vacuum_page(Relation rel, FSMAddress addr, uint8 threshold, bool *eof_p)
 {
 	Buffer		buf;
 	Page		page;
@@ -816,11 +825,19 @@ fsm_vacuum_page(Relation rel, FSMAddress addr, bool *eof_p)
 		{
 			int			child_avail;
 
+			/* Tree pruning for partial vacuums */
+			if (threshold)
+			{
+				child_avail = fsm_get_avail(page, slot);
+				if (child_avail >= threshold)
+					continue;
+			}
+
 			CHECK_FOR_INTERRUPTS();
 
 			/* After we hit end-of-file, just clear the rest of the slots */
 			if (!eof)
-				child_avail = fsm_vacuum_page(rel, fsm_get_child(addr, slot), &eof);
+				child_avail = fsm_vacuum_page(rel, fsm_get_child(addr, slot), threshold, &eof);
 			else
 				child_avail = 0;
 
diff --git a/src/backend/storage/freespace/indexfsm.c b/src/backend/storage/freespace/indexfsm.c
index e21047b..dd77a16 100644
--- a/src/backend/storage/freespace/indexfsm.c
+++ b/src/backend/storage/freespace/indexfsm.c
@@ -70,5 +70,5 @@ RecordUsedIndexPage(Relation rel, BlockNumber usedBlock)
 void
 IndexFreeSpaceMapVacuum(Relation rel)
 {
-	FreeSpaceMapVacuum(rel);
+	FreeSpaceMapVacuum(rel, 0);
 }
diff --git a/src/include/storage/freespace.h b/src/include/storage/freespace.h
index a517d7e..e8bdaa2 100644
--- a/src/include/storage/freespace.h
+++ b/src/include/storage/freespace.h
@@ -31,7 +31,7 @@ extern void XLogRecordPageWithFreeSpace(RelFileNode rnode, BlockNumber heapBlk,
 							Size spaceAvail);
 
 extern void FreeSpaceMapTruncateRel(Relation rel, BlockNumber nblocks);
-extern void FreeSpaceMapVacuum(Relation rel);
+extern void FreeSpaceMapVacuum(Relation rel, Size threshold);
 extern void UpdateFreeSpaceMap(Relation rel,
 				   BlockNumber startBlkNum,
 				   BlockNumber endBlkNum,
-- 
1.8.4.5

From 9f9f985e365f5c56ad194511a52d15f025c49967 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Thu, 8 Feb 2018 12:39:15 -0300
Subject: [PATCH 2/3] Vacuum: do a partial FSM vacuum at the beginning

A partial FSM vacuum right at the start of the vacuum process
helps ameliorate issues with cancelled autovacuums never updating
the FSM.
---
 src/backend/commands/vacuumlazy.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index a529d55..c5803fe 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -117,6 +117,18 @@
  */
 #define PREFETCH_SIZE			((BlockNumber) 32)
 
+/*
+ * If autovacuums get regularly cancelled, the FSM may never be
+ * vacuumed. To work around that, we perform an initial partial
+ * FSM vacuum at the beginning of the vacuum process, to quickly
+ * make existing unmarked free space visible. To avoid useless
+ * redundant work, however, we avoid recursing into branches
+ * that already have a set amount of free space, we only try
+ * to discover unmarked free space. This value controls how
+ * much free space is enough free space in that context.
+ */
+#define INITIAL_PARTIAL_FSM_VACUUM_THRESHOLD	((Size) BLCKSZ/4)
+
 typedef struct LVRelStats
 {
 	/* hasindex = true means two-pass strategy; false means one-pass */
@@ -264,6 +276,17 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
 	vacrelstats->pages_removed = 0;
 	vacrelstats->lock_waiter_detected = false;
 
+	/*
+	 * Vacuum the Free Space Map partially before we start.
+	 * If an earlier vacuum was canceled, and that's likely in
+	 * highly contended tables, we may need to finish up. If we do
+	 * it now, we make the space visible to other backends regardless
+	 * of whether we succeed in finishing this time around.
+	 * Don't bother checking branches that already have usable space,
+	 * though.
+	 */
+	FreeSpaceMapVacuum(onerel, INITIAL_PARTIAL_FSM_VACUUM_THRESHOLD);
+
 	/* Open all indexes of the relation */
 	vac_open_indexes(onerel, RowExclusiveLock, &nindexes, &Irel);
 	vacrelstats->hasindex = (nindexes > 0);
-- 
1.8.4.5

From 48435c0e94eb945cf65f2fe4bc92a552f9f10843 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Mon, 26 Feb 2018 13:23:59 -0300
Subject: [PATCH 3/3] FSM: Fix relation extension FSM update

When updating the FSM recursively during relation extension,
the update could step on higher-level entries with an incorrect
value, by always bubbling up the leaf value instead of the root
value that is the result of the bubble up in fsm_set_avail.

If somehow the root contained a different value from the new value
set by fsm_update_recursive, upper levels would be set to the
wrong value instead.

This didn't happen often since the value set during relation
extension is usually pretty high and unlikely to be beaten by
other pre-existing values, but the possibility existed nonetheless,
especially during bulk loads.
---
 src/backend/storage/freespace/freespace.c | 36 ++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index ff79d71..5d305c1 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -107,6 +107,8 @@ static Size fsm_space_cat_to_avail(uint8 cat);
 /* workhorse functions for various operations */
 static int fsm_set_and_search(Relation rel, FSMAddress addr, uint16 slot,
 				   uint8 newValue, uint8 minValue);
+static uint8 fsm_set_and_get_maxval(Relation rel, FSMAddress addr, uint16 slot,
+									uint8 newValue);
 static BlockNumber fsm_search(Relation rel, uint8 min_cat);
 static uint8 fsm_vacuum_page(Relation rel, FSMAddress addr, uint8 threshold, bool *eof);
 static BlockNumber fsm_get_lastblckno(Relation rel, FSMAddress addr);
@@ -700,6 +702,33 @@ fsm_set_and_search(Relation rel, FSMAddress addr, uint16 slot,
 }
 
 /*
+ * Set value in given FSM page and slot.
+ *
+ * The new value at the root node is returned.
+ */
+static uint8
+fsm_set_and_get_maxval(Relation rel, FSMAddress addr, uint16 slot, uint8 newValue)
+{
+	Buffer		buf;
+	Page		page;
+	uint8		newmax = 0;
+
+	buf = fsm_readbuf(rel, addr, true);
+	LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
+
+	page = BufferGetPage(buf);
+
+	if (fsm_set_avail(page, slot, newValue))
+		MarkBufferDirtyHint(buf, false);
+
+	newmax = fsm_get_avail(page, 0);
+
+	UnlockReleaseBuffer(buf);
+
+	return newmax;
+}
+
+/*
  * Search the tree for a heap page with at least min_cat of free space
  */
 static BlockNumber
@@ -901,6 +930,11 @@ fsm_update_recursive(Relation rel, FSMAddress addr, uint8 new_cat)
 	 * information in that.
 	 */
 	parent = fsm_get_parent(addr, &parentslot);
-	fsm_set_and_search(rel, parent, parentslot, new_cat, 0);
+	new_cat = fsm_set_and_get_maxval(rel, parent, parentslot, new_cat);
+
+	/*
+	 * Bubble up, not the value we just set, but the one now in the root
+	 * node of the just-updated page, which is the page's highest value.
+	 */
 	fsm_update_recursive(rel, parent, new_cat);
 }
-- 
1.8.4.5

Reply via email to