Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7154aa1

Browse files
Fix another minor page deletion buffer lock issue.
Avoid accessing the leaf page's top parent tuple without a buffer lock held during the second phase of nbtree page deletion. The old approach was safe, though only because VACUUM never drops its buffer pin (and because only VACUUM itself can modify a half-dead page). Even still, it seems like a good idea to be strict here. Tighten things up by copying the top parent page's block number to a local variable before releasing the buffer lock on the leaf page -- not after. This is a follow-up to commit fa7ff64, which fixed a similar issue in the first phase of nbtree page deletion. Update some related comments in passing. Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com
1 parent fa7ff64 commit 7154aa1

File tree

1 file changed

+20
-13
lines changed

1 file changed

+20
-13
lines changed

src/backend/access/nbtree/nbtpage.c

+20-13
Original file line numberDiff line numberDiff line change
@@ -1951,6 +1951,7 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
19511951
*/
19521952
itemid = PageGetItemId(page, P_HIKEY);
19531953
leafhikey = (IndexTuple) PageGetItem(page, itemid);
1954+
target = BTreeTupleGetTopParent(leafhikey);
19541955
leafleftsib = opaque->btpo_prev;
19551956
leafrightsib = opaque->btpo_next;
19561957

@@ -1965,36 +1966,36 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
19651966
/*
19661967
* If the leaf page still has a parent pointing to it (or a chain of
19671968
* parents), we don't unlink the leaf page yet, but the topmost remaining
1968-
* parent in the branch. Set 'target' and 'buf' to reference the page
1969-
* actually being unlinked.
1969+
* parent in the branch (i.e. the "top parent")
19701970
*/
1971-
target = BTreeTupleGetTopParent(leafhikey);
1971+
if (!BlockNumberIsValid(target))
1972+
{
1973+
/* No top parent, so target is leaf page */
1974+
target = leafblkno;
19721975

1973-
if (target != InvalidBlockNumber)
1976+
buf = leafbuf;
1977+
leftsib = leafleftsib;
1978+
targetlevel = 0;
1979+
}
1980+
else
19741981
{
1982+
/* Target is the internal page taken from leaf's top parent */
19751983
Assert(target != leafblkno);
19761984

1977-
/* fetch the block number of the topmost parent's left sibling */
1985+
/* Fetch the block number of the target's left sibling */
19781986
buf = _bt_getbuf(rel, target, BT_READ);
19791987
page = BufferGetPage(buf);
19801988
opaque = (BTPageOpaque) PageGetSpecialPointer(page);
19811989
leftsib = opaque->btpo_prev;
19821990
targetlevel = opaque->btpo.level;
1991+
Assert(targetlevel > 0);
19831992

19841993
/*
19851994
* To avoid deadlocks, we'd better drop the target page lock before
19861995
* going further.
19871996
*/
19881997
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
19891998
}
1990-
else
1991-
{
1992-
target = leafblkno;
1993-
1994-
buf = leafbuf;
1995-
leftsib = leafleftsib;
1996-
targetlevel = 0;
1997-
}
19981999

19992000
/*
20002001
* We have to lock the pages we need to modify in the standard order:
@@ -2181,6 +2182,12 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, bool *rightsib_empty)
21812182
* If we deleted a parent of the targeted leaf page, instead of the leaf
21822183
* itself, update the leaf to point to the next remaining child in the
21832184
* branch.
2185+
*
2186+
* Note: We rely on the fact that a buffer pin on the leaf page has been
2187+
* held since leafhikey was initialized. This is safe, though only
2188+
* because the page was already half-dead at that point. The leaf page
2189+
* cannot have been modified by any other backend during the period when
2190+
* no lock was held.
21842191
*/
21852192
if (target != leafblkno)
21862193
BTreeTupleSetTopParent(leafhikey, nextchild);

0 commit comments

Comments
 (0)