NotificationsYou must be signed in to change notification settings
Fork28
Star151

Commitfa7ff64

committed

Fix minor nbtree page deletion buffer lock issue.

Avoid accessing the deletion target page's special area during nbtreepage deletion at a point where there is no buffer lock held. This issuewas detected by a patch that extends Valgrind's memcheck tool to marknbtree pages that are unsafe to access (due to not having a buffer lockor 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 totighten up the rules in this area. There is no reason to not simplyinsist on always holding a buffer lock (not just a pin) when accessingnbtree pages.Update some related comments in passing.Discussion:https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com

1 parentf246ea3 commitfa7ff64Copy full SHA for fa7ff64

File tree

1 file changed

-3

lines changed

src/backend/access/nbtree
- nbtpage.c

1 file changed

-3

lines changed

`‎src/backend/access/nbtree/nbtpage.c`

Lines changed: 9 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -1611,7 +1611,7 @@ _bt_pagedel(Relation rel, Buffer buf)`
`1611`	`1611`	`* to-be-deleted doesn't have a downlink, and the page`
`1612`	`1612`	`* deletion algorithm isn't prepared to handle that.`
`1613`	`1613`	`*/`
`1614`		`-if (!P_LEFTMOST(opaque))`
	`1614`	`+if (leftsib!=P_NONE)`
`1615`	`1615`	`{`
`1616`	`1616`	`BTPageOpaquelopaque;`
`1617`	`1617`	`Pagelpage;`
`@@ -1695,6 +1695,12 @@ _bt_pagedel(Relation rel, Buffer buf)`
`1695`	`1695`	`* the only child of the parent, and could be removed. It would be`
`1696`	`1696`	`* picked up by the next vacuum anyway, but might as well try to`
`1697`	`1697`	`* 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.`
`1698`	`1704`	`*/`
`1699`	`1705`	`if (!rightsib_empty)`
`1700`	`1706`	`break;`
`@@ -1904,8 +1910,8 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack)`
`1904`	`1910`	`* leaf page is deleted.`
`1905`	`1911`	`*`
`1906`	`1912`	`* 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`	`+ *setto true.`
`1909`	`1915`	`*`
`1910`	`1916`	`* Must hold pin and lock on leafbuf at entry (read or write doesn't matter).`
`1911`	`1917`	`* On success exit, we'll be holding pin and write lock. On failure exit,`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitfa7ff64

File tree

1 file changed

1 file changed

`‎src/backend/access/nbtree/nbtpage.c`

0 commit comments