Re: Microvacuum support for Hash Index - Mailing list pgsql-hackers

On Mon, Oct 24, 2016 at 2:21 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> Hi All,
>
> I have added a microvacuum support for hash index access method and
> attached is the v1 patch for the same.
>

This is an important functionality for hash index as we already do
have same functionality for other types of indexes like btree.

> The patch basically takes care
> of the following things:
>
> 1. Firstly, it changes the marking of dead tuples from
> 'tuple-at-a-time' to 'page-at-a-time' during hash index scan. For this
> we accumulate the heap tids and offset of all the hash index tuples if
> it is pointed by kill_prior_tuple during scan and then mark all
> accumulated tids as LP_DEAD either while stepping from one page to
> another (assuming the scan in both forward and backward direction) or
> during end of the hash index scan or during rescan.
>
> 2. Secondly, when inserting tuple into hash index table, if not enough
> space is found on a current page then it ensures that we first clean
> the dead tuples if found in the current hash index page before moving
> to the next page in a bucket chain or going for a bucket split. This
> basically increases the page reusability and reduces the number of
> page splits, thereby reducing the overall size of hash index table.
>

Few comments on patch:

1.
+static void
+hash_xlog_vacuum_one_page(XLogReaderState *record)
+{
+ XLogRecPtr lsn = record->EndRecPtr;
+ xl_hash_vacuum *xldata = (xl_hash_vacuum *) XLogRecGetData(record);
+ Buffer bucketbuf = InvalidBuffer;
+ Buffer buffer;
+ Buffer metabuf;
+ Page page;
+ XLogRedoAction action;

While replaying the delete/vacuum record on standby, it can conflict
with some already running queries.  Basically the replay can remove
some row which can be visible on standby.  You need to resolve
conflicts similar to what we do in btree delete records (refer
btree_xlog_delete).

2.
+ /*
+ * Write-lock the meta page so that we can decrement
+ * tuple count.
+ */
+ _hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
+
+ _hash_vacuum_one_page(rel, metabuf, buf, bucket_buf,
+  (buf == bucket_buf) ? true : false);
+
+ _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);

It seems here meta page lock is acquired for duration more than
required and also it is not required when there are no deletable items
on page. You can take the metapage lock before decrementing the count.

3. Assert(offnum <= maxoff);
+

Spurious space.  There are some other similar spurious white space
changes in patch, remove them as well.



-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com