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

Commit fa7ff64

Browse files
Fix minor nbtree page deletion buffer lock issue.
Avoid accessing the deletion target page's special area during nbtree page deletion at a point where there is no buffer lock held. This issue was detected by a patch that extends Valgrind's memcheck tool to mark nbtree pages that are unsafe to access (due to not having a buffer lock or buffer pin) as NOACCESS. We do hold a buffer pin at this point, and only access the special area, so the old approach was safe. Even still, it seems like a good idea to tighten up the rules in this area. There is no reason to not simply insist on always holding a buffer lock (not just a pin) when accessing nbtree pages. Update some related comments in passing. Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com
1 parent f246ea3 commit fa7ff64

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

src/backend/access/nbtree/nbtpage.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,7 +1611,7 @@ _bt_pagedel(Relation rel, Buffer buf)
16111611
* to-be-deleted doesn't have a downlink, and the page
16121612
* deletion algorithm isn't prepared to handle that.
16131613
*/
1614-
if (!P_LEFTMOST(opaque))
1614+
if (leftsib != P_NONE)
16151615
{
16161616
BTPageOpaque lopaque;
16171617
Page lpage;
@@ -1695,6 +1695,12 @@ _bt_pagedel(Relation rel, Buffer buf)
16951695
* the only child of the parent, and could be removed. It would be
16961696
* picked up by the next vacuum anyway, but might as well try to
16971697
* remove it now, so loop back to process the right sibling.
1698+
*
1699+
* Note: This relies on the assumption that _bt_getstackbuf() will be
1700+
* able to reuse our original descent stack with a different child
1701+
* block (provided that the child block is to the right of the
1702+
* original leaf page reached by _bt_search()). It will even update
1703+
* the descent stack each time we loop around, avoiding repeated work.
16981704
*/
16991705
if (!rightsib_empty)
17001706
break;
@@ -1904,8 +1910,8 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack)
19041910
* leaf page is deleted.
19051911
*
19061912
* Returns 'false' if the page could not be unlinked (shouldn't happen).
1907-
* If the (new) right sibling of the page is empty, *rightsib_empty is set
1908-
* to true.
1913+
* If the (current) right sibling of the page is empty, *rightsib_empty is
1914+
* set to true.
19091915
*
19101916
* Must hold pin and lock on leafbuf at entry (read or write doesn't matter).
19111917
* On success exit, we'll be holding pin and write lock. On failure exit,

0 commit comments

Comments
 (0)