0003-FSM-Fix-relation-extension-FSM-update-v2.patch

text/x-patch

Filename: 0003-FSM-Fix-relation-extension-FSM-update-v2.patch
Type: text/x-patch
Part: 2
Message: Re: [HACKERS] [PATCH] Vacuum: Update FSM more frequently

Patch

Format: format-patch
Series: patch v2-0003
Subject: FSM: Fix relation extension FSM update
File+
src/backend/storage/freespace/freespace.c 35 1
From 48435c0e94eb945cf65f2fe4bc92a552f9f10843 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfreire@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