Dan Ports wrote: > On Wed, Jun 08, 2011 at 09:17:04PM -0500, Kevin Grittner wrote: >> A patch is attached which just covers the predicate lock >> acquisition, where a snapshot is available without too much pain. >> There are two functions which acquire predicate locks where a >> snapshot was not readily available: _bt_search() and >> _bt_get_endpoint(). Not only was it not clear how to get a >> snapshot in, it was not entirely clear from reading the code that >> we need to acquire predicate locks here. Now, I suspect that we >> probably do, because I spent many long hours stepping through gdb >> to pick the spots where they are, but that was about a year ago >> and my memory of the details has faded. > > For _bt_search(), the lock calls should move to _bt_first() where > the ScanDesc is available. This also keeps us from trying to take > locks during _bt_pagedel(), which is only called during vacuum and > recovery. Sounds reasonable, but why did you pass the snapshot to the PredicateLockPage() call but not the PredicateLockRelation() call? Oversight? > The call in _bt_get_endpoint() seems unnecessary, because after it > returns, _bt_endpoint() takes the same lock. The only other callers > of _bt_get_endpoint() are _bt_pagedel() and _bt_insert_parent(), > neither of which should take predicate locks. That also sounds reasonable. > I've updated the patch, attached. I've confirmed that it passes the usual regression tests (including isolation tests and the normal regression tests at serializable). I'll take a closer look once I wake up and get the caffeine going. Thanks for following up on this! -Kevin
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
