From ae6d536ed0dcb5e29126975d4a07eb308fdc5cfa Mon Sep 17 00:00:00 2001
From: Thomas Munro <tmunro@postgresql.org>
Date: Mon, 3 Jul 2023 16:16:27 +1200
Subject: [PATCH] Fix race in SSI interaction with empty btrees.
When predicate-locking btrees, we have a special case for completely
empty btrees, since there is no page to lock. This was racy, because,
without buffer lock held, a matching key could be inserted between the
_bt_search() and the PredicateLockRelation() calls.
Fix, by rechecking _bt_search() after taking the relation-level SIREAD
lock, if using SERIALIZABLE isolation and an empty btree is discovered.
Back-patch to all supported releases. Fixes one aspect of bug #17949.
Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com>
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
---
src/backend/access/nbtree/nbtsearch.c | 39 ++++++++++++++++++---------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index fdf0e5654a1..eba47c2f0cf 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -17,6 +17,7 @@
#include "access/nbtree.h"
#include "access/relscan.h"
+#include "access/xact.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/predicate.h"
@@ -1377,22 +1378,34 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
{
/*
* We only get here if the index is completely empty. Lock relation
- * because nothing finer to lock exists.
+ * because nothing finer to lock exists. Without a buffer lock, it's
+ * possible for another transaction to insert data between
+ * _bt_search() and PredicateLockRelation(). We have to try again
+ * after taking the relation-level predicate lock, to close a narrow
+ * window where we wouldn't scan concurrently inserted tuples, but the
+ * writer wouldn't see our predicate lock.
*/
- PredicateLockRelation(rel, scan->xs_snapshot);
+ if (IsolationIsSerializable())
+ {
+ PredicateLockRelation(rel, scan->xs_snapshot);
+ stack = _bt_search(rel, &inskey, &buf, BT_READ,
+ scan->xs_snapshot);
+ _bt_freestack(stack);
+ }
- /*
- * mark parallel scan as done, so that all the workers can finish
- * their scan
- */
- _bt_parallel_done(scan);
- BTScanPosInvalidate(so->currPos);
-
- return false;
+ if (!BufferIsValid(buf))
+ {
+ /*
+ * Mark parallel scan as done, so that all the workers can finish
+ * their scan.
+ */
+ _bt_parallel_done(scan);
+ BTScanPosInvalidate(so->currPos);
+ return false;
+ }
}
- else
- PredicateLockPage(rel, BufferGetBlockNumber(buf),
- scan->xs_snapshot);
+
+ PredicateLockPage(rel, BufferGetBlockNumber(buf), scan->xs_snapshot);
_bt_initialize_more_data(so, dir);