Hi, On Sat, Apr 1, 2017 at 7:06 AM, Ashutosh Sharma <ashu.coe...@gmail.com> wrote: > On Sat, Apr 1, 2017 at 6:00 AM, Robert Haas <robertmh...@gmail.com> wrote: >> On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coe...@gmail.com> >> wrote: >>> Well, That is another option but the main idea was to be inline with >>> the btree code. >> >> That's not a bad goal in principal, but _bt_killitems() doesn't have >> any similar argument. > > It was there but later got removed with some patch (may be the patch > for reducing pinning and buffer content lock for btree scans). The > following commit has this changes. > > commit 09cb5c0e7d6fbc9dee26dc429e4fc0f2a88e5272 > Author: Tom Lane <t...@sss.pgh.pa.us> > Date: Sun May 7 01:21:30 2006 +0000 > >> >> (Also, speaking of consistency, why did we end up with >> _hash_kill_items, with an underscore between kill and items, and >> _bt_killitems, without one?) > > That is just to follow the naming convention in hash.h Please check > the review comments for this at - [1]. > >> >>> Moreover, I am not sure if acquiring lwlock inside >>> hashendscan (basically the place where we are trying to close down the >>> things) would look good. >> >> Well, if that's not a good thing to do, hiding it inside some other >> function doesn't make it better. > > okay, agreed. I will submit the patch very shortly.
As suggested, I am now acquiring lock inside the caller function. Attached is the patch having this changes. Thanks. > > > [1] - > https://www.postgresql.org/message-id/cf6abd8a-77b5-f1c7-8e50-5bef461e0522%40redhat.com -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 34cc08f..b835f77 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -476,9 +476,17 @@ hashrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys, HashScanOpaque so = (HashScanOpaque) scan->opaque; Relation rel = scan->indexRelation; - /* Before leaving current page, deal with any killed items */ + /* + * Before leaving current page, deal with any killed items. + * Also, ensure that we acquire lock on current page before + * calling _hash_kill_items. + */ if (so->numKilled > 0) + { + LockBuffer(so->hashso_curbuf, BUFFER_LOCK_SHARE); _hash_kill_items(scan); + LockBuffer(so->hashso_curbuf, BUFFER_LOCK_UNLOCK); + } _hash_dropscanbuf(rel, so); @@ -507,9 +515,17 @@ hashendscan(IndexScanDesc scan) HashScanOpaque so = (HashScanOpaque) scan->opaque; Relation rel = scan->indexRelation; - /* Before leaving current page, deal with any killed items */ + /* + * Before leaving current page, deal with any killed items. + * Also, ensure that we acquire lock on current page before + * calling _hash_kill_items. + */ if (so->numKilled > 0) + { + LockBuffer(so->hashso_curbuf, BUFFER_LOCK_SHARE); _hash_kill_items(scan); + LockBuffer(so->hashso_curbuf, BUFFER_LOCK_UNLOCK); + } _hash_dropscanbuf(rel, so);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers