Re: pgsql: Optimize btree insertions for common case of increasing values

On Mon, Mar 26, 2018 at 5:10 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> Optimize btree insertions for common case of increasing values
>
> Remember the last page of an index insert if it's the rightmost leaf
> page. If the next entry belongs on and can fit in the remembered page,
> insert the new entry there as long as we can get a lock on the page.
> Otherwise, fall back on the more expensive method of searching for
> the right place to insert the entry.

This code can allocate memory in a critical section, since
RelationSetTargetBlock() can call smgropen():

            if (P_ISLEAF(lpageop))
+           {
                xlinfo = XLOG_BTREE_INSERT_LEAF;
+
+               /*
+                * Cache the block information if we just inserted into the
+                * rightmost leaf page of the index.
+                */
+               if (P_RIGHTMOST(lpageop))
+                   RelationSetTargetBlock(rel, BufferGetBlockNumber(buf));
+           }

rd_smgr can be closed by a relcache flush, so it is in fact possible
that we'll reach smgropen().

As I already pointed out, it's unclear why some of these tests are
used, such as P_INCOMPLETE_SPLIT():

+           /*
+            * Check if the page is still the rightmost leaf page, has enough
+            * free space to accommodate the new tuple, no split is in progress
+            * and the scankey is greater than or equal to the first key on the
+            * page.
+            */
+           if (P_ISLEAF(lpageop) && P_RIGHTMOST(lpageop) &&
+                   !P_INCOMPLETE_SPLIT(lpageop) &&
+                   !P_IGNORE(lpageop) &&
+                   (PageGetFreeSpace(page) > itemsz) &&
+                   PageGetMaxOffsetNumber(page) >= P_FIRSTDATAKEY(lpageop) &&
+                   _bt_compare(rel, natts, itup_scankey, page,
+                       P_FIRSTDATAKEY(lpageop)) > 0)

The BTP_INCOMPLETE_SPLIT bit means that the page's right sibling's
downlink is missing, but how can a rightmost page have a right sibling
in the first place? We don't bother to check that when we already know
the page is rightmost within _bt_moveright() or _bt_findinsertloc().

I also noticed that favorable/favourable is misspelled "favourble".
Also, "workout" is a noun.

This patch was committed in haste, and it shows.

-- 
Peter Geoghegan