On Wed, Mar 15, 2017 at 2:10 PM, Ashutosh Sharma <ashu.coek88@gmail.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
