On Thu, Feb 16, 2017 at 8:16 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > Attached are refactoring patches. WAL patch needs some changes based > on above comments, so will post it later.
After some study, I have committed 0001, and also committed 0002 and 0003 as a single commit, with only minor tweaks. I haven't made a full study of 0004 yet, but I'm a bit concerned about a couple of the hunks, specifically this one: @@ -985,9 +962,9 @@ _hash_splitbucket_guts(Relation rel, if (PageGetFreeSpace(npage) < itemsz) { - /* write out nbuf and drop lock, but keep pin */ - MarkBufferDirty(nbuf); + /* drop lock, but keep pin */ LockBuffer(nbuf, BUFFER_LOCK_UNLOCK); + /* chain to a new overflow page */ nbuf = _hash_addovflpage(rel, metabuf, nbuf, (nbuf == bucket_nbuf) ? true : false); npage = BufferGetPage(nbuf); And also this one: @@ -1041,17 +1024,6 @@ _hash_splitbucket_guts(Relation rel, * To avoid deadlocks due to locking order of buckets, first lock the old * bucket and then the new bucket. */ - if (nbuf == bucket_nbuf) - { - MarkBufferDirty(bucket_nbuf); - LockBuffer(bucket_nbuf, BUFFER_LOCK_UNLOCK); - } - else - { - MarkBufferDirty(nbuf); - _hash_relbuf(rel, nbuf); - } - LockBuffer(bucket_obuf, BUFFER_LOCK_EXCLUSIVE); opage = BufferGetPage(bucket_obuf); oopaque = (HashPageOpaque) PageGetSpecialPointer(opage); I haven't quite grasped what's going on here, but it looks like those MarkBufferDirty() calls didn't move someplace else, but rather just vanished. That would seem to be not good. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers