Re: Write Ahead Logging for Hash Indexes

On Thu, Feb 16, 2017 at 8:16 PM, Amit Kapila <amit.kapila16@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