Re: Microvacuum support for Hash Index

On Wed, Mar 15, 2017 at 2:10 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> Okay, I have done the changes as suggested by you. Please refer to the
> attached v8 patch. Thanks.

Cool, but this doesn't look right:

+ action = XLogReadBufferForRedo(record, 0, &buffer);
+
+ if (!IsBufferCleanupOK(buffer))
+ elog(PANIC, "hash_xlog_vacuum_one_page: failed to acquire
cleanup lock");

That could fail, I think, because of a pin from a Hot Standby backend.
You want to call XLogReadBufferForRedoExtended() with a third argument
of true. Come to think of it, shouldn't hash_xlog_split_allocate_page
be changed the same way?

+ /*
+ * Let us mark the page as clean if vacuum removes the DEAD tuples
+ * from an index page. We do this by clearing
LH_PAGE_HAS_DEAD_TUPLES
+ * flag.
+ */

Maybe add: Clearing this flag is just a hint; replay won't redo this.

+ * Hash Index records that are marked as LP_DEAD and being removed during
+ * hash index tuple insertion can conflict with standby queries.You might

The word Index shouldn't be capitalized here. There should be a space
before "You".

The formatting of this comment is oddly narrow:

+ * _hash_vacuum_one_page - vacuum just one index page.
+ * Try to remove LP_DEAD items from the given page. We
+ * must acquire cleanup lock on the page being modified
+ * before calling this function.

I'd add a blank line after the first line and reflow the rest to be
something more like 75 characters. pgindent evidently doesn't think
this needs reformatting, but it's oddly narrow.

I suggest changing the header comment of
hash_xlog_vacuum_get_latestRemovedXid like this:

+ * Get the latestRemovedXid from the heap pages pointed at by the index
+ * tuples being deleted. See also btree_xlog_delete_get_latestRemovedXid,
+ * on which this function is based.

This is looking good.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company