Re: Write Ahead Logging for Hash Indexes

Hi,

On 09/07/2016 05:58 AM, Amit Kapila wrote:
> Okay, I have fixed this issue as explained above. Apart from that, I
> have fixed another issue reported by Mark Kirkwood upthread and few
> other issues found during internal testing by Ashutosh Sharma.
>
> The locking issue reported by Mark and Ashutosh is that the patch
> didn't maintain the locking order while adding overflow page as it
> maintains in other write operations (lock the bucket pages first and
> then metapage to perform the write operation). I have added the
> comments in _hash_addovflpage() to explain the locking order used in
> modified patch.
>
> During stress testing with pgbench using master-standby setup, we
> found an issue which indicates that WAL replay machinery doesn't
> expect completely zeroed pages (See explanation of RBM_NORMAL mode
> atop XLogReadBufferExtended). Previously before freeing the overflow
> page we were zeroing it, now I have changed it to just initialize the
> page such that the page will be empty.
>
> Apart from above, I have added support for old snapshot threshold in
> the hash index code.
>
> Thanks to Ashutosh Sharma for doing the testing of the patch and
> helping me in analyzing some of the above issues.
>
> I forgot to mention in my initial mail that Robert and I had some
> off-list discussions about the design of this patch, many thanks to
> him for providing inputs.
>

Some initial feedback.

README:
+in_complete split flag. The reader algorithm works correctly, as it
will scan

What flag ?

hashxlog.c:

hash_xlog_move_page_contents
hash_xlog_squeeze_page

Both have "bukcetbuf" (-> "bucketbuf"), and

+ if (BufferIsValid(bukcetbuf));

->

+ if (BufferIsValid(bucketbuf))

and indent following line:

LockBufferForCleanup(bukcetbuf);

hash_xlog_delete

has the "if" issue too.

hash.h:

Move the XLog related content to hash_xlog.h

Best regards,
Jesper